public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] Dynamic properties of pointers
@ 2023-09-04 22:29 Abdul Basit Ijaz
  2023-09-04 22:29 ` [PATCH v3 1/4] gdb, testsuite: handle icc and icpc deprecated remarks Abdul Basit Ijaz
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Abdul Basit Ijaz @ 2023-09-04 22:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: abdul.b.ijaz, simark, tom

From: "Ijaz, Abdul B" <abdul.b.ijaz@intel.com>

Hi!

Please find attached v3 of this series where for v2 series there is
already some feedback and the main change in patch 3 review is still
missing to fix 'some compiler DWARF that is wrong but we still want to
support it' patch after a discussion in
https://sourceware.org/pipermail/gdb-patches/2022-September/192159.html

V2 patch 2 was approved by Tom already in this discussion but there are
minor changes since then:
https://sourceware.org/pipermail/gdb-patches/2023-January/195353.html

V2 can be found here:
https://sourceware.org/pipermail/gdb-patches/2022-October/192389.html

V1 with feedback can be found here:
https://sourceware.org/pipermail/gdb-patches/2022-September/191934.html

Changes since v2:

  * Patch 1 has minor change where now test for icc versions more
  generally.

  * Patch 2:
  Patch 2 has minor change in TYPE_CODE_PTR handling and rest was already
  reviewed in V2 series for handling of DW_AT_associated attribute
  in patch 3.

  * Patch 3:
  This already has the DW_AT_associated handling from V2 series and only
  handling of reference/pointer type is improved for Intel classic compilers.

  * Patch 4: Added a comment to the change for handling of
  DW_TAG_pointer_type.

I'm looking forward to comments.

Thanks & Best Regards,
Abdul Basit

Bernhard Heckel (1):
  gdb, types: Resolve pointer types dynamically

Nils-Christian Kempke (3):
  gdb, testsuite: handle icc and icpc deprecated remarks
  gdb, intel-classic-compilers, testsuite: workaround icc/icpc/ifort
    pointer/reference DWARF
  gdb, testsuite, fortran: Fix sizeof intrinsic for ifort Fortran
    pointers

 gdb/eval.c                                    |   9 +
 gdb/gdbtypes.c                                | 101 +++++++++-
 gdb/gdbtypes.h                                |   5 +
 gdb/testsuite/gdb.cp/vla-cxx.cc               |   4 +
 gdb/testsuite/gdb.cp/vla-cxx.exp              |  15 ++
 gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp       |  16 +-
 .../icc-ifort-pointers-and-references.c       |  38 ++++
 .../icc-ifort-pointers-and-references.exp     | 169 +++++++++++++++++
 .../gdb.fortran/pointer-to-pointer.exp        |   2 +-
 gdb/testsuite/gdb.fortran/pointers.exp        | 173 ++++++++++++++++++
 gdb/testsuite/gdb.fortran/pointers.f90        |  29 +++
 gdb/testsuite/gdb.fortran/sizeof.exp          | 115 ++++++++++++
 gdb/testsuite/gdb.fortran/sizeof.f90          | 108 +++++++++++
 gdb/testsuite/lib/gdb.exp                     |  14 +-
 gdb/valprint.c                                |  40 +++-
 15 files changed, 812 insertions(+), 26 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.exp
 create mode 100644 gdb/testsuite/gdb.fortran/pointers.exp
 create mode 100644 gdb/testsuite/gdb.fortran/sizeof.exp
 create mode 100644 gdb/testsuite/gdb.fortran/sizeof.f90

-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 1/4] gdb, testsuite: handle icc and icpc deprecated remarks
  2023-09-04 22:29 [PATCH v3 0/4] Dynamic properties of pointers Abdul Basit Ijaz
@ 2023-09-04 22:29 ` Abdul Basit Ijaz
  2023-10-03  0:04   ` Thiago Jung Bauermann
  2023-09-04 22:29 ` [PATCH v3 2/4] gdb, types: Resolve pointer types dynamically Abdul Basit Ijaz
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Abdul Basit Ijaz @ 2023-09-04 22:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: abdul.b.ijaz, simark, tom

From: "Ijaz, Abdul B" <abdul.b.ijaz@intel.com>

Starting with icc/icpc version 2021.7.0 and higher both compilers emit a
deprecation remark when used.  E.g.

  >> icc --version
  icc: remark #10441: The Intel(R) C++ Compiler Classic (ICC) is
  deprecated and will be removed from product release in the second half
  of 2023. The Intel(R) oneAPI DPC++/C++ Compiler (ICX) is the recommended
  compiler moving forward. Please transition to use this compiler. Use
  '-diag-disable=10441' to disable this message.
  icc (ICC) 2021.7.0 20220713
  Copyright (C) 1985-2022 Intel Corporation.  All rights reserved.

  >> icpc --version
  icpc: remark #10441: The Intel(R) C++ Compiler Classic (ICC) is
  deprecated ...
  icpc (ICC) 2021.7.0 20220720
  Copyright (C) 1985-2022 Intel Corporation.  All rights reserved.

As the testsuite compile fails when unexpected output by the compiler is
seen this change in the compiler breaks all existing icc and icpc tests.
This patch makes the gdb testsuite more forgiving by a) allowing the
output of the remark when trying to figure out the compiler version
and by b) adding '-diag-disable=10441' to the compile command whenever
gdb_compile is called without the intention to detect the compiler.

gdb/testsuite/ChangeLog:
2022-07-20  Nils-Christian Kempke  <nils-christian.kempke@intel.com>

	* lib/gdb.exp: Handle icc/icpc deprecation warnings.

2023-09-04 Nils-Christian Kempke <nils-christian.kempke@intel.com>
---
 gdb/testsuite/lib/gdb.exp | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 1b9179401c4..def6a75dfee 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -4980,13 +4980,15 @@ proc gdb_compile {source dest type options} {
 	    }
 	}
 
-	# Starting with 2021.7.0 (recognized as icc-20-21-7 by GDB) icc and
-	# icpc are marked as deprecated and both compilers emit the remark
-	# #10441.  To let GDB still compile successfully, we disable these
-	# warnings here.
+	# Starting with 2021.7.0 (recognized as icc-20-21-7 by
+	# GDB) icc and icpc are marked as deprecated and both
+	# compilers emit the remark #10441.  To let GDB still
+	# compile successfully, we disable these warnings here.
 	if {([lsearch -exact $options c++] != -1
-	     && [test_compiler_info {icc-20-21-[7-9]} c++])
-	    || [test_compiler_info {icc-20-21-[7-9]}]} {
+	     && [test_compiler_info {icc-20-21-[7-9]} c++]
+	     && [test_compiler_info {icc-20-21-[1-9][0-9]} c++])
+	    || [test_compiler_info {icc-20-21-[7-9]}]
+	    || [test_compiler_info {icc-20-21-[1-9][0-9]}]} {
 	    lappend new_options "additional_flags=-diag-disable=10441"
 	}
     }
-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 2/4] gdb, types: Resolve pointer types dynamically
  2023-09-04 22:29 [PATCH v3 0/4] Dynamic properties of pointers Abdul Basit Ijaz
  2023-09-04 22:29 ` [PATCH v3 1/4] gdb, testsuite: handle icc and icpc deprecated remarks Abdul Basit Ijaz
@ 2023-09-04 22:29 ` Abdul Basit Ijaz
  2023-10-03  0:07   ` Thiago Jung Bauermann
  2023-10-10 19:49   ` Tom Tromey
  2023-09-04 22:29 ` [PATCH v3 3/4] gdb, intel-classic-compilers, testsuite: workaround icc/icpc/ifort pointer/reference DWARF Abdul Basit Ijaz
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 18+ messages in thread
From: Abdul Basit Ijaz @ 2023-09-04 22:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: abdul.b.ijaz, simark, tom

From: "Ijaz, Abdul B" <abdul.b.ijaz@intel.com>

This commit allows pointers to be dynamic types (on the outmost
level).  Similar to references, a pointer is considered a dynamic type
if its target type is a dynamic type and it is on the outmost level.

The pointer resolution follows the one of references.

This generally makes the GDB output more verbose.  We are able to print
more details about a pointer's target like the dimension of an array.

In Fortran, if we have a pointer to a dynamic type

  type buffer
    real, dimension(:), pointer :: ptr
  end type buffer
  type(buffer), pointer :: buffer_ptr
  allocate (buffer_ptr)
  allocate (buffer_ptr%ptr (5))

which then gets allocated, we now resolve the dynamic type before
printing the pointer's type:

Before:

  (gdb) ptype buffer_ptr
  type = PTR TO -> ( Type buffer
    real(kind=4) :: alpha(:)
  End Type buffer )

After:

  (gdb) ptype buffer_ptr
  type = PTR TO -> ( Type buffer
    real(kind=4) :: alpha(5)
  End Type buffer )

Similarly in C++ we can dynamically resolve e.g. pointers to arrays:

  int len = 3;
  int arr[len];
  int (*ptr)[len];
  int ptr = &arr;

Once the pointer is assigned one gets:

Before:

  (gdb) p ptr
  $1 = (int (*)[variable length]) 0x123456
  (gdb) ptype ptr
  type = int (*)[variable length]

After:

  (gdb) p ptr
  $1 = (int (*)[3]) 0x123456
  (gdb) ptype ptr
  type = int (*)[3]

For more examples see the modified/added test cases.
---
 gdb/gdbtypes.c                                |   7 +-
 gdb/testsuite/gdb.cp/vla-cxx.cc               |   4 +
 gdb/testsuite/gdb.cp/vla-cxx.exp              |  15 +++
 gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp       |  16 +--
 .../gdb.fortran/pointer-to-pointer.exp        |   2 +-
 gdb/testsuite/gdb.fortran/pointers.exp        | 115 ++++++++++++++++++
 gdb/testsuite/gdb.fortran/pointers.f90        |  29 +++++
 gdb/valprint.c                                |   6 -
 8 files changed, 177 insertions(+), 17 deletions(-)
 create mode 100644 gdb/testsuite/gdb.fortran/pointers.exp

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 5e15ec64c41..4b1787b62e6 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2038,8 +2038,10 @@ is_dynamic_type_internal (struct type *type, int top_level)
 {
   type = check_typedef (type);
 
-  /* We only want to recognize references at the outermost level.  */
-  if (top_level && type->code () == TYPE_CODE_REF)
+  /* We only want to recognize references and pointers at the outermost
+     level.  */
+  if (top_level
+      && (type->code () == TYPE_CODE_REF || type->code () == TYPE_CODE_PTR))
     type = check_typedef (type->target_type ());
 
   /* Types that have a dynamic TYPE_DATA_LOCATION are considered
@@ -2775,6 +2777,7 @@ resolve_dynamic_type_internal (struct type *type,
       switch (type->code ())
 	{
 	case TYPE_CODE_REF:
+	case TYPE_CODE_PTR:
 	  {
 	    struct property_addr_info pinfo;
 
diff --git a/gdb/testsuite/gdb.cp/vla-cxx.cc b/gdb/testsuite/gdb.cp/vla-cxx.cc
index 0ecb130f676..83e238339ac 100644
--- a/gdb/testsuite/gdb.cp/vla-cxx.cc
+++ b/gdb/testsuite/gdb.cp/vla-cxx.cc
@@ -40,6 +40,10 @@ int main(int argc, char **argv)
   typedef typeof (vla) &vlareftypedef;
   vlareftypedef vlaref2 (vla);
   container c;
+  typeof (vla) *ptr = nullptr;
+
+  // Before pointer assignment.
+  ptr = &vla;
 
   for (int i = 0; i < z; ++i)
     vla[i] = 5 + 2 * i;
diff --git a/gdb/testsuite/gdb.cp/vla-cxx.exp b/gdb/testsuite/gdb.cp/vla-cxx.exp
index 0a588220679..ade688cedda 100644
--- a/gdb/testsuite/gdb.cp/vla-cxx.exp
+++ b/gdb/testsuite/gdb.cp/vla-cxx.exp
@@ -23,6 +23,18 @@ if ![runto_main] {
     return -1
 }
 
+gdb_breakpoint [gdb_get_line_number "Before pointer assignment"]
+gdb_continue_to_breakpoint "Before pointer assignment"
+
+gdb_test "ptype ptr" "= int \\(\\*\\)\\\[3\\\]" \
+    "ptype ptr, before pointer assignment"
+
+gdb_test "print ptr" "= \\(int \\(\\*\\)\\\[3\\\]\\) 0x0" \
+    "print ptr, before pointer assignment"
+
+gdb_test "print *ptr" "Cannot access memory at address 0x0" \
+    "print *ptr, before pointer assignment"
+
 gdb_breakpoint [gdb_get_line_number "vlas_filled"]
 gdb_continue_to_breakpoint "vlas_filled"
 
@@ -33,3 +45,6 @@ gdb_test "print vlaref" " = \\(int \\(&\\)\\\[3\\\]\\) @$hex: \\{5, 7, 9\\}"
 # bug being tested, it's better not to depend on the exact spelling.
 gdb_test "print vlaref2" " = \\(.*\\) @$hex: \\{5, 7, 9\\}"
 gdb_test "print c" " = \\{e = \\{c = @$hex\\}\\}"
+gdb_test "ptype ptr" "int \\(\\*\\)\\\[3\\\]"
+gdb_test "print ptr" "\\(int \\(\\*\\)\\\[3\\\]\\) $hex"
+gdb_test "print *ptr" " = \\{5, 7, 9\\}"
diff --git a/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp b/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp
index 6e4a331eca8..d6b20086c89 100644
--- a/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp
+++ b/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp
@@ -154,7 +154,7 @@ gdb_test "print foo.three_ptr.all'length" \
          " = 3"
 
 gdb_test "ptype foo.three_ptr.all" \
-         " = array \\(<>\\) of integer"
+    " = array \\(1 \\.\\. 3\\) of integer"
 
 # foo.three_ptr
 
@@ -177,7 +177,7 @@ gdb_test "print foo.three_ptr'length" \
          " = 3"
 
 gdb_test "ptype foo.three_ptr" \
-         " = access array \\(<>\\) of integer"
+    " = access array \\(1 \\.\\. 3\\) of integer"
 
 # foo.three_ptr_tdef.all
 
@@ -203,7 +203,7 @@ gdb_test "print foo.three_ptr_tdef.all'length" \
          " = 3"
 
 gdb_test "ptype foo.three_ptr_tdef.all" \
-         " = array \\(<>\\) of integer"
+    " = array \\(1 \\.\\. 3\\) of integer"
 
 # foo.three_ptr_tdef
 
@@ -226,7 +226,7 @@ gdb_test "print foo.three_ptr_tdef'length" \
          " = 3"
 
 gdb_test "ptype foo.three_ptr_tdef" \
-         " = access array \\(<>\\) of integer"
+    " = access array \\(1 \\.\\. 3\\) of integer"
 
 # foo.five_ptr.all
 
@@ -258,7 +258,7 @@ gdb_test "print foo.five_ptr.all'length" \
          " = 5"
 
 gdb_test "ptype foo.five_ptr.all" \
-         " = array \\(<>\\) of integer"
+    " = array \\(2 \\.\\. 6\\) of integer"
 
 # foo.five_ptr
 
@@ -287,7 +287,7 @@ gdb_test "print foo.five_ptr'length" \
          " = 5"
 
 gdb_test "ptype foo.five_ptr" \
-         " = access array \\(<>\\) of integer"
+    " = access array \\(2 \\.\\. 6\\) of integer"
 
 # foo.five_ptr_tdef.all
 
@@ -319,7 +319,7 @@ gdb_test "print foo.five_ptr_tdef.all'length" \
          " = 5"
 
 gdb_test "ptype foo.five_ptr_tdef.all" \
-         " = array \\(<>\\) of integer"
+    " = array \\(2 \\.\\. 6\\) of integer"
 
 # foo.five_ptr_tdef
 
@@ -348,4 +348,4 @@ gdb_test "print foo.five_ptr_tdef'length" \
          " = 5"
 
 gdb_test "ptype foo.five_ptr_tdef" \
-         " = access array \\(<>\\) of integer"
+    " = access array \\(2 \\.\\. 6\\) of integer"
diff --git a/gdb/testsuite/gdb.fortran/pointer-to-pointer.exp b/gdb/testsuite/gdb.fortran/pointer-to-pointer.exp
index 0fd5fb0996f..78b7ba17588 100644
--- a/gdb/testsuite/gdb.fortran/pointer-to-pointer.exp
+++ b/gdb/testsuite/gdb.fortran/pointer-to-pointer.exp
@@ -41,7 +41,7 @@ gdb_test "print buffer" \
 gdb_test "ptype buffer" \
     [multi_line \
 	 "type = PTR TO -> \\( Type l_buffer" \
-	 "    $real4 :: alpha\\(:\\)" \
+	 "    $real4 :: alpha\\(5\\)" \
 	 "End Type l_buffer \\)" ]
 gdb_test "ptype buffer%alpha" "type = $real4 \\(5\\)"
 
diff --git a/gdb/testsuite/gdb.fortran/pointers.exp b/gdb/testsuite/gdb.fortran/pointers.exp
new file mode 100644
index 00000000000..ca2195bbfe6
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/pointers.exp
@@ -0,0 +1,115 @@
+# Copyright 2016-2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile "pointers.f90"
+load_lib fortran.exp
+
+if {[prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} \
+    {debug f90 quiet}]} {
+    return -1
+}
+
+if {![fortran_runto_main]} {
+    untested "could not run to main"
+    return -1
+}
+
+# Depending on the compiler being used, the type names can be printed
+# differently.
+set logical [fortran_logical4]
+set real [fortran_real4]
+set int [fortran_int4]
+set complex [fortran_complex4]
+
+gdb_breakpoint [gdb_get_line_number "Before pointer assignment"]
+gdb_continue_to_breakpoint "Before pointer assignment"
+gdb_test "print logp" "= \\(PTR TO -> \\( $logical \\)\\) 0x0" \
+    "print logp, not associated"
+gdb_test "print *logp" "Cannot access memory at address 0x0" \
+    "print *logp, not associated"
+gdb_test "print comp" "= \\(PTR TO -> \\( $complex \\)\\) 0x0" \
+    "print comp, not associated"
+gdb_test "print *comp" "Cannot access memory at address 0x0" \
+    "print *comp, not associated"
+gdb_test "print charp" "= \\(PTR TO -> \\( character\\*1 \\)\\) 0x0" \
+    "print charp, not associated"
+gdb_test "print *charp" "Cannot access memory at address 0x0" \
+    "print *charp, not associated"
+gdb_test "print charap" "= \\(PTR TO -> \\( character\\*3 \\)\\) 0x0" \
+    "print charap, not associated"
+gdb_test "print *charap" "Cannot access memory at address 0x0" \
+    "print *charap, not associated"
+gdb_test "print intp" "= \\(PTR TO -> \\( $int \\)\\) 0x0" \
+    "print intp, not associated"
+gdb_test "print *intp" "Cannot access memory at address 0x0" \
+    "print *intp, not associated"
+gdb_test "print intap" " = <not associated>" "print intap, not associated"
+gdb_test "print realp" "= \\(PTR TO -> \\( $real \\)\\) 0x0" \
+    "print realp, not associated"
+gdb_test "print *realp" "Cannot access memory at address 0x0" \
+    "print *realp, not associated"
+gdb_test "print \$my_var = intp" "= \\(PTR TO -> \\( $int \\)\\) 0x0"
+gdb_test "print cyclicp1" "= \\( i = -?\\d+, p = 0x0 \\)" \
+    "print cyclicp1, not associated"
+gdb_test "print cyclicp1%p" \
+    "= \\(PTR TO -> \\( Type typewithpointer \\)\\) 0x0" \
+    "print cyclicp1%p, not associated"
+
+gdb_breakpoint [gdb_get_line_number "Before value assignment"]
+gdb_continue_to_breakpoint "Before value assignment"
+gdb_test "print *(twop)%ivla2" "= <not allocated>"
+
+gdb_breakpoint [gdb_get_line_number "After value assignment"]
+gdb_continue_to_breakpoint "After value assignment"
+gdb_test "print logp" "= \\(PTR TO -> \\( $logical \\)\\) $hex\( <.*>\)?"
+gdb_test "print *logp" "= \\.TRUE\\."
+gdb_test "print comp" "= \\(PTR TO -> \\( $complex \\)\\) $hex\( <.*>\)?"
+gdb_test "print *comp" "= \\(1,2\\)"
+gdb_test "print charp" "= \\(PTR TO -> \\( character\\*1 \\)\\) $hex\( <.*>\)?"
+gdb_test "print *charp" "= 'a'"
+gdb_test "print charap" "= \\(PTR TO -> \\( character\\*3 \\)\\) $hex\( <.*>\)?"
+gdb_test "print *charap" "= 'abc'"
+gdb_test "print intp" "= \\(PTR TO -> \\( $int \\)\\) $hex\( <.*>\)?"
+gdb_test "print *intp" "= 10"
+gdb_test "print intap" "= \\(\\(1, 1, 3(, 1){7}\\) \\(1(, 1){9}\\)\\)" \
+    "print intap, associated"
+gdb_test "print intvlap" "= \\(2, 2, 2, 4(, 2){6}\\)" \
+    "print intvlap, associated"
+gdb_test "print realp" "= \\(PTR TO -> \\( $real \\)\\) $hex\( <.*>\)?"
+gdb_test "print *realp" "= 3\\.14000\\d+"
+gdb_test "print arrayOfPtr(2)%p" "= \\(PTR TO -> \\( Type two \\)\\) $hex\( <.*>\)?"
+gdb_test "print *(arrayOfPtr(2)%p)" \
+    "= \\( ivla1 = \\(11, 12, 13\\), ivla2 = \\(\\(211, 221\\) \\(212, 222\\)\\) \\)"
+gdb_test "print arrayOfPtr(3)%p" "= \\(PTR TO -> \\( Type two \\)\\) 0x0" \
+    "print arrayOfPtr(3)%p"
+
+gdb_test_multiple "print *(arrayOfPtr(3)%p)" \
+    "print *(arrayOfPtr(3)%p), associated" {
+    # gfortran
+    -re -wrap "Cannot access memory at address 0x0" {
+	pass $gdb_test_name
+    }
+    # ifx
+    -re -wrap "Location address is not set." {
+	pass $gdb_test_name
+    }
+}
+
+gdb_test "print cyclicp1" "= \\( i = 1, p = $hex\( <.*>\)? \\)"
+gdb_test "print cyclicp1%p" "= \\(PTR TO -> \\( Type typewithpointer \\)\\) $hex\( <.*>\)?"
+gdb_test "print *((integer*) &inta + 2)" "= 3" "print temporary pointer, array"
+gdb_test "print *((integer*) &intvla + 3)" "= 4" "print temporary pointer, allocated vla"
+gdb_test "print \$pc" "\\(PTR TO -> \\( void \\(\\) \\(\\) \\)\\) $hex <pointers\\+\\d+>" \
+    "Print program counter"
diff --git a/gdb/testsuite/gdb.fortran/pointers.f90 b/gdb/testsuite/gdb.fortran/pointers.f90
index 8708d505ada..0fb6b36a25c 100644
--- a/gdb/testsuite/gdb.fortran/pointers.f90
+++ b/gdb/testsuite/gdb.fortran/pointers.f90
@@ -20,14 +20,26 @@ program pointers
     integer, allocatable :: ivla2 (:, :)
   end type two
 
+  type :: typeWithPointer
+    integer i
+    type(typeWithPointer), pointer:: p
+  end type typeWithPointer
+
+  type :: twoPtr
+    type (two), pointer :: p
+  end type twoPtr
+
   logical, target :: logv
   complex, target :: comv
   character, target :: charv
   character (len=3), target :: chara
   integer, target :: intv
   integer, target, dimension (10,2) :: inta
+  integer, target, allocatable, dimension (:) :: intvla
   real, target :: realv
   type(two), target :: twov
+  type(twoPtr) :: arrayOfPtr (3)
+  type(typeWithPointer), target:: cyclicp1,cyclicp2
 
   logical, pointer :: logp
   complex, pointer :: comp
@@ -35,6 +47,7 @@ program pointers
   character (len=3), pointer :: charap
   integer, pointer :: intp
   integer, pointer, dimension (:,:) :: intap
+  integer, pointer, dimension (:) :: intvlap
   real, pointer :: realp
   type(two), pointer :: twop
 
@@ -44,8 +57,14 @@ program pointers
   nullify (charap)
   nullify (intp)
   nullify (intap)
+  nullify (intvlap)
   nullify (realp)
   nullify (twop)
+  nullify (arrayOfPtr(1)%p)
+  nullify (arrayOfPtr(2)%p)
+  nullify (arrayOfPtr(3)%p)
+  nullify (cyclicp1%p)
+  nullify (cyclicp2%p)
 
   logp => logv    ! Before pointer assignment
   comp => comv
@@ -53,8 +72,14 @@ program pointers
   charap => chara
   intp => intv
   intap => inta
+  intvlap => intvla
   realp => realv
   twop => twov
+  arrayOfPtr(2)%p => twov
+  cyclicp1%i = 1
+  cyclicp1%p => cyclicp2
+  cyclicp2%i = 2
+  cyclicp2%p => cyclicp1
 
   logv = associated(logp)     ! Before value assignment
   comv = cmplx(1,2)
@@ -63,6 +88,10 @@ program pointers
   intv = 10
   inta(:,:) = 1
   inta(3,1) = 3
+  allocate (intvla(10))
+  intvla(:) = 2
+  intvla(4) = 4
+  intvlap => intvla
   realv = 3.14
 
   allocate (twov%ivla1(3))
diff --git a/gdb/valprint.c b/gdb/valprint.c
index b65dda15c04..c71ae089f46 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -1156,12 +1156,6 @@ value_check_printable (struct value *val, struct ui_file *stream,
       return 0;
     }
 
-  if (type_not_associated (val->type ()))
-    {
-      val_print_not_associated (stream);
-      return 0;
-    }
-
   if (type_not_allocated (val->type ()))
     {
       val_print_not_allocated (stream);
-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 3/4] gdb, intel-classic-compilers, testsuite: workaround icc/icpc/ifort pointer/reference DWARF
  2023-09-04 22:29 [PATCH v3 0/4] Dynamic properties of pointers Abdul Basit Ijaz
  2023-09-04 22:29 ` [PATCH v3 1/4] gdb, testsuite: handle icc and icpc deprecated remarks Abdul Basit Ijaz
  2023-09-04 22:29 ` [PATCH v3 2/4] gdb, types: Resolve pointer types dynamically Abdul Basit Ijaz
@ 2023-09-04 22:29 ` Abdul Basit Ijaz
  2023-10-03  0:09   ` Thiago Jung Bauermann
  2023-10-10 19:52   ` Tom Tromey
  2023-09-04 22:29 ` [PATCH v3 4/4] gdb, testsuite, fortran: Fix sizeof intrinsic for ifort Fortran pointers Abdul Basit Ijaz
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 18+ messages in thread
From: Abdul Basit Ijaz @ 2023-09-04 22:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: abdul.b.ijaz, simark, tom

From: "Ijaz, Abdul B" <abdul.b.ijaz@intel.com>

Intel classic compilers (icc/icpc/ifort) for references/pointers
to arrays generate DWARF that looks like

 <2><17d>: Abbrev Number: 22 (DW_TAG_variable)
    <17e>   DW_AT_decl_line   : 41
    <17f>   DW_AT_decl_file   : 1
    <180>   DW_AT_name        : (indirect string, offset: 0x1f1): vlaref
    <184>   DW_AT_type        : <0x214>
    <188>   DW_AT_location    : 2 byte block: 76 50
      (DW_OP_breg6 (rbp): -48)
 ...
 <1><214>: Abbrev Number: 12 (DW_TAG_reference/pointer_type)
    <215>   DW_AT_type        : <0x219>
    <216>   DW_AT_associated  : ...     <- for Fortran pointers
 <1><219>: Abbrev Number: 27 (DW_TAG_array_type)
    <21a>   DW_AT_type        : <0x10e>
    <21e>   DW_AT_data_location: 2 byte block: 97 6
      (DW_OP_push_object_address; DW_OP_deref)
 <2><221>: Abbrev Number: 28 (DW_TAG_subrange_type)
    <222>   DW_AT_upper_bound : <0x154>
 <2><226>: Abbrev Number: 0

This is, to my knowledge, somewhat allowed and correct DWARF, however,
there are 2 issues with the emitted DWARF.

First, the DW_AT_associated that is emitted (only for Fortran pointers) is
not supposed to be emitted with pointer types.  Rather, the tag is expected
on types that have the 'pointer property'.  In Fortran a pointer is more
than just an address - it is a fully selfcontained type that can be
associated with any object of the same type that has the target/pointer
property.
As such, it will have fields for, e.g., the rank of its underlying array.
The pointer property is normally implicitly modelled in DWARF by
emitting, e.g., a DW_TAG_array_type and giving it a DW_AT_associated
property.  This automatically makes it a Fortran pointer-to-array type
and is also the way gfortran/ifx model their Fortran pointers in DWARF.
Intel classic compilers deviated from this way of modeling Fortran
pointers.

Second, the above DWARF assumes that the address needed to resolve the
DW_OP_push_object_address is the address of the original variable, not
the address of the array itself.  This seems incorrect as when resolving
the array, the address of the object currently being evaluated is the
one of the array, not the one of the pointer.  This lets GDB fail when
trying to resolve the arrays underlying the above pointer/reference
construct, as GDB assumes that the address of the array is needed
instead.

While this DWARF is wrong, icc/icpc/ifort will likely not change their
DWARF anymore as they are slowly being EOLed.  Additionally, any older
versions of the compilers will anyway not work with GDB.  This patch
implements a workaround that makes GDB work with the Intel classic
compiler's DWARF.  It adds workarounds guarded by compiler checks.
Whenever resolving a dynamic type that is a pointer/reference we check
whether the type's producer has been an Intel classic compiler (by checking
the types objfile and all producers in this objfile) and, if this is the
case, we take the presence of the DW_AT_data_location in the
pointer's/reference's target_type () as an indication that we need to use
the pointer's address rather than its target address to resolve the
target_type ().

Additionally, we resolve the DW_AT_associated property on pointers when
their producer is an Intel classic compiler.

Without the above patch GDB would usually display

  // line 51
  (gdb) print vlaref
  $1 = (int (&)[3]) <error reading variable>

(in rare cases the memory address might even be valid and GDB would
print random output for the array) for references using the above
construct (the example is taken from vla-cxx.exp, which is failing for
ifort without this patch).  For Fortran pointers one would run into a
similar problem (from pointers.exp)

  // line 107
  (gdb) p intap
  $1 = (PTR TO -> ( INTEGER(4) (:,:) )) 0x4866e0 <pointers_$INTA>
  (gdb) p *intap
  value requires 8589934593 bytes, which is more than max-value-size

With this patch the above examples print as

  // line 51
  (gdb) print vlaref
  $1 = (int (&)[3]) @0x7fffffffc4e0: {5, 7, 9}

and

  // line 107
  (gdb) p intap
  $1 = (PTR TO -> ( INTEGER(4) (10,2) )) 0x4866e0 <pointers_$INTA>
  (gdb) p *intap
  $2 = ((1, 1, 3, 1, 1, 1, 1, 1, 1, 1) (1, 1, 1, 1, 1, 1, 1, 1, 1, 1))

greatly increasing usability of icc/icpc/ifort emitted objectfiles
inside GDB.

A test has been added to gdb.dwarf2 explicitly constructing both, the
wrong pointer and the wrong reference DWARF.
---
 gdb/gdbtypes.c                                |  94 +++++++++-
 gdb/gdbtypes.h                                |   5 +
 .../icc-ifort-pointers-and-references.c       |  38 ++++
 .../icc-ifort-pointers-and-references.exp     | 169 ++++++++++++++++++
 gdb/testsuite/gdb.fortran/pointers.exp        |  86 +++++++--
 gdb/valprint.c                                |  34 ++++
 6 files changed, 409 insertions(+), 17 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.exp

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 4b1787b62e6..c2fabbcb2c2 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -43,6 +43,7 @@
 #include "f-lang.h"
 #include <algorithm>
 #include "gmp-utils.h"
+#include "producer.h"
 
 /* The value of an invalid conversion badness.  */
 #define INVALID_CONVERSION 100
@@ -2043,6 +2044,14 @@ is_dynamic_type_internal (struct type *type, int top_level)
   if (top_level
       && (type->code () == TYPE_CODE_REF || type->code () == TYPE_CODE_PTR))
     type = check_typedef (type->target_type ());
+  if (!top_level && icc_pointer_or_reference_type (type))
+    {
+      /* Icc/ifort emit the DW_AT_associated for pointers and references.  To
+	 not mark such types as dynamic further down, which would lead to
+	 infinite resolution loops for, e.g., cyclic dynamic pointers, we
+	 return here already.  */
+      return 0;
+    }
 
   /* Types that have a dynamic TYPE_DATA_LOCATION are considered
      dynamic, even if the type itself is statically defined.
@@ -2740,6 +2749,79 @@ resolve_dynamic_struct (struct type *type,
   return resolved_type;
 }
 
+/* See gdbtypes.h.  */
+
+bool
+icc_pointer_or_reference_type (const struct type *type)
+{
+  return (type->code () == TYPE_CODE_PTR || type->code () == TYPE_CODE_REF)
+	 && type->is_objfile_owned ()
+	 && std::any_of (type->objfile_owner ()->compunits ().begin (),
+			 type->objfile_owner ()->compunits ().end (),
+			 [] (const compunit_symtab *cu)
+			 {
+			   return producer_is_icc (cu->producer (), nullptr,
+						   nullptr);
+			 });
+}
+
+/* Resolve the special icc/icpc/ifort DWARF for pointers and references
+   and update resolved_type accordingly.  */
+
+static void
+resolve_dynamic_icc_pointer_or_reference (type *orig_type, type* resolved_type,
+					  property_addr_info *addr_stack,
+					  property_addr_info *pinfo,
+					  const frame_info_ptr &frame,
+					  int top_level)
+{
+  if (TYPE_DATA_LOCATION (orig_type->target_type ()) != nullptr)
+    {
+      /* Icc/ifort emit some wrong DWARF for pointers and references
+	 with underlying arrays.  They emit DWARF like
+	 <2><11>: Abbrev Number: 22 (DW_TAG_variable)
+	    <12>   DW_AT_name      : ...
+	    <13>   DW_AT_type      : <0x214>
+	    <14>   DW_AT_location  : ...
+	 ...
+	 <1><111>: Abbrev Number: 12 (DW_TAG_reference_type)
+	    <112>   DW_AT_type     : <0x219>
+	 <1><113>: Abbrev Number: 27 (DW_TAG_array_type)
+	    <114>   DW_AT_type     : <0x10e>
+	    <115>   DW_AT_data_location: 2 byte block: 97 6
+	(DW_OP_push_object_address; DW_OP_deref)
+	 <2><116>: Abbrev Number: 28 (DW_TAG_subrange_type)
+	    <117>   DW_AT_upper_bound : <0x154>
+	 <2><118>: Abbrev Number: 0
+
+	 For icc/ifort the DW_AT_data_location requires the address
+	 of the original DW_TAG_variable for the evaluation of
+	 DW_OP_push_object_address instead of the address of
+	 the DW_TAG_array_type typically obtained by resolving
+	 dereferencing the DW_TAG_reference_type/DW_TAG_pointer_type
+	 once.  If icc/ifort are detected as producers here and if
+	 the type underlying the current pointer/reference variable
+	 has a DW_AT_data_location, we thus pass the address of
+	 the variable to resolve the target type instead of the
+	 dereferenced address of the pointer/reference.  */
+      pinfo->addr = addr_stack->addr;
+    }
+
+    /* Another peculiarity of icc's/ifort's dwarf is the usage of
+       DW_AT_associated for pointers/references.  */
+    CORE_ADDR value;
+    dynamic_prop *prop = TYPE_ASSOCIATED_PROP (resolved_type);
+    if (prop != nullptr
+	&& dwarf2_evaluate_property (prop, nullptr, addr_stack, &value))
+    prop->set_const_val (value);
+
+    /* As we have the associated property here, we need to check it.  */
+    if (!type_not_associated (resolved_type))
+      resolved_type->set_target_type
+	(resolve_dynamic_type_internal (orig_type->target_type (), pinfo,
+					frame, top_level));
+}
+
 /* Worker for resolved_dynamic_type.  */
 
 static struct type *
@@ -2791,9 +2873,15 @@ resolve_dynamic_type_internal (struct type *type,
 	    pinfo.next = addr_stack;
 
 	    resolved_type = copy_type (type);
-	    resolved_type->set_target_type
-	      (resolve_dynamic_type_internal (type->target_type (),
-					      &pinfo, frame, top_level));
+
+	    if (icc_pointer_or_reference_type (type))
+	      resolve_dynamic_icc_pointer_or_reference (type, resolved_type,
+							addr_stack, &pinfo,
+							frame, top_level);
+	    else
+	      resolved_type->set_target_type
+		(resolve_dynamic_type_internal (type->target_type (),
+						&pinfo, frame, top_level));
 	    break;
 	  }
 
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index f45a957f344..fd149f2ce8e 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -2801,4 +2801,9 @@ extern unsigned int overload_debug;
 
 extern bool is_nocall_function (const struct type *type);
 
+/* Check whether icc/ifort could have been the producers of the TYPE_CODE_REF
+   or TYPE_CODE_PTR type.  */
+
+extern bool icc_pointer_or_reference_type (const struct type *type);
+
 #endif /* GDBTYPES_H */
diff --git a/gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.c b/gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.c
new file mode 100644
index 00000000000..0c0be751833
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.c
@@ -0,0 +1,38 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2021-2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+struct fat_pointer
+{
+  int *data;
+  int *associated;
+  int *size;
+};
+
+int one[] = {1};
+int zero1[] = {0};
+int zero2[] = {0};
+int four[] = {4};
+int data[] = {11, 22, 33, 44};
+
+struct fat_pointer fp_associated = {data, one, four};
+struct fat_pointer fp_not_associated = {0, zero1, zero2};
+
+int
+main ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.exp b/gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.exp
new file mode 100644
index 00000000000..278124bbdde
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.exp
@@ -0,0 +1,169 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This test checks that GDB can handle some slightly wrong DWARF that is being
+# produces by icc/icpc/ifort for pointers and references.  Namely the DWARF
+# looks like
+#
+# <2><17d>: Abbrev Number: 22 (DW_TAG_variable)
+#    <17e>   DW_AT_decl_line : 41
+#    <17f>   DW_AT_decl_file : 1
+#    <180>   DW_AT_name      : (indirect string, offset: 0x1f1): vlaref
+#    <184>   DW_AT_type      : <0x214>
+#    <188>   DW_AT_location  : 2 byte block: 76 50
+#      (DW_OP_breg6 (rbp): -48)
+# ...
+# <1><214>: Abbrev Number: 12 (DW_TAG_reference/pointer_type)
+#    <215>   DW_AT_type      : <0x219>
+#    <216>   DW_AT_associated: ...     <- for Fortran pointers
+# <1><219>: Abbrev Number: 27 (DW_TAG_array_type)
+#    <21a>   DW_AT_type      : <0x10e>
+#    <21e>   DW_AT_data_location: 2 byte block: 97 6
+#      (DW_OP_push_object_address; DW_OP_deref)
+# <2><221>: Abbrev Number: 28 (DW_TAG_subrange_type)
+#    <222>   DW_AT_upper_bound : <0x154>
+# <2><226>: Abbrev Number: 0
+#
+# With a) DW_OP_push_object_address expecting the address of the
+# DW_TAG_variable used for its resolution instead of the address of the
+# underlying array and b) some Fortran pointers exhibiting the DW_AT_associated
+# attribute on DW_TAG_pointer_types.
+# To test a) this test constructs a pointer and a reference type to an array
+# with the above usage of DW_AT_data_location and DW_OP_push_object_address.
+# To test b) we simply create a pointer with the DW_AT_associated attribute
+# and check whether this is being resolved or not.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+standard_testfile .c -dw.S
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+    return -1
+}
+
+# Make some DWARF for the test.
+set asm_file [standard_output_file $srcfile2]
+
+Dwarf::assemble $asm_file {
+    global srcfile
+    set int_size [get_sizeof "int" 4]
+    set voidp_size [get_sizeof "void *" 96]
+    declare_labels integer_label array_label pointer_label reference_label
+
+    cu {} {
+	compile_unit {
+	    {DW_AT_language @DW_LANG_Fortran90}
+	    {DW_AT_name $srcfile}
+	    {DW_AT_producer "Intel(R) compiler VERSION 123.456"}
+	    {DW_AT_comp_dir /tmp}
+	} {
+	    integer_label: DW_TAG_base_type {
+		{name "int"}
+		{byte_size $int_size sdata}
+		{encoding @DW_ATE_signed}
+	    }
+
+	    array_label: DW_TAG_array_type {
+		{DW_AT_type :$integer_label}
+		{DW_AT_data_location {
+		    DW_OP_push_object_address
+		    DW_OP_deref
+		} SPECIAL_expr}
+	    } {
+		DW_TAG_subrange_type {
+		    {DW_AT_type :$integer_label}
+		    {DW_AT_upper_bound {
+			DW_OP_push_object_address
+			DW_OP_plus_uconst $voidp_size
+			DW_OP_plus_uconst $voidp_size
+			DW_OP_deref
+			DW_OP_deref_size $int_size
+		     } SPECIAL_expr }
+		}
+	    }
+
+	    pointer_label: DW_TAG_pointer_type {
+		{DW_AT_type :$array_label}
+		{DW_AT_associated {
+		    DW_OP_push_object_address
+		    DW_OP_plus_uconst $voidp_size
+		    DW_OP_deref
+		    DW_OP_deref_size $int_size
+		    DW_OP_constu 0
+		    DW_OP_ne
+		} SPECIAL_expr }
+	    }
+
+	    reference_label: DW_TAG_reference_type {
+		{DW_AT_type :$array_label}
+	    }
+
+	    DW_TAG_variable {
+		{DW_AT_name "fp_associated"}
+		{DW_AT_type :$pointer_label}
+		{DW_AT_location {
+		    DW_OP_addr [gdb_target_symbol fp_associated]
+		} SPECIAL_expr}
+	    }
+
+	    DW_TAG_variable {
+		{DW_AT_name "fp_not_associated"}
+		{DW_AT_type :$pointer_label}
+		{DW_AT_location {
+		    DW_OP_addr [gdb_target_symbol fp_not_associated]
+		} SPECIAL_expr}
+	    }
+
+	    DW_TAG_variable {
+		{DW_AT_name "array_ref"}
+		{DW_AT_type :$reference_label}
+		{DW_AT_location {
+		    DW_OP_addr [gdb_target_symbol fp_associated]
+		} SPECIAL_expr}
+	    }
+	}
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+# Test whether GDB can handle ifort's DWARF for Fortran pointers.
+gdb_test_no_output "set language fortran"
+gdb_test "p associated(fp_associated)" "\\.TRUE\\."
+gdb_test "p associated(fp_not_associated)" "\\.FALSE\\."
+gdb_test "p fp_not_associated" \
+    " = \\(PTR TO -> \\( int \\(:\\) \\)\\) <not associated>"
+gdb_test "p *fp_not_associated" "Location address is not set\."
+
+gdb_test "p fp_associated" "= \\(PTR TO -> \\( int \\(4\\) \\)\\) $hex <.*>"
+gdb_test "p *fp_associated" "= \\(11, 22, 33, 44\\)"
+
+# Test whether GDB can handle icc's DWARF for c++ references.
+gdb_test_no_output "set language c++"
+
+gdb_test "print array_ref" \
+    " = \\(int \\(&\\)\\\[4\\\]\\) @$hex: \\{11, 22, 33, 44\\}"
diff --git a/gdb/testsuite/gdb.fortran/pointers.exp b/gdb/testsuite/gdb.fortran/pointers.exp
index ca2195bbfe6..e340c78de6d 100644
--- a/gdb/testsuite/gdb.fortran/pointers.exp
+++ b/gdb/testsuite/gdb.fortran/pointers.exp
@@ -55,17 +55,45 @@ gdb_test "print intp" "= \\(PTR TO -> \\( $int \\)\\) 0x0" \
     "print intp, not associated"
 gdb_test "print *intp" "Cannot access memory at address 0x0" \
     "print *intp, not associated"
-gdb_test "print intap" " = <not associated>" "print intap, not associated"
+
+gdb_test_multiple "print intap" "print intap, not associated" {
+    # gfortran/ifx.
+    -re -wrap " = <not associated>" {
+	pass $gdb_test_name
+    }
+    # ifort.
+    -re -wrap " = \\(PTR TO -> \\( $int \\(:,:\\) \\)\\) <not associated>" {
+	pass $gdb_test_name
+    }
+}
+
 gdb_test "print realp" "= \\(PTR TO -> \\( $real \\)\\) 0x0" \
     "print realp, not associated"
 gdb_test "print *realp" "Cannot access memory at address 0x0" \
     "print *realp, not associated"
 gdb_test "print \$my_var = intp" "= \\(PTR TO -> \\( $int \\)\\) 0x0"
-gdb_test "print cyclicp1" "= \\( i = -?\\d+, p = 0x0 \\)" \
-    "print cyclicp1, not associated"
-gdb_test "print cyclicp1%p" \
-    "= \\(PTR TO -> \\( Type typewithpointer \\)\\) 0x0" \
-    "print cyclicp1%p, not associated"
+
+gdb_test_multiple "print cyclicp1" "print cyclicp1, not associated" {
+    # gfortran/ifx.
+    -re -wrap "= \\( i = -?\\d+, p = 0x0 \\)" {
+	pass $gdb_test_name
+    }
+    # ifort.
+    -re -wrap "= \\( i = -?\\d+, p = <not associated> \\)" {
+	pass $gdb_test_name
+    }
+}
+
+gdb_test_multiple "print cyclicp1%p" "print cyclicp1%p, not associated" {
+    # gfortran/ifx.
+    -re -wrap "= \\(PTR TO -> \\( Type typewithpointer \\)\\) 0x0" {
+	pass $gdb_test_name
+    }
+    # ifort.
+    -re -wrap "= \\(PTR TO -> \\( Type typewithpointer \\)\\) <not associated>" {
+	pass $gdb_test_name
+    }
+}
 
 gdb_breakpoint [gdb_get_line_number "Before value assignment"]
 gdb_continue_to_breakpoint "Before value assignment"
@@ -83,25 +111,55 @@ gdb_test "print charap" "= \\(PTR TO -> \\( character\\*3 \\)\\) $hex\( <.*>\)?"
 gdb_test "print *charap" "= 'abc'"
 gdb_test "print intp" "= \\(PTR TO -> \\( $int \\)\\) $hex\( <.*>\)?"
 gdb_test "print *intp" "= 10"
-gdb_test "print intap" "= \\(\\(1, 1, 3(, 1){7}\\) \\(1(, 1){9}\\)\\)" \
-    "print intap, associated"
-gdb_test "print intvlap" "= \\(2, 2, 2, 4(, 2){6}\\)" \
-    "print intvlap, associated"
+
+gdb_test_multiple "print intap" "print intap, associated" {
+    # gfortran/ifx.
+    -re -wrap "= \\(\\(1, 1, 3(, 1){7}\\) \\(1(, 1){9}\\)\\)" {
+	pass $gdb_test_name
+    }
+    # ifort.
+    -re -wrap "= \\(PTR TO -> \\( $int \\(10,2\\) \\)\\) $hex\( <.*>\)?" {
+	gdb_test "print *intap" "= \\(\\(1, 1, 3(, 1){7}\\) \\(1(, 1){9}\\)\\)"
+	pass $gdb_test_name
+    }
+}
+
+gdb_test_multiple "print intvlap" "print intvlap, associated" {
+    # gfortran/ifx.
+    -re -wrap "= \\(2, 2, 2, 4(, 2){6}\\)" {
+	pass $gdb_test_name
+    }
+    # ifort.
+    -re -wrap "= \\(PTR TO -> \\( $int \\(10\\) \\)\\) $hex\( <.*>\)?" {
+	gdb_test "print *intvlap" "= \\(2, 2, 2, 4(, 2){6}\\)"
+	pass $gdb_test_name
+    }
+}
+
 gdb_test "print realp" "= \\(PTR TO -> \\( $real \\)\\) $hex\( <.*>\)?"
 gdb_test "print *realp" "= 3\\.14000\\d+"
 gdb_test "print arrayOfPtr(2)%p" "= \\(PTR TO -> \\( Type two \\)\\) $hex\( <.*>\)?"
 gdb_test "print *(arrayOfPtr(2)%p)" \
     "= \\( ivla1 = \\(11, 12, 13\\), ivla2 = \\(\\(211, 221\\) \\(212, 222\\)\\) \\)"
-gdb_test "print arrayOfPtr(3)%p" "= \\(PTR TO -> \\( Type two \\)\\) 0x0" \
-    "print arrayOfPtr(3)%p"
+
+gdb_test_multiple "print arrayOfPtr(3)%p" "print arrayOfPtr(3)%p" {
+    # gfortran/ifx
+    -re -wrap "= \\(PTR TO -> \\( Type two \\)\\) 0x0" {
+	pass $gdb_test_name
+    }
+    # ifort
+     -re -wrap "= \\(PTR TO -> \\( Type two \\)\\) <not associated>" {
+	pass $gdb_test_name
+    }
+}
 
 gdb_test_multiple "print *(arrayOfPtr(3)%p)" \
     "print *(arrayOfPtr(3)%p), associated" {
-    # gfortran
+    # gfortran.
     -re -wrap "Cannot access memory at address 0x0" {
 	pass $gdb_test_name
     }
-    # ifx
+    # ifx/ifort.
     -re -wrap "Location address is not set." {
 	pass $gdb_test_name
     }
diff --git a/gdb/valprint.c b/gdb/valprint.c
index c71ae089f46..ccd3c435ac9 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -583,6 +583,40 @@ generic_val_print_ref (struct type *type,
 	  /* More complicated computed references are not supported.  */
 	  gdb_assert (embedded_offset == 0);
 	}
+      else if (icc_pointer_or_reference_type (type)
+	       && TYPE_DATA_LOCATION (type->target_type ()) != nullptr)
+	{
+	  /* Icc/ifort emit some wrong DWARF for pointers and references
+	     with underlying arrays.  They emit DWARF like
+
+	     <2><11>: Abbrev Number: 22 (DW_TAG_variable)
+		<12>   DW_AT_name      : ...
+		<13>   DW_AT_type      : <0x214>
+		<14>   DW_AT_location  : ...
+	     ...
+	     <1><111>: Abbrev Number: 12 (DW_TAG_reference_type)
+		<112>   DW_AT_type     : <0x219>
+	     <1><113>: Abbrev Number: 27 (DW_TAG_array_type)
+		<114>   DW_AT_type     : <0x10e>
+		<115>   DW_AT_data_location: 2 byte block: 97 6
+		(DW_OP_push_object_address; DW_OP_deref)
+	     <2><116>: Abbrev Number: 28 (DW_TAG_subrange_type)
+		<117>   DW_AT_upper_bound : <0x154>
+	     <2><118>: Abbrev Number: 0
+
+	     For icc/ifort the DW_AT_data_location requires the address
+	     of the original DW_TAG_variable for the evaluation of
+	     DW_OP_push_object_address instead of the address of
+	     the DW_TAG_array_type typically obtained by resolving
+	     dereferencing the DW_TAG_reference_type/DW_TAG_pointer_type
+	     once.  If icc/ifort are detected as producers here and if
+	     the type underlying the current pointer/reference variable
+	     has a DW_AT_data_location, we thus pass the address of
+	     the variable to resolve the target type instead of the
+	     dereferenced address of the pointer/reference.  */
+	  deref_val = value_at (type->target_type (),
+				original_value->address ());
+	}
       else
 	deref_val = value_at (type->target_type (),
 			      unpack_pointer (type, valaddr + embedded_offset));
-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 4/4] gdb, testsuite, fortran: Fix sizeof intrinsic for ifort Fortran pointers
  2023-09-04 22:29 [PATCH v3 0/4] Dynamic properties of pointers Abdul Basit Ijaz
                   ` (2 preceding siblings ...)
  2023-09-04 22:29 ` [PATCH v3 3/4] gdb, intel-classic-compilers, testsuite: workaround icc/icpc/ifort pointer/reference DWARF Abdul Basit Ijaz
@ 2023-09-04 22:29 ` Abdul Basit Ijaz
  2023-10-03  0:16   ` Thiago Jung Bauermann
  2023-09-27 21:11 ` [PING][PATCH v3 0/4] Dynamic properties of pointers Ijaz, Abdul B
  2023-10-03  0:17 ` [PATCH " Thiago Jung Bauermann
  5 siblings, 1 reply; 18+ messages in thread
From: Abdul Basit Ijaz @ 2023-09-04 22:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: abdul.b.ijaz, simark, tom

From: "Ijaz, Abdul B" <abdul.b.ijaz@intel.com>

For Fortran pointers ifort emits actual DW_TAG_pointer_types like

<2><17d>: Abbrev Number: 22 (DW_TAG_variable)
   <180>   DW_AT_name        : (indirect string, offset: 0x1f1): fptr
   <184>   DW_AT_type        : <0x214>
...
<1><214>: Abbrev Number: 12 (DW_TAG_pointer_type)
   <215>   DW_AT_type        : <0x219>
   <216>   DW_AT_associated  : ...
<1><219>: Abbrev Number: 27 (DW_TAG_array_type)
   <21a>   DW_AT_type        : <0x10e>
...

whereas gfortran/ifx emit DWARF like

<2><17d>: Abbrev Number: 22 (DW_TAG_variable)
   <180>   DW_AT_name        : (indirect string, offset: 0x1f1): fptr
   <184>   DW_AT_type        : <0x214>
...
<1><219>: Abbrev Number: 27 (DW_TAG_array_type)
   <21a>   DW_AT_type        : <0x10e>
   <216>   DW_AT_associated  : ...

The 'pointer property' in Fortran is implicitly modeled by adding a
DW_AT_associated to the type of the variable (see also the
DW_AT_associated description in DWARF 5).  A Fortran pointer is more
than an address and thus different from a C pointer.  It is a
selfcontained type having additional fields such as, e.g., the rank of
its underlying array.  This motivates the intended DWARF modeling of
Fortran pointers (like gfortran and ifx do it) via the DW_AT_associated
attribute.

As ifort will not change its DWARF anymore and we still want to support
its DWARF we adapt GDB in the case of ifort Fortran pointers a bit.

This patch adds support for the sizeof intrinsic, which can now also be
applied to ifort pointer types by simply dereferencing them when
encountered during a sizeof evaluation.  Before, the application of sizeof
was only possible for gfortran's/ifx' Fortran pointers.

The patch also adds a test for the sizeof intrinsic which was not tested
before.
---
 gdb/eval.c                           |   9 +++
 gdb/testsuite/gdb.fortran/sizeof.exp | 115 +++++++++++++++++++++++++++
 gdb/testsuite/gdb.fortran/sizeof.f90 | 108 +++++++++++++++++++++++++
 3 files changed, 232 insertions(+)
 create mode 100644 gdb/testsuite/gdb.fortran/sizeof.exp
 create mode 100644 gdb/testsuite/gdb.fortran/sizeof.f90

diff --git a/gdb/eval.c b/gdb/eval.c
index 794698f85bd..65408c2358b 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -2705,6 +2705,15 @@ evaluate_subexp_for_sizeof_base (struct expression *exp, struct type *type)
   if (exp->language_defn->la_language == language_cplus
       && (TYPE_IS_REFERENCE (type)))
     type = check_typedef (type->target_type ());
+  else if (exp->language_defn->la_language == language_fortran
+	   && type->code () == TYPE_CODE_PTR)
+    {
+      /* IFORT emits DW_TAG_pointer_type for Fortran pointers.  While this is
+	 not the intended DWARF way of describing pointer types, we still
+	 support it here.  There is no harm in dereferencing such pointer types
+	 and allowing them for the Fortran sizeof intrinsic.  */
+      type = check_typedef (type->target_type ());
+    }
   return value_from_longest (size_type, (LONGEST) type->length ());
 }
 
diff --git a/gdb/testsuite/gdb.fortran/sizeof.exp b/gdb/testsuite/gdb.fortran/sizeof.exp
new file mode 100644
index 00000000000..5e3710373ea
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/sizeof.exp
@@ -0,0 +1,115 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/> .
+
+# Testing GDB's implementation of SIZE keyword.
+
+require allow_fortran_tests
+
+standard_testfile ".f90"
+load_lib fortran.exp
+
+if {[prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} \
+	 {debug f90}]} {
+    return -1
+}
+
+if ![fortran_runto_main] {
+    return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "Test breakpoint"]
+gdb_breakpoint [gdb_get_line_number "Past unassigned pointers"]
+gdb_breakpoint [gdb_get_line_number "Final breakpoint"]
+
+set done_unassigned 0
+set found_final_breakpoint 0
+set test_count 0
+
+# We are running tests defined in the executable here.  So, in the .exp file
+# we do not know when the 'Final breakpoint' will be hit exactly.  We place a
+# limit on the number of tests that can be run, just in case something goes
+# wrong, and GDB gets stuck in an loop here.
+while { $test_count < 200 } {
+    with_test_prefix "test $test_count" {
+	incr test_count
+
+	gdb_test_multiple "continue" "continue" {
+	    -re -wrap "! Test breakpoint" {
+		# We can run a test from here.
+	    }
+	    -re -wrap "! Past unassigned pointers" {
+		# Done with testing unassigned pointers.
+		set done_unassigned 1
+		continue
+	    }
+	    -re -wrap "! Final breakpoint" {
+		# We're done with the tests.
+		set found_final_breakpoint 1
+	    }
+	}
+
+	if ($found_final_breakpoint) {
+	    break
+	}
+
+	# First grab the expected answer.
+	set answer [get_valueof "" "answer" "**unknown**"]
+
+	# Now move up a frame and figure out a command for us to run
+	# as a test.
+	set command ""
+	gdb_test_multiple "up" "up" {
+	    -re -wrap "\r\n\[0-9\]+\[ \t\]+call test_sizeof \\((\[^\r\n\]+)\\)" {
+		set command $expect_out(1,string)
+	    }
+	}
+
+	gdb_assert { ![string equal $command ""] } "found a command to run"
+
+	set is_pointer_to_array [string match "sizeof (*a_p)*" $command]
+
+	if {$done_unassigned || !$is_pointer_to_array} {
+	    gdb_test "p $command" " = $answer"
+	} else {
+	    # Gfortran, ifx and ifort have slightly different behavior for
+	    # unassigned pointers to arrays.  While ifx and ifort will print 0
+	    # as the sizeof result, gfortran will print the size of the base
+	    # type of the pointer/array.  Since the default behavior in GDB was
+	    # to print 0 we keep this and make an exception for gfortran here.
+	    gdb_test_multiple "p $command" "p $command" {
+		-re -wrap " = $answer" {
+		    pass $gdb_test_name
+		}
+		-re -wrap " = 0" {
+		    pass $gdb_test_name
+		}
+	    }
+	}
+    }
+}
+
+gdb_assert {$found_final_breakpoint} "ran all compiled in tests"
+
+# Here some more GDB specific tests that might fail with compilers.
+# GDB will print sizeof(1.4) = 8 while gfortran will probably print 4 but
+# GDB says ptype 1.4 is real*8 so the output is expected.
+
+gdb_test "ptype 1" "type = int"
+gdb_test "p sizeof(1)" "= 4"
+
+gdb_test "ptype 1.3" "type = real\\*8"
+gdb_test "p sizeof(1.3)" "= 8"
+
+gdb_test  "p sizeof ('asdsasd')" "= 7"
diff --git a/gdb/testsuite/gdb.fortran/sizeof.f90 b/gdb/testsuite/gdb.fortran/sizeof.f90
new file mode 100644
index 00000000000..5f20a4effee
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/sizeof.f90
@@ -0,0 +1,108 @@
+! Copyright 2023 Free Software Foundation, Inc.
+
+! This program is free software; you can redistribute it and/or modify
+! it under the terms of the GNU General Public License as published by
+! the Free Software Foundation; either version 3 of the License, or
+! (at your option) any later version.
+!
+! This program is distributed in the hope that it will be useful,
+! but WITHOUT ANY WARRANTY; without even the implied warranty of
+! MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+! GNU General Public License for more details.
+!
+! You should have received a copy of the GNU General Public License
+! along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+module data
+  use, intrinsic :: iso_c_binding, only : C_SIZE_T
+  implicit none
+
+  character, target :: char_v
+  character (len=3), target :: char_a
+  integer, target :: int_v
+  integer, target, dimension(:,:) :: int_2da (3,2)
+  real*4, target :: real_v
+  real*4, target :: real_a(4)
+  real*4, target, dimension (:), allocatable :: real_a_alloc
+
+  character, pointer :: char_v_p
+  character (len=3), pointer :: char_a_p
+  integer, pointer :: int_v_p
+  integer, pointer, dimension (:,:) :: int_2da_p
+  real*4, pointer :: real_v_p
+  real*4, pointer, dimension(:) :: real_a_p
+  real*4, dimension(:), pointer :: real_alloc_a_p
+
+contains
+subroutine test_sizeof (answer)
+  integer(C_SIZE_T) :: answer
+
+  print *, answer ! Test breakpoint
+end subroutine test_sizeof
+
+subroutine run_tests ()
+  call test_sizeof (sizeof (char_v))
+  call test_sizeof (sizeof (char_a))
+  call test_sizeof (sizeof (int_v))
+  call test_sizeof (sizeof (int_2da))
+  call test_sizeof (sizeof (real_v))
+  call test_sizeof (sizeof (real_a))
+  call test_sizeof (sizeof (real_a_alloc))
+
+  call test_sizeof (sizeof (char_v_p))
+  call test_sizeof (sizeof (char_a_p))
+  call test_sizeof (sizeof (int_v_p))
+  call test_sizeof (sizeof (int_2da_p))
+  call test_sizeof (sizeof (real_v_p))
+  call test_sizeof (sizeof (real_a_p))
+  call test_sizeof (sizeof (real_alloc_a_p))
+end subroutine run_tests
+
+end module data
+
+program sizeof_tests
+  use iso_c_binding
+  use data
+
+  implicit none
+
+  allocate (real_a_alloc(5))
+
+  nullify (char_v_p)
+  nullify (char_a_p)
+  nullify (int_v_p)
+  nullify (int_2da_p)
+  nullify (real_v_p)
+  nullify (real_a_p)
+  nullify (real_alloc_a_p)
+
+  ! Test nullified
+  call run_tests ()
+
+  char_v_p => char_v ! Past unassigned pointers
+  char_a_p => char_a
+  int_v_p => int_v
+  int_2da_p => int_2da
+  real_v_p => real_v
+  real_a_p => real_a
+  real_alloc_a_p => real_a_alloc
+
+  ! Test pointer assignment
+  call run_tests ()
+
+  char_v = 'a'
+  char_a = "aaa"
+  int_v = 10
+  int_2da = reshape((/1, 2, 3, 4, 5, 6/), shape(int_2da))
+  real_v = 123.123
+  real_a_p = (/-1.1, -1.2, -1.3, -1.4/)
+  real_a_alloc = (/1.1, 2.2, 3.3, 4.4, 5.5/)
+
+  ! After allocate/value assignment
+  call run_tests ()
+
+  deallocate (real_a_alloc)
+
+  print *, "done" ! Final breakpoint
+
+end program sizeof_tests
-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PING][PATCH v3 0/4] Dynamic properties of pointers
  2023-09-04 22:29 [PATCH v3 0/4] Dynamic properties of pointers Abdul Basit Ijaz
                   ` (3 preceding siblings ...)
  2023-09-04 22:29 ` [PATCH v3 4/4] gdb, testsuite, fortran: Fix sizeof intrinsic for ifort Fortran pointers Abdul Basit Ijaz
@ 2023-09-27 21:11 ` Ijaz, Abdul B
  2023-10-03  0:17 ` [PATCH " Thiago Jung Bauermann
  5 siblings, 0 replies; 18+ messages in thread
From: Ijaz, Abdul B @ 2023-09-27 21:11 UTC (permalink / raw)
  To: gdb-patches; +Cc: simark, tom

Ping!

Thanks & Best Regards,
Abdul Basit

-----Original Message-----
From: Ijaz, Abdul B <abdul.b.ijaz@intel.com> 
Sent: Tuesday, September 5, 2023 12:30 AM
To: gdb-patches@sourceware.org
Cc: Ijaz, Abdul B <abdul.b.ijaz@intel.com>; simark@simark.ca; tom@tromey.com
Subject: [PATCH v3 0/4] Dynamic properties of pointers

From: "Ijaz, Abdul B" <abdul.b.ijaz@intel.com>

Hi!

Please find attached v3 of this series where for v2 series there is already some feedback and the main change in patch 3 review is still missing to fix 'some compiler DWARF that is wrong but we still want to support it' patch after a discussion in https://sourceware.org/pipermail/gdb-patches/2022-September/192159.html

V2 patch 2 was approved by Tom already in this discussion but there are minor changes since then:
https://sourceware.org/pipermail/gdb-patches/2023-January/195353.html

V2 can be found here:
https://sourceware.org/pipermail/gdb-patches/2022-October/192389.html

V1 with feedback can be found here:
https://sourceware.org/pipermail/gdb-patches/2022-September/191934.html

Changes since v2:

  * Patch 1 has minor change where now test for icc versions more
  generally.

  * Patch 2:
  Patch 2 has minor change in TYPE_CODE_PTR handling and rest was already
  reviewed in V2 series for handling of DW_AT_associated attribute
  in patch 3.

  * Patch 3:
  This already has the DW_AT_associated handling from V2 series and only
  handling of reference/pointer type is improved for Intel classic compilers.

  * Patch 4: Added a comment to the change for handling of
  DW_TAG_pointer_type.

I'm looking forward to comments.

Thanks & Best Regards,
Abdul Basit

Bernhard Heckel (1):
  gdb, types: Resolve pointer types dynamically

Nils-Christian Kempke (3):
  gdb, testsuite: handle icc and icpc deprecated remarks
  gdb, intel-classic-compilers, testsuite: workaround icc/icpc/ifort
    pointer/reference DWARF
  gdb, testsuite, fortran: Fix sizeof intrinsic for ifort Fortran
    pointers

 gdb/eval.c                                    |   9 +
 gdb/gdbtypes.c                                | 101 +++++++++-
 gdb/gdbtypes.h                                |   5 +
 gdb/testsuite/gdb.cp/vla-cxx.cc               |   4 +
 gdb/testsuite/gdb.cp/vla-cxx.exp              |  15 ++
 gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp       |  16 +-
 .../icc-ifort-pointers-and-references.c       |  38 ++++
 .../icc-ifort-pointers-and-references.exp     | 169 +++++++++++++++++
 .../gdb.fortran/pointer-to-pointer.exp        |   2 +-
 gdb/testsuite/gdb.fortran/pointers.exp        | 173 ++++++++++++++++++
 gdb/testsuite/gdb.fortran/pointers.f90        |  29 +++
 gdb/testsuite/gdb.fortran/sizeof.exp          | 115 ++++++++++++
 gdb/testsuite/gdb.fortran/sizeof.f90          | 108 +++++++++++
 gdb/testsuite/lib/gdb.exp                     |  14 +-
 gdb/valprint.c                                |  40 +++-
 15 files changed, 812 insertions(+), 26 deletions(-)  create mode 100644 gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.exp
 create mode 100644 gdb/testsuite/gdb.fortran/pointers.exp
 create mode 100644 gdb/testsuite/gdb.fortran/sizeof.exp
 create mode 100644 gdb/testsuite/gdb.fortran/sizeof.f90

--
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 1/4] gdb, testsuite: handle icc and icpc deprecated remarks
  2023-09-04 22:29 ` [PATCH v3 1/4] gdb, testsuite: handle icc and icpc deprecated remarks Abdul Basit Ijaz
@ 2023-10-03  0:04   ` Thiago Jung Bauermann
  0 siblings, 0 replies; 18+ messages in thread
From: Thiago Jung Bauermann @ 2023-10-03  0:04 UTC (permalink / raw)
  To: Abdul Basit Ijaz; +Cc: simark, tom, gdb-patches


Hello,

Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 1b9179401c4..def6a75dfee 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -4980,13 +4980,15 @@ proc gdb_compile {source dest type options} {
>  	    }
>  	}
>  
> -	# Starting with 2021.7.0 (recognized as icc-20-21-7 by GDB) icc and
> -	# icpc are marked as deprecated and both compilers emit the remark
> -	# #10441.  To let GDB still compile successfully, we disable these
> -	# warnings here.
> +	# Starting with 2021.7.0 (recognized as icc-20-21-7 by
> +	# GDB) icc and icpc are marked as deprecated and both
> +	# compilers emit the remark #10441.  To let GDB still
> +	# compile successfully, we disable these warnings here.

I don't see any change in this comment except for the reformatting.
IMO, the text is more readable at the larger width and considering that
it's already less than 80 columns, I suggest dropping this part of the
change.

>  	if {([lsearch -exact $options c++] != -1
> -	     && [test_compiler_info {icc-20-21-[7-9]} c++])
> -	    || [test_compiler_info {icc-20-21-[7-9]}]} {
> +	     && [test_compiler_info {icc-20-21-[7-9]} c++]
> +	     && [test_compiler_info {icc-20-21-[1-9][0-9]} c++])

Unless I'm missing something, the above should be:

  ([lsearch …] != -1
   && ([test_compiler_info <pattern 1> c++]
       || [test_compiler_info <pattern 2> c++])

> +	    || [test_compiler_info {icc-20-21-[7-9]}]
> +	    || [test_compiler_info {icc-20-21-[1-9][0-9]}]} {
>  	    lappend new_options "additional_flags=-diag-disable=10441"
>  	}
>      }

-- 
Thiago

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/4] gdb, types: Resolve pointer types dynamically
  2023-09-04 22:29 ` [PATCH v3 2/4] gdb, types: Resolve pointer types dynamically Abdul Basit Ijaz
@ 2023-10-03  0:07   ` Thiago Jung Bauermann
  2023-10-10 19:45     ` Tom Tromey
  2024-01-03 21:06     ` Ijaz, Abdul B
  2023-10-10 19:49   ` Tom Tromey
  1 sibling, 2 replies; 18+ messages in thread
From: Thiago Jung Bauermann @ 2023-10-03  0:07 UTC (permalink / raw)
  To: Abdul Basit Ijaz; +Cc: simark, tom, gdb-patches


Hello,

I've been working a bit with dynamic types lately, but I'm not confident
enough to provide a Reviewed-by.

In any case, some small nits. Everything else LGTM FWIW.

Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

> diff --git a/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp b/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp
> index 6e4a331eca8..d6b20086c89 100644
> --- a/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp
> @@ -154,7 +154,7 @@ gdb_test "print foo.three_ptr.all'length" \
>           " = 3"
>  
>  gdb_test "ptype foo.three_ptr.all" \
> -         " = array \\(<>\\) of integer"
> +    " = array \\(1 \\.\\. 3\\) of integer"

The new indentation level is inconsistent with the rest of the file.
Please preserve the original indentation level.

>  
>  # foo.three_ptr
>  
> @@ -177,7 +177,7 @@ gdb_test "print foo.three_ptr'length" \
>           " = 3"
>  
>  gdb_test "ptype foo.three_ptr" \
> -         " = access array \\(<>\\) of integer"
> +    " = access array \\(1 \\.\\. 3\\) of integer"

Ditto for the other changes in this file.

> diff --git a/gdb/testsuite/gdb.fortran/pointers.exp b/gdb/testsuite/gdb.fortran/pointers.exp
> new file mode 100644
> index 00000000000..ca2195bbfe6
> --- /dev/null
> +++ b/gdb/testsuite/gdb.fortran/pointers.exp
> @@ -0,0 +1,115 @@
> +# Copyright 2016-2023 Free Software Foundation, Inc.

This is a new file. The copyright starts in 2023.

> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.

-- 
Thiago

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/4] gdb, intel-classic-compilers, testsuite: workaround icc/icpc/ifort pointer/reference DWARF
  2023-09-04 22:29 ` [PATCH v3 3/4] gdb, intel-classic-compilers, testsuite: workaround icc/icpc/ifort pointer/reference DWARF Abdul Basit Ijaz
@ 2023-10-03  0:09   ` Thiago Jung Bauermann
  2023-10-10 19:52   ` Tom Tromey
  1 sibling, 0 replies; 18+ messages in thread
From: Thiago Jung Bauermann @ 2023-10-03  0:09 UTC (permalink / raw)
  To: Abdul Basit Ijaz; +Cc: simark, tom, gdb-patches


Hello,

Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

> ---
>  gdb/gdbtypes.c                                |  94 +++++++++-
>  gdb/gdbtypes.h                                |   5 +
>  .../icc-ifort-pointers-and-references.c       |  38 ++++
>  .../icc-ifort-pointers-and-references.exp     | 169 ++++++++++++++++++
>  gdb/testsuite/gdb.fortran/pointers.exp        |  86 +++++++--
>  gdb/valprint.c                                |  34 ++++
>  6 files changed, 409 insertions(+), 17 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.c
>  create mode 100644 gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.exp

This patch reorganizes the code a bit, but still uses the same approach
as v2. Have you tried implementing the approach suggested by Tom Tromey
in https://inbox.sourceware.org/gdb-patches/87fscoepd5.fsf@tromey.com/ ?

> diff --git a/gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.c b/gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.c
> new file mode 100644
> index 00000000000..0c0be751833
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.c
> @@ -0,0 +1,38 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2021-2023 Free Software Foundation, Inc.

The copyright starts in 2023.

> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

-- 
Thiago

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 4/4] gdb, testsuite, fortran: Fix sizeof intrinsic for ifort Fortran pointers
  2023-09-04 22:29 ` [PATCH v3 4/4] gdb, testsuite, fortran: Fix sizeof intrinsic for ifort Fortran pointers Abdul Basit Ijaz
@ 2023-10-03  0:16   ` Thiago Jung Bauermann
  0 siblings, 0 replies; 18+ messages in thread
From: Thiago Jung Bauermann @ 2023-10-03  0:16 UTC (permalink / raw)
  To: Abdul Basit Ijaz; +Cc: simark, tom, gdb-patches


Hello,

Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

> The patch also adds a test for the sizeof intrinsic which was not tested
> before.

Nice, thanks!

> ---
>  gdb/eval.c                           |   9 +++
>  gdb/testsuite/gdb.fortran/sizeof.exp | 115 +++++++++++++++++++++++++++
>  gdb/testsuite/gdb.fortran/sizeof.f90 | 108 +++++++++++++++++++++++++
>  3 files changed, 232 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.fortran/sizeof.exp
>  create mode 100644 gdb/testsuite/gdb.fortran/sizeof.f90

Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>

-- 
Thiago

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 0/4] Dynamic properties of pointers
  2023-09-04 22:29 [PATCH v3 0/4] Dynamic properties of pointers Abdul Basit Ijaz
                   ` (4 preceding siblings ...)
  2023-09-27 21:11 ` [PING][PATCH v3 0/4] Dynamic properties of pointers Ijaz, Abdul B
@ 2023-10-03  0:17 ` Thiago Jung Bauermann
  5 siblings, 0 replies; 18+ messages in thread
From: Thiago Jung Bauermann @ 2023-10-03  0:17 UTC (permalink / raw)
  To: Abdul Basit Ijaz; +Cc: simark, tom, gdb-patches


Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

> Bernhard Heckel (1):
>   gdb, types: Resolve pointer types dynamically
>
> Nils-Christian Kempke (3):
>   gdb, testsuite: handle icc and icpc deprecated remarks
>   gdb, intel-classic-compilers, testsuite: workaround icc/icpc/ifort
>     pointer/reference DWARF
>   gdb, testsuite, fortran: Fix sizeof intrinsic for ifort Fortran
>     pointers
>
>  gdb/eval.c                                    |   9 +
>  gdb/gdbtypes.c                                | 101 +++++++++-
>  gdb/gdbtypes.h                                |   5 +
>  gdb/testsuite/gdb.cp/vla-cxx.cc               |   4 +
>  gdb/testsuite/gdb.cp/vla-cxx.exp              |  15 ++
>  gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp       |  16 +-
>  .../icc-ifort-pointers-and-references.c       |  38 ++++
>  .../icc-ifort-pointers-and-references.exp     | 169 +++++++++++++++++
>  .../gdb.fortran/pointer-to-pointer.exp        |   2 +-
>  gdb/testsuite/gdb.fortran/pointers.exp        | 173 ++++++++++++++++++
>  gdb/testsuite/gdb.fortran/pointers.f90        |  29 +++
>  gdb/testsuite/gdb.fortran/sizeof.exp          | 115 ++++++++++++
>  gdb/testsuite/gdb.fortran/sizeof.f90          | 108 +++++++++++
>  gdb/testsuite/lib/gdb.exp                     |  14 +-
>  gdb/valprint.c                                |  40 +++-
>  15 files changed, 812 insertions(+), 26 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.c
>  create mode 100644 gdb/testsuite/gdb.dwarf2/icc-ifort-pointers-and-references.exp
>  create mode 100644 gdb/testsuite/gdb.fortran/pointers.exp
>  create mode 100644 gdb/testsuite/gdb.fortran/sizeof.exp
>  create mode 100644 gdb/testsuite/gdb.fortran/sizeof.f90


I ran the tests added or modified by this patch series on aarch64-linux
with gfortran 11.3.0 (from Ubuntu 22.04) and all tests pass:

Tested-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>


-- 
Thiago

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/4] gdb, types: Resolve pointer types dynamically
  2023-10-03  0:07   ` Thiago Jung Bauermann
@ 2023-10-10 19:45     ` Tom Tromey
  2024-01-03 21:06       ` Ijaz, Abdul B
  2024-01-03 21:06     ` Ijaz, Abdul B
  1 sibling, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2023-10-10 19:45 UTC (permalink / raw)
  To: Thiago Jung Bauermann via Gdb-patches
  Cc: Abdul Basit Ijaz, Thiago Jung Bauermann, simark, tom

>>>>> "Thiago" == Thiago Jung Bauermann via Gdb-patches <gdb-patches@sourceware.org> writes:

>> gdb_test "ptype foo.three_ptr.all" \
>> -         " = array \\(<>\\) of integer"
>> +    " = array \\(1 \\.\\. 3\\) of integer"

Thiago> The new indentation level is inconsistent with the rest of the file.
Thiago> Please preserve the original indentation level.

I think gdb standardized on 4 space indent for Tcl, but some files are
still wrong here.  However normally I think we'd want reindentations to
be a separate patch.  Fine to do that as a prelude if it makes your task
easier.

Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/4] gdb, types: Resolve pointer types dynamically
  2023-09-04 22:29 ` [PATCH v3 2/4] gdb, types: Resolve pointer types dynamically Abdul Basit Ijaz
  2023-10-03  0:07   ` Thiago Jung Bauermann
@ 2023-10-10 19:49   ` Tom Tromey
  2024-01-03 21:31     ` Ijaz, Abdul B
  1 sibling, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2023-10-10 19:49 UTC (permalink / raw)
  To: Abdul Basit Ijaz via Gdb-patches; +Cc: Abdul Basit Ijaz, simark, tom

>>>>> Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

> -  /* We only want to recognize references at the outermost level.  */
> -  if (top_level && type->code () == TYPE_CODE_REF)
> +  /* We only want to recognize references and pointers at the outermost
> +     level.  */
> +  if (top_level
> +      && (type->code () == TYPE_CODE_REF || type->code () == TYPE_CODE_PTR))

Pre-existing but I wonder why this code checks TYPE_CODE_REF and not
TYPE_CODE_RVALUE_REF.

> diff --git a/gdb/valprint.c b/gdb/valprint.c
> index b65dda15c04..c71ae089f46 100644
> --- a/gdb/valprint.c
> +++ b/gdb/valprint.c
> @@ -1156,12 +1156,6 @@ value_check_printable (struct value *val, struct ui_file *stream,
>        return 0;
>      }
 
> -  if (type_not_associated (val->type ()))
> -    {
> -      val_print_not_associated (stream);
> -      return 0;
> -    }

I don't really know anything about Fortran, so I don't know why this
code was here in the first place, nor what its removal might mean.
Could you say why this is being removed?

Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/4] gdb, intel-classic-compilers, testsuite: workaround icc/icpc/ifort pointer/reference DWARF
  2023-09-04 22:29 ` [PATCH v3 3/4] gdb, intel-classic-compilers, testsuite: workaround icc/icpc/ifort pointer/reference DWARF Abdul Basit Ijaz
  2023-10-03  0:09   ` Thiago Jung Bauermann
@ 2023-10-10 19:52   ` Tom Tromey
  2024-01-03 21:15     ` Ijaz, Abdul B
  1 sibling, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2023-10-10 19:52 UTC (permalink / raw)
  To: Abdul Basit Ijaz via Gdb-patches; +Cc: Abdul Basit Ijaz, simark, tom

>>>>> Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

> +bool
> +icc_pointer_or_reference_type (const struct type *type)
> +{
> +  return (type->code () == TYPE_CODE_PTR || type->code () == TYPE_CODE_REF)
> +	 && type->is_objfile_owned ()
> +	 && std::any_of (type->objfile_owner ()->compunits ().begin (),
> +			 type->objfile_owner ()->compunits ().end (),
> +			 [] (const compunit_symtab *cu)
> +			 {
> +			   return producer_is_icc (cu->producer (), nullptr,
> +						   nullptr);
> +			 });
> +}

It seems to me that it would be better to do this kind of checking when
reading the DWARF.  That way it could be targeted to exactly types in
ICC CUs, and it would avoid putting ICC workarounds into generic code --
the workarounds would be confined to the DWARF reader.

Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v3 2/4] gdb, types: Resolve pointer types dynamically
  2023-10-10 19:45     ` Tom Tromey
@ 2024-01-03 21:06       ` Ijaz, Abdul B
  0 siblings, 0 replies; 18+ messages in thread
From: Ijaz, Abdul B @ 2024-01-03 21:06 UTC (permalink / raw)
  To: Tom Tromey, Thiago Jung Bauermann via Gdb-patches
  Cc: Thiago Jung Bauermann, simark

Hi Tom and Thiago,

Thanks a lot for the feedback.

>> gdb_test "ptype foo.three_ptr.all" \
>> -         " = array \\(<>\\) of integer"
>> +    " = array \\(1 \\.\\. 3\\) of integer"

Thiago> The new indentation level is inconsistent with the rest of the file.
Thiago> Please preserve the original indentation level.

Tom > I think gdb standardized on 4 space indent for Tcl, but some files are still wrong here.  However normally I think we'd want reindentations to be a separate patch.  Fine to do that as a prelude if it makes your task easier.

I will add an extra patch to this series for fixing existing indentation problem in test gdb.dwarf2/dynarr-ptr.exp and then add changes to test on top of it.


Thanks & Best Regards
Abdul Basit

-----Original Message-----
From: Tom Tromey <tom@tromey.com> 
Sent: Tuesday, October 10, 2023 9:46 PM
To: Thiago Jung Bauermann via Gdb-patches <gdb-patches@sourceware.org>
Cc: Ijaz, Abdul B <abdul.b.ijaz@intel.com>; Thiago Jung Bauermann <thiago.bauermann@linaro.org>; simark@simark.ca; tom@tromey.com
Subject: Re: [PATCH v3 2/4] gdb, types: Resolve pointer types dynamically

>>>>> "Thiago" == Thiago Jung Bauermann via Gdb-patches <gdb-patches@sourceware.org> writes:

>> gdb_test "ptype foo.three_ptr.all" \
>> -         " = array \\(<>\\) of integer"
>> +    " = array \\(1 \\.\\. 3\\) of integer"

Thiago> The new indentation level is inconsistent with the rest of the file.
Thiago> Please preserve the original indentation level.

I think gdb standardized on 4 space indent for Tcl, but some files are still wrong here.  However normally I think we'd want reindentations to be a separate patch.  Fine to do that as a prelude if it makes your task easier.

Tom
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v3 2/4] gdb, types: Resolve pointer types dynamically
  2023-10-03  0:07   ` Thiago Jung Bauermann
  2023-10-10 19:45     ` Tom Tromey
@ 2024-01-03 21:06     ` Ijaz, Abdul B
  1 sibling, 0 replies; 18+ messages in thread
From: Ijaz, Abdul B @ 2024-01-03 21:06 UTC (permalink / raw)
  To: Thiago Jung Bauermann; +Cc: simark, tom, gdb-patches

Hi Thiago,

Thanks for the feedback.

> The new indentation level is inconsistent with the rest of the file.
> Please preserve the original indentation level.

Replied in other email also that will add an extra patch in this series to fix indentation in this test.

> This is a new file. The copyright starts in 2023.

Will update it to 2024 in v4 series.

Best Regards,
Abdul Basit

-----Original Message-----
From: Thiago Jung Bauermann <thiago.bauermann@linaro.org> 
Sent: Tuesday, October 3, 2023 2:08 AM
To: Ijaz, Abdul B <abdul.b.ijaz@intel.com>
Cc: simark@simark.ca; tom@tromey.com; gdb-patches@sourceware.org
Subject: Re: [PATCH v3 2/4] gdb, types: Resolve pointer types dynamically


Hello,

I've been working a bit with dynamic types lately, but I'm not confident enough to provide a Reviewed-by.

In any case, some small nits. Everything else LGTM FWIW.

Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

> diff --git a/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp 
> b/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp
> index 6e4a331eca8..d6b20086c89 100644
> --- a/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp
> @@ -154,7 +154,7 @@ gdb_test "print foo.three_ptr.all'length" \
>           " = 3"
>  
>  gdb_test "ptype foo.three_ptr.all" \
> -         " = array \\(<>\\) of integer"
> +    " = array \\(1 \\.\\. 3\\) of integer"

The new indentation level is inconsistent with the rest of the file.
Please preserve the original indentation level.

>  
>  # foo.three_ptr
>  
> @@ -177,7 +177,7 @@ gdb_test "print foo.three_ptr'length" \
>           " = 3"
>  
>  gdb_test "ptype foo.three_ptr" \
> -         " = access array \\(<>\\) of integer"
> +    " = access array \\(1 \\.\\. 3\\) of integer"

Ditto for the other changes in this file.

> diff --git a/gdb/testsuite/gdb.fortran/pointers.exp 
> b/gdb/testsuite/gdb.fortran/pointers.exp
> new file mode 100644
> index 00000000000..ca2195bbfe6
> --- /dev/null
> +++ b/gdb/testsuite/gdb.fortran/pointers.exp
> @@ -0,0 +1,115 @@
> +# Copyright 2016-2023 Free Software Foundation, Inc.

This is a new file. The copyright starts in 2023.

> +
> +# This program is free software; you can redistribute it and/or 
> +modify # it under the terms of the GNU General Public License as 
> +published by # the Free Software Foundation; either version 3 of the 
> +License, or # (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful, # 
> +but WITHOUT ANY WARRANTY; without even the implied warranty of # 
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the # GNU 
> +General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License # 
> +along with this program.  If not, see <http://www.gnu.org/licenses/>.

--
Thiago
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v3 3/4] gdb, intel-classic-compilers, testsuite: workaround icc/icpc/ifort pointer/reference DWARF
  2023-10-10 19:52   ` Tom Tromey
@ 2024-01-03 21:15     ` Ijaz, Abdul B
  0 siblings, 0 replies; 18+ messages in thread
From: Ijaz, Abdul B @ 2024-01-03 21:15 UTC (permalink / raw)
  To: Tom Tromey, Abdul Basit Ijaz via Gdb-patches; +Cc: simark

Hi Tom,

Thanks a lot for the feedback.

> It seems to me that it would be better to do this kind of checking when reading the DWARF.  That way it could be targeted to exactly types in ICC CUs

I will drop these intel-classic-compilers (icc/ifort) related changes in the V4 patch series as there is no special dwarf handling needed for icx/ifx compilers.  So only patch 2/4 from this series would be used in V4 series.

Best Regards
Abdul Basit

-----Original Message-----
From: Tom Tromey <tom@tromey.com> 
Sent: Tuesday, October 10, 2023 9:53 PM
To: Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org>
Cc: Ijaz, Abdul B <abdul.b.ijaz@intel.com>; simark@simark.ca; tom@tromey.com
Subject: Re: [PATCH v3 3/4] gdb, intel-classic-compilers, testsuite: workaround icc/icpc/ifort pointer/reference DWARF

>>>>> Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

> +bool
> +icc_pointer_or_reference_type (const struct type *type) {
> +  return (type->code () == TYPE_CODE_PTR || type->code () == TYPE_CODE_REF)
> +	 && type->is_objfile_owned ()
> +	 && std::any_of (type->objfile_owner ()->compunits ().begin (),
> +			 type->objfile_owner ()->compunits ().end (),
> +			 [] (const compunit_symtab *cu)
> +			 {
> +			   return producer_is_icc (cu->producer (), nullptr,
> +						   nullptr);
> +			 });
> +}

It seems to me that it would be better to do this kind of checking when reading the DWARF.  That way it could be targeted to exactly types in ICC CUs, and it would avoid putting ICC workarounds into generic code -- the workarounds would be confined to the DWARF reader.

Tom
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH v3 2/4] gdb, types: Resolve pointer types dynamically
  2023-10-10 19:49   ` Tom Tromey
@ 2024-01-03 21:31     ` Ijaz, Abdul B
  0 siblings, 0 replies; 18+ messages in thread
From: Ijaz, Abdul B @ 2024-01-03 21:31 UTC (permalink / raw)
  To: Tom Tromey, Abdul Basit Ijaz via Gdb-patches; +Cc: simark, gdb-patches

Hi Tom,

Thanks a lot for the feedback.

>> -  /* We only want to recognize references at the outermost level.  */
>> -  if (top_level && type->code () == TYPE_CODE_REF)
>> +  /* We only want to recognize references and pointers at the outermost
>> +     level.  */
>> +  if (top_level
>> +      && (type->code () == TYPE_CODE_REF || type->code () == 
>> + TYPE_CODE_PTR))

Tom > Pre-existing but I wonder why this code checks TYPE_CODE_REF and not TYPE_CODE_RVALUE_REF.

Here we are checking for LVALUE or Pointers dynamic value and for them need to resolve the target memory to resolve pointer/memory at outmost level.  So as far as I understood checking for rvalue here would not be needed.  In case you have some case to try then please let me know I can give it a try.

>> -  if (type_not_associated (val->type ()))
>> -    {
>> -      val_print_not_associated (stream);
>> -      return 0;
>> -    }

Tom > I don't really know anything about Fortran, so I don't know why this code was here in the first place, nor what its removal might mean.

This was originally added at time to support DW_AT_Associated/Allocated DIEs for Fortran dynamic arrays but this is a redundant code so no need to keep it. Also in existing testsuite there is no affect with or without it.  Will mention in the commit message in V4 series.

Best Regards,
Abdul Basit

-----Original Message-----
From: Tom Tromey <tom@tromey.com> 
Sent: Tuesday, October 10, 2023 9:50 PM
To: Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org>
Cc: Ijaz, Abdul B <abdul.b.ijaz@intel.com>; simark@simark.ca; tom@tromey.com
Subject: Re: [PATCH v3 2/4] gdb, types: Resolve pointer types dynamically

>>>>> Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

> -  /* We only want to recognize references at the outermost level.  */
> -  if (top_level && type->code () == TYPE_CODE_REF)
> +  /* We only want to recognize references and pointers at the outermost
> +     level.  */
> +  if (top_level
> +      && (type->code () == TYPE_CODE_REF || type->code () == 
> + TYPE_CODE_PTR))

Pre-existing but I wonder why this code checks TYPE_CODE_REF and not TYPE_CODE_RVALUE_REF.

> diff --git a/gdb/valprint.c b/gdb/valprint.c index 
> b65dda15c04..c71ae089f46 100644
> --- a/gdb/valprint.c
> +++ b/gdb/valprint.c
> @@ -1156,12 +1156,6 @@ value_check_printable (struct value *val, struct ui_file *stream,
>        return 0;
>      }
 
> -  if (type_not_associated (val->type ()))
> -    {
> -      val_print_not_associated (stream);
> -      return 0;
> -    }

I don't really know anything about Fortran, so I don't know why this code was here in the first place, nor what its removal might mean.
Could you say why this is being removed?

Tom
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2024-01-03 21:31 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-04 22:29 [PATCH v3 0/4] Dynamic properties of pointers Abdul Basit Ijaz
2023-09-04 22:29 ` [PATCH v3 1/4] gdb, testsuite: handle icc and icpc deprecated remarks Abdul Basit Ijaz
2023-10-03  0:04   ` Thiago Jung Bauermann
2023-09-04 22:29 ` [PATCH v3 2/4] gdb, types: Resolve pointer types dynamically Abdul Basit Ijaz
2023-10-03  0:07   ` Thiago Jung Bauermann
2023-10-10 19:45     ` Tom Tromey
2024-01-03 21:06       ` Ijaz, Abdul B
2024-01-03 21:06     ` Ijaz, Abdul B
2023-10-10 19:49   ` Tom Tromey
2024-01-03 21:31     ` Ijaz, Abdul B
2023-09-04 22:29 ` [PATCH v3 3/4] gdb, intel-classic-compilers, testsuite: workaround icc/icpc/ifort pointer/reference DWARF Abdul Basit Ijaz
2023-10-03  0:09   ` Thiago Jung Bauermann
2023-10-10 19:52   ` Tom Tromey
2024-01-03 21:15     ` Ijaz, Abdul B
2023-09-04 22:29 ` [PATCH v3 4/4] gdb, testsuite, fortran: Fix sizeof intrinsic for ifort Fortran pointers Abdul Basit Ijaz
2023-10-03  0:16   ` Thiago Jung Bauermann
2023-09-27 21:11 ` [PING][PATCH v3 0/4] Dynamic properties of pointers Ijaz, Abdul B
2023-10-03  0:17 ` [PATCH " Thiago Jung Bauermann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).