Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH 0/2] Resolve dynamic types for pointers
@ 2022-01-18 13:26 Nils-Christian Kempke via Gdb-patches
  2022-01-18 13:26 ` [PATCH 1/2] gdb/types: Resolve dynamic properties of pointer types Nils-Christian Kempke via Gdb-patches
  2022-01-18 13:26 ` [PATCH 2/2] gdb: Resolve dynamic target types of pointers Nils-Christian Kempke via Gdb-patches
  0 siblings, 2 replies; 10+ messages in thread
From: Nils-Christian Kempke via Gdb-patches @ 2022-01-18 13:26 UTC (permalink / raw)
  To: gdb-patches

Hi,

please find attached two patches aming at improving usability,
especially but not limited to Fortran and C/Cpp.

The following two patches attempt to resolve dynamic properties for
pointers when printing/accessing them.  When printing/dereferencing 
pointers with dynamic target types we resolve the target type first.
This enables GDB to print additional information about the type of the 
pointer/underlying object.

For example, in C this enables the printing of the dimension of a
dynamic array pointed to by an array pointer.

Any comments are much appreciated!

Cheers,

Nils


Bernhard Heckel (2):
  gdb/types: Resolve dynamic properties of pointer types.
  gdb: Resolve dynamic target types of pointers.

 gdb/c-valprint.c                              |  19 +-
 gdb/gdbtypes.c                                |  29 ++-
 gdb/testsuite/gdb.cp/vla-cxx.cc               |   4 +
 gdb/testsuite/gdb.cp/vla-cxx.exp              |  33 ++++
 gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp       |   4 +-
 .../gdb.fortran/pointer-to-pointer.exp        |   2 +-
 gdb/testsuite/gdb.fortran/pointers.exp        | 181 ++++++++++++++++++
 gdb/testsuite/gdb.fortran/pointers.f90        |  29 +++
 gdb/typeprint.c                               |  17 ++
 gdb/valops.c                                  |  11 ++
 gdb/valprint.c                                |   6 -
 11 files changed, 323 insertions(+), 12 deletions(-)
 create mode 100644 gdb/testsuite/gdb.fortran/pointers.exp

-- 
2.25.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] 10+ messages in thread

* [PATCH 1/2] gdb/types: Resolve dynamic properties of pointer types.
  2022-01-18 13:26 [PATCH 0/2] Resolve dynamic types for pointers Nils-Christian Kempke via Gdb-patches
@ 2022-01-18 13:26 ` Nils-Christian Kempke via Gdb-patches
  2022-01-28 18:30   ` Tom Tromey
  2022-01-18 13:26 ` [PATCH 2/2] gdb: Resolve dynamic target types of pointers Nils-Christian Kempke via Gdb-patches
  1 sibling, 1 reply; 10+ messages in thread
From: Nils-Christian Kempke via Gdb-patches @ 2022-01-18 13:26 UTC (permalink / raw)
  To: gdb-patches

Mark pointers with target type that is considered as a dynamic type
as dynamic types themselves (on the outmost level).  Resolve dynamic
type pointers.  Since ifx and ifort emit the DW_AT_associated for
pointers we treat this as well when resolving dynamic pointers.

This commit prepares the next one which will resolve dynamic target
types of pointers when printing them enabling the print of e.g. array
dimension when printing a pointer pointing to a dynamic array.
---
 gdb/gdbtypes.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 8af96c79e6..cd72a23c6f 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2077,8 +2077,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 (type));
 
   /* Types that have a dynamic TYPE_DATA_LOCATION are considered
@@ -2664,6 +2666,25 @@ resolve_dynamic_struct (struct type *type,
   return resolved_type;
 }
 
+/* Worker for pointer types.  */
+
+static struct type *
+resolve_dynamic_pointer (struct type *type,
+			 struct property_addr_info *addr_stack)
+{
+  struct dynamic_prop *prop;
+  CORE_ADDR value;
+
+  type = copy_type (type);
+
+  /* Resolve associated property.  */
+  prop = TYPE_ASSOCIATED_PROP (type);
+  if (prop != NULL && dwarf2_evaluate_property (prop, NULL, addr_stack, &value))
+    prop->set_const_val (value);
+
+  return type;
+}
+
 /* Worker for resolved_dynamic_type.  */
 
 static struct type *
@@ -2719,6 +2740,10 @@ resolve_dynamic_type_internal (struct type *type,
 	    break;
 	  }
 
+	case TYPE_CODE_PTR:
+	  resolved_type = resolve_dynamic_pointer (type, addr_stack);
+	  break;
+
 	case TYPE_CODE_STRING:
 	  /* Strings are very much like an array of characters, and can be
 	     treated as one here.  */
-- 
2.25.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] 10+ messages in thread

* [PATCH 2/2] gdb: Resolve dynamic target types of pointers.
  2022-01-18 13:26 [PATCH 0/2] Resolve dynamic types for pointers Nils-Christian Kempke via Gdb-patches
  2022-01-18 13:26 ` [PATCH 1/2] gdb/types: Resolve dynamic properties of pointer types Nils-Christian Kempke via Gdb-patches
@ 2022-01-18 13:26 ` Nils-Christian Kempke via Gdb-patches
  2022-02-10 19:47   ` Tom Tromey
  1 sibling, 1 reply; 10+ messages in thread
From: Nils-Christian Kempke via Gdb-patches @ 2022-01-18 13:26 UTC (permalink / raw)
  To: gdb-patches

Resolve the dynamic target type of a pointer as we might want to print
more details of the target like the dimension of an array.  When
dereferencing pointers to dynamic target types we resolve the target type.

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 pointers 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/c-valprint.c                              |  19 +-
 gdb/testsuite/gdb.cp/vla-cxx.cc               |   4 +
 gdb/testsuite/gdb.cp/vla-cxx.exp              |  33 ++++
 gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp       |   4 +-
 .../gdb.fortran/pointer-to-pointer.exp        |   2 +-
 gdb/testsuite/gdb.fortran/pointers.exp        | 181 ++++++++++++++++++
 gdb/testsuite/gdb.fortran/pointers.f90        |  29 +++
 gdb/typeprint.c                               |  17 ++
 gdb/valops.c                                  |  11 ++
 gdb/valprint.c                                |   6 -
 10 files changed, 296 insertions(+), 10 deletions(-)
 create mode 100644 gdb/testsuite/gdb.fortran/pointers.exp

diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
index 461763075e..3eb0a72228 100644
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -545,7 +545,24 @@ c_value_print (struct value *val, struct ui_file *stream,
 	}
       else
 	{
-	  /* normal case */
+	  /* Normal case.  */
+	  if (type->code () == TYPE_CODE_PTR && is_dynamic_type (type))
+	    {
+	      CORE_ADDR addr;
+	      if (nullptr != TYPE_DATA_LOCATION (TYPE_TARGET_TYPE (type)))
+		addr = value_address (val);
+	      else
+		addr = value_as_address (val);
+
+	      /* Resolve target-type only when the pointer is associated.  */
+	      if ((addr != 0) && !type_not_associated (type))
+		  TYPE_TARGET_TYPE (type) =
+		      resolve_dynamic_type (TYPE_TARGET_TYPE (type), {}, addr);
+	    }
+	  else
+	    {
+	      /* Do nothing.  References are already resolved.  */
+	    }
 	  fprintf_filtered (stream, "(");
 	  type_print (value_type (val), "", stream, -1);
 	  fprintf_filtered (stream, ") ");
diff --git a/gdb/testsuite/gdb.cp/vla-cxx.cc b/gdb/testsuite/gdb.cp/vla-cxx.cc
index 9795f8cc39..c03d1a80ac 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 3494b5e8b7..9c81b1700d 100644
--- a/gdb/testsuite/gdb.cp/vla-cxx.exp
+++ b/gdb/testsuite/gdb.cp/vla-cxx.exp
@@ -23,6 +23,36 @@ if ![runto_main] {
     return -1
 }
 
+gdb_breakpoint [gdb_get_line_number "Before pointer assignment"]
+gdb_continue_to_breakpoint "Before pointer assignment"
+
+set test_name "ptype ptr, Before pointer assignment"
+gdb_test_multiple "ptype ptr" $test_name {
+  # gfortran
+  -re "= int \\(\\*\\)\\\[variable length\\\]\r\n$gdb_prompt $" {
+    pass $test_name
+  }
+  # ifort/ifx
+  -re "= int \\(\\*\\)\\\[3\\\]\r\n$gdb_prompt $" {
+    pass $test_name
+  }
+}
+
+set test_name "print ptr, Before pointer assignment"
+gdb_test_multiple "print ptr" $test_name {
+  # gfortran
+  -re "= \\(int \\(\\*\\)\\\[variable length\\\]\\) 0x0\r\n$gdb_prompt $" {
+    pass $test_name
+  }
+  # ifort/ifx
+  -re "= \\(int \\(\\*\\)\\\[3\\\]\\) 0x0\r\n$gdb_prompt $" {
+    pass $test_name
+  }
+}
+
+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 +63,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 83a5fccd83..ce28868d60 100644
--- a/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp
+++ b/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp
@@ -179,7 +179,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
 
@@ -289,7 +289,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
 
diff --git a/gdb/testsuite/gdb.fortran/pointer-to-pointer.exp b/gdb/testsuite/gdb.fortran/pointer-to-pointer.exp
index 8c43d17729..fcaa4bc970 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 0000000000..b093ce72bd
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/pointers.exp
@@ -0,0 +1,181 @@
+# Copyright 2016 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 ![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"
+
+set test "print intap, not associated"
+gdb_test_multiple "print intap" $test {
+  # gfortran
+  -re " = <not associated>\r\n$gdb_prompt $" {
+    pass $test
+  }
+  # ifort/ifx
+  -re " = \\(PTR TO -> \\( $int \\(:,:\\) \\)\\) <not associated>\r\n$gdb_prompt $" {
+    pass $test
+  }
+}
+
+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"
+set test "print cyclicp1, not associated"
+gdb_test_multiple "print cyclicp1" $test {
+  # gfortran
+  -re "= \\( i = -?\\d+, p = 0x0 \\)\r\n$gdb_prompt $" {
+    pass $test
+  }
+  # ifort/ifx
+  -re "= \\( i = -?\\d+, p = <not associated> \\)\r\n$gdb_prompt $" {
+    pass $test
+  }
+}
+
+set test "print cyclicp1%p, not associated"
+gdb_test_multiple "print cyclicp1%p" $test {
+  # gfortran
+  -re "= \\(PTR TO -> \\( Type typewithpointer \\)\\) 0x0\r\n$gdb_prompt $" {
+    pass $test
+  }
+  # ifort/ifx
+  -re "= \\(PTR TO -> \\( Type typewithpointer \\)\\) <not associated>\r\n$gdb_prompt $" {
+    pass $test
+  }
+}
+
+
+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"
+
+set test_name "print intap, associated"
+gdb_test_multiple "print intap" $test_name {
+  # gfortran
+  -re "= \\(\\(1, 1, 3(, 1){7}\\) \\(1(, 1){9}\\)\\)\r\n$gdb_prompt $" {
+    pass $test_name
+  }
+  # ifort/ifx
+  -re "= \\(PTR TO -> \\( $int \\(10,2\\) \\)\\) $hex\( <.*>\)?\r\n$gdb_prompt $" {
+    gdb_test "print *intap" "= \\(\\(1, 1, 3(, 1){7}\\) \\(1(, 1){9}\\)\\)"
+    pass $test_name
+  }
+}
+
+set test_name "print intvlap, associated"
+gdb_test_multiple "print intvlap" $test_name {
+  # gfortran
+  -re "= \\(2, 2, 2, 4(, 2){6}\\)\r\n$gdb_prompt $" {
+    pass $test_name
+  }
+  # ifort/ifx
+  -re "= \\(PTR TO -> \\( $int \\(10\\) \\)\\) $hex\( <.*>\)?\r\n$gdb_prompt $" {
+    gdb_test "print *intvlap" "= \\(2, 2, 2, 4(, 2){6}\\)"
+    pass $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\\)\\) \\)"
+
+set test_name "print arrayOfPtr(3)%p"
+gdb_test_multiple $test_name $test_name {
+  # gfortran
+  -re "= \\(PTR TO -> \\( Type two \\)\\) 0x0\r\n$gdb_prompt $" {
+    pass $test_name
+  }
+  # ifort/ifx
+   -re "= \\(PTR TO -> \\( Type two \\)\\) <not associated>\r\n$gdb_prompt $" {
+    pass $test_name
+  }
+}
+
+set test "print *(arrayOfPtr(3)%p)"
+set test_name "print *(arrayOfPtr(3)%p), associated"
+gdb_test_multiple $test $test_name {
+  # gfortran
+  -re "Cannot access memory at address 0x0\r\n$gdb_prompt $" {
+    pass $test_name
+  }
+  # ifort/ifx
+  -re "Location address is not set.\r\n$gdb_prompt $" {
+    pass $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 7f62866254..e480bdc7fb 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/typeprint.c b/gdb/typeprint.c
index 2428aa2244..c07dede118 100644
--- a/gdb/typeprint.c
+++ b/gdb/typeprint.c
@@ -578,6 +578,23 @@ whatis_exp (const char *exp, int show)
       printf_filtered (" */\n");    
     }
 
+  /* Resolve any dynamic target type, as we might print additional information
+     about the target.  For example, in Fortran and C we are printing the
+     dimension of the dynamic array the pointer is pointing to.  */
+  if (type->code () == TYPE_CODE_PTR && is_dynamic_type (type))
+    {
+      CORE_ADDR addr;
+
+      if (nullptr != TYPE_DATA_LOCATION (TYPE_TARGET_TYPE (type)))
+	addr = value_address (val);
+      else
+	addr = value_as_address (val);
+
+      if (addr != 0 && !type_not_associated (type))
+	TYPE_TARGET_TYPE (type) = resolve_dynamic_type (TYPE_TARGET_TYPE (type),
+							{}, addr);
+    }
+
   LA_PRINT_TYPE (type, "", gdb_stdout, show, 0, &flags);
   printf_filtered ("\n");
 }
diff --git a/gdb/valops.c b/gdb/valops.c
index e091c445e7..08d3f3f3fa 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -1644,6 +1644,16 @@ value_ind (struct value *arg1)
   if (base_type->code () == TYPE_CODE_PTR)
     {
       struct type *enc_type;
+      CORE_ADDR addr;
+
+      if (nullptr != TYPE_DATA_LOCATION (TYPE_TARGET_TYPE (base_type)))
+	addr = value_address (arg1);
+      else
+	addr = value_as_address (arg1);
+
+      if (addr != 0)
+	TYPE_TARGET_TYPE (base_type) =
+	    resolve_dynamic_type (TYPE_TARGET_TYPE (base_type), {}, addr);
 
       /* We may be pointing to something embedded in a larger object.
 	 Get the real type of the enclosing object.  */
@@ -1664,6 +1674,7 @@ value_ind (struct value *arg1)
 	  base_addr = (value_as_address (arg1)
 		       - value_pointed_to_offset (arg1));
 	}
+      /* Retrieve the enclosing object pointed to.  */
       arg2 = value_at_lazy (enc_type, base_addr);
       enc_type = value_type (arg2);
       return readjust_indirect_value_type (arg2, enc_type, base_type,
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 0bc739cf2e..041f544c3d 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -1112,12 +1112,6 @@ value_check_printable (struct value *val, struct ui_file *stream,
       return 0;
     }
 
-  if (type_not_associated (value_type (val)))
-    {
-      val_print_not_associated (stream);
-      return 0;
-    }
-
   if (type_not_allocated (value_type (val)))
     {
       val_print_not_allocated (stream);
-- 
2.25.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] 10+ messages in thread

* Re: [PATCH 1/2] gdb/types: Resolve dynamic properties of pointer types.
  2022-01-18 13:26 ` [PATCH 1/2] gdb/types: Resolve dynamic properties of pointer types Nils-Christian Kempke via Gdb-patches
@ 2022-01-28 18:30   ` Tom Tromey
  2022-01-31  8:15     ` Kempke, Nils-Christian via Gdb-patches
  0 siblings, 1 reply; 10+ messages in thread
From: Tom Tromey @ 2022-01-28 18:30 UTC (permalink / raw)
  To: Nils-Christian Kempke via Gdb-patches

>> +  if (top_level &&
>> +      (type->code () == TYPE_CODE_REF || type->code () == TYPE_CODE_PTR))

In the gdb style, operators like "&&" go on the start of the
continuation line.

Tom

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

* RE: [PATCH 1/2] gdb/types: Resolve dynamic properties of pointer types.
  2022-01-28 18:30   ` Tom Tromey
@ 2022-01-31  8:15     ` Kempke, Nils-Christian via Gdb-patches
  0 siblings, 0 replies; 10+ messages in thread
From: Kempke, Nils-Christian via Gdb-patches @ 2022-01-31  8:15 UTC (permalink / raw)
  To: Tom Tromey, Nils-Christian Kempke via Gdb-patches



> -----Original Message-----
> From: Tom Tromey <tom@tromey.com>
> Sent: Friday, January 28, 2022 7:30 PM
> To: Nils-Christian Kempke via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Kempke, Nils-Christian <nils-christian.kempke@intel.com>
> Subject: Re: [PATCH 1/2] gdb/types: Resolve dynamic properties of pointer
> types.
> 
> >> +  if (top_level &&
> >> +      (type->code () == TYPE_CODE_REF || type->code () ==
> TYPE_CODE_PTR))
> 
> In the gdb style, operators like "&&" go on the start of the
> continuation line.
> 
> Tom

Hi Tom,

Thanks for looking at this. Yes - that's indeed correct I'll change
it and keep it in mind for a possible v2 of this patch. Did you
have a chance to look into this more deeply/have any more comments
on this patch?

Cheers,
Nils
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] 10+ messages in thread

* Re: [PATCH 2/2] gdb: Resolve dynamic target types of pointers.
  2022-01-18 13:26 ` [PATCH 2/2] gdb: Resolve dynamic target types of pointers Nils-Christian Kempke via Gdb-patches
@ 2022-02-10 19:47   ` Tom Tromey
  2022-04-14 10:22     ` Kempke, Nils-Christian via Gdb-patches
  0 siblings, 1 reply; 10+ messages in thread
From: Tom Tromey @ 2022-02-10 19:47 UTC (permalink / raw)
  To: Nils-Christian Kempke via Gdb-patches

>>>>> Nils-Christian Kempke via Gdb-patches <gdb-patches@sourceware.org> writes:

Hi.  Thanks for the patch.

> +	  if (type->code () == TYPE_CODE_PTR && is_dynamic_type (type))
> +	    {
> +	      CORE_ADDR addr;
> +	      if (nullptr != TYPE_DATA_LOCATION (TYPE_TARGET_TYPE (type)))
> +		addr = value_address (val);
> +	      else
> +		addr = value_as_address (val);

This seems weird to me.  When does value_as_address fail?
It seems to me that a value whose type is TYPE_CODE_PTR should be more
easily convertible to a CORE_ADDR without examining TYPE_DATA_LOCATION.

The same thing applies in a couple more spots in the patch.

I'm curious why the code added here is not also needed in f-valprint.c.

> --- a/gdb/testsuite/gdb.cp/vla-cxx.exp
> +++ b/gdb/testsuite/gdb.cp/vla-cxx.exp
> @@ -23,6 +23,36 @@ if ![runto_main] {
>      return -1
>  }
 
> +gdb_breakpoint [gdb_get_line_number "Before pointer assignment"]
> +gdb_continue_to_breakpoint "Before pointer assignment"
> +
> +set test_name "ptype ptr, Before pointer assignment"
> +gdb_test_multiple "ptype ptr" $test_name {
> +  # gfortran
> +  -re "= int \\(\\*\\)\\\[variable length\\\]\r\n$gdb_prompt $" {
> +    pass $test_name
> +  }
> +  # ifort/ifx

This references fortran compilers but is a c++ test.

> +gdb_test "ptype ptr" "int \\(\\*\\)\\\[3\\\]"
> +gdb_test "print ptr" "\\(int \\(\\*\\)\\\[3\\\]\\) $hex"
> +gdb_test "print *ptr" " = \\{5, 7, 9\\}"

Do these pass with all compilers or do they also need to be conditional?

thanks,
Tom

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

* RE: [PATCH 2/2] gdb: Resolve dynamic target types of pointers.
  2022-02-10 19:47   ` Tom Tromey
@ 2022-04-14 10:22     ` Kempke, Nils-Christian via Gdb-patches
  2022-04-15 16:14       ` Tom Tromey
  0 siblings, 1 reply; 10+ messages in thread
From: Kempke, Nils-Christian via Gdb-patches @ 2022-04-14 10:22 UTC (permalink / raw)
  To: Tom Tromey, Nils-Christian Kempke via Gdb-patches

Hi Tom,
Thanks for the feedback. Sorry that it took me so long to reply - the mail
somehow got lost..

> -----Original Message-----
> From: Tom Tromey <tom@tromey.com>
> Sent: Thursday, February 10, 2022 8:47 PM
> To: Nils-Christian Kempke via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Kempke, Nils-Christian <nils-christian.kempke@intel.com>
> Subject: Re: [PATCH 2/2] gdb: Resolve dynamic target types of pointers.
> 
> >>>>> Nils-Christian Kempke via Gdb-patches <gdb-
> patches@sourceware.org> writes:
> 
> Hi.  Thanks for the patch.
> 
> > +	  if (type->code () == TYPE_CODE_PTR && is_dynamic_type (type))
> > +	    {
> > +	      CORE_ADDR addr;
> > +	      if (nullptr != TYPE_DATA_LOCATION (TYPE_TARGET_TYPE (type)))
> > +		addr = value_address (val);
> > +	      else
> > +		addr = value_as_address (val);
> 
> This seems weird to me.  When does value_as_address fail?
> It seems to me that a value whose type is TYPE_CODE_PTR should be more
> easily convertible to a CORE_ADDR without examining
> TYPE_DATA_LOCATION.
> 
> The same thing applies in a couple more spots in the patch.
> 

This if else has been introduced because of the way ifort/icc describe pointers
vs. gcc/gfortran and icx/ifx.  For the simplified DWARF below I'll try and omit
non-useful entries.  As an example I'll take a pointer to an array (which is what
we want in the situation of dynamic target types I guess).  Different from 
gfortran and ifx the DWARF entry for a pointer in ifort is a

<1><AAA> DW_TAG_variable
    <> DW_AT_type: <BBB>
    <> DW_AT_location: (location of the pointer)
<1><BBB> DW_TAG_pointer_type
    DW_AT_type: <CCC>
<1><CCC> DW_TAG_array_type
    <> DW_AT_type: <DDD>
    <> DW_AT_data_location: (location of the array)
<2><EEE> DW_TAG_subrange_type

So, in ifort the DW_AT_location of the DW_TAG_variable is actually the
location of the pointer variable and not the location of the array.
Dereferencing this location then actually yields the location of the array. And
this location is then used in the DW_AT_data_location expression to
retrieve the actual array.
When printing such a pointer in gdb one will end up in the c_value_print
function in c-valprint.c. Taking the value_as_address now fails - since
it is not the address of the target_type but the address of the address of the
target_type. The same thing holds for dereferencing such a DWARF
construct in value_ind in valops.c. 
To resolve this, an additional check is added for the target_type's
data_location. If the target type of a pointer does have a data_location, then
that one is being used as location of the underlying type.
For gfortran and ifx the same pointer to array in Fortran is described as

Fortran:

<1><AAA> DW_TAG_variable
    <>   DW_AT_type: <BBB>
    <>   DW_AT_location: (location of the pointer) 
<1><BBB> DW_TAG_array_type
    <> DW_AT_type: <CCC>
    <> DW_AT_data_location: (location of the array)
<2><DDD> DW_TAG_subrange_type

When printing this pointer to array in gfortran/ifx the pointer print function is
not actually called - since the DWARF does not emit a pointer here.  Instead,
the normal array print is invoked which will use the DW_AT_data_location.
So we do not run into this same problem

> I'm curious why the code added here is not also needed in f-valprint.c.

The reason here is simply that Fortran does not have this implemented for
Itself.  Instead the fortran f_langugage inherits from language_defn (the all
languages base class) which simply delegates the value-print to 
c_value_print / so changing it there suffices.

> 
> > --- a/gdb/testsuite/gdb.cp/vla-cxx.exp
> > +++ b/gdb/testsuite/gdb.cp/vla-cxx.exp
> > @@ -23,6 +23,36 @@ if ![runto_main] {
> >      return -1
> >  }
> 
> > +gdb_breakpoint [gdb_get_line_number "Before pointer assignment"]
> > +gdb_continue_to_breakpoint "Before pointer assignment"
> > +
> > +set test_name "ptype ptr, Before pointer assignment"
> > +gdb_test_multiple "ptype ptr" $test_name {
> > +  # gfortran
> > +  -re "= int \\(\\*\\)\\\[variable length\\\]\r\n$gdb_prompt $" {
> > +    pass $test_name
> > +  }
> > +  # ifort/ifx
> 
> This references fortran compilers but is a c++ test.
> 

Thanks, good spot - corrected in v2.

> > +gdb_test "ptype ptr" "int \\(\\*\\)\\\[3\\\]"
> > +gdb_test "print ptr" "\\(int \\(\\*\\)\\\[3\\\]\\) $hex"
> > +gdb_test "print *ptr" " = \\{5, 7, 9\\}"
> 
> Do these pass with all compilers or do they also need to be conditional?
> 

These pass with all combinations I tried. The test has another failure for
ifort but this one is not related to the actual ptype but some issue in the
DWARF from ifort (print vlaref and print vlaref2 fail).

But actually I noticed an error on my side - in fact (and as pointed out above)
ifx and gfortran use similar DWARF to describe the above compiled test
while ifort differs.  So, the two cases for the conditional tests would be
gfortran/ifx and ifort. I'll correct this in my second version.

Maybe a word about what I tested:
The compiler stacks I tested were gcc + gfortran, icc + icpc + ifort and
icx + icpx + ifx (here a non-released version with some more bugfixes).

My main focus laid on the GNU stack and here all test are passing. For any
testing with ifx I had to also apply a not-yet-upstreamed patch since ifx
uses a slightly different format for its emitted dwarf type name (so, e.g.
the $int for ifx is different from ifort and gfortran). I intend to upstream
this one soon to actually improve ifx pass rates.

All modified test are passing for the GNU and any of the Intel stacks. For
GNU I could not detect new issues in the rest of the testsuite - for the
Intel stacks I only really tested the directly affected gdb.fortran and
gdb.cp parts of the testsuite.

I just rebased this patch to master but there were (nearly) no conflicts so I'd
want to resolve this discussion before sending a v2.

Thanks,
Nils
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] 10+ messages in thread

* Re: [PATCH 2/2] gdb: Resolve dynamic target types of pointers.
  2022-04-14 10:22     ` Kempke, Nils-Christian via Gdb-patches
@ 2022-04-15 16:14       ` Tom Tromey
  2022-04-19  6:59         ` Kempke, Nils-Christian via Gdb-patches
  0 siblings, 1 reply; 10+ messages in thread
From: Tom Tromey @ 2022-04-15 16:14 UTC (permalink / raw)
  To: Kempke, Nils-Christian via Gdb-patches; +Cc: Tom Tromey

> Thanks for the feedback. Sorry that it took me so long to reply - the mail
> somehow got lost..

No need to apologize!  I'm constantly months behind and/or dropping email.

>> > +	  if (type->code () == TYPE_CODE_PTR && is_dynamic_type (type))
>> > +	    {
>> > +	      CORE_ADDR addr;
>> > +	      if (nullptr != TYPE_DATA_LOCATION (TYPE_TARGET_TYPE (type)))
>> > +		addr = value_address (val);
>> > +	      else
>> > +		addr = value_as_address (val);
>> 
>> This seems weird to me.  When does value_as_address fail?

> This if else has been introduced because of the way ifort/icc describe pointers
> vs. gcc/gfortran and icx/ifx.

> <1><AAA> DW_TAG_variable
>     <> DW_AT_type: <BBB>
>     <> DW_AT_location: (location of the pointer)
> <1><BBB> DW_TAG_pointer_type
>     DW_AT_type: <CCC>
> <1><CCC> DW_TAG_array_type
>     <> DW_AT_type: <DDD>
>     <> DW_AT_data_location: (location of the array)
> <2><EEE> DW_TAG_subrange_type

> Fortran:

> <1><AAA> DW_TAG_variable
>     <>   DW_AT_type: <BBB>
>     <>   DW_AT_location: (location of the pointer) 
> <1><BBB> DW_TAG_array_type
>     <> DW_AT_type: <CCC>
>     <> DW_AT_data_location: (location of the array)
> <2><DDD> DW_TAG_subrange_type

> When printing this pointer to array in gfortran/ifx the pointer print function is
> not actually called - since the DWARF does not emit a pointer here.  Instead,
> the normal array print is invoked which will use the DW_AT_data_location.
> So we do not run into this same problem

I still do not understand, sorry.

For me this looks like two different types.  In one situation, the type
is a pointer to an array.  In the second, it's just an array.  So, I
would expect them to print differently.

Are you saying these should print identically?  If so, that's fine --
but then it seems like something to do in the Fortran code, not in
c-valprint.c.

Maybe in the code the check for TYPE_DATA_LOCATION is a proxy for
checking that the pointer is a pointer to a Fortran array?  I guess I
could understand that, but it doesn't seem like a good approach, because
what if some C compiler starts emitting this?

>> I'm curious why the code added here is not also needed in f-valprint.c.

> The reason here is simply that Fortran does not have this implemented for
> Itself.  Instead the fortran f_langugage inherits from language_defn (the all
> languages base class) which simply delegates the value-print to 
> c_value_print / so changing it there suffices.

If Fortran-specific printing is needed, this will have to change.

Tom

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

* RE: [PATCH 2/2] gdb: Resolve dynamic target types of pointers.
  2022-04-15 16:14       ` Tom Tromey
@ 2022-04-19  6:59         ` Kempke, Nils-Christian via Gdb-patches
  2022-05-13 14:45           ` Kempke, Nils-Christian via Gdb-patches
  0 siblings, 1 reply; 10+ messages in thread
From: Kempke, Nils-Christian via Gdb-patches @ 2022-04-19  6:59 UTC (permalink / raw)
  To: Tom Tromey, Kempke, Nils-Christian via Gdb-patches



> -----Original Message-----
> From: Tom Tromey <tom@tromey.com>
> Sent: Friday, April 15, 2022 6:14 PM
> To: Kempke, Nils-Christian via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Tom Tromey <tom@tromey.com>; Kempke, Nils-Christian <nils-
> christian.kempke@intel.com>
> Subject: Re: [PATCH 2/2] gdb: Resolve dynamic target types of pointers.
> 
> > Thanks for the feedback. Sorry that it took me so long to reply - the mail
> > somehow got lost..
> 
> No need to apologize!  I'm constantly months behind and/or dropping email.
> 
> >> > +	  if (type->code () == TYPE_CODE_PTR && is_dynamic_type (type))
> >> > +	    {
> >> > +	      CORE_ADDR addr;
> >> > +	      if (nullptr != TYPE_DATA_LOCATION (TYPE_TARGET_TYPE (type)))
> >> > +		addr = value_address (val);
> >> > +	      else
> >> > +		addr = value_as_address (val);
> >>
> >> This seems weird to me.  When does value_as_address fail?
> 
> > This if else has been introduced because of the way ifort/icc describe
> pointers
> > vs. gcc/gfortran and icx/ifx.
> 
> > <1><AAA> DW_TAG_variable
> >     <> DW_AT_type: <BBB>
> >     <> DW_AT_location: (location of the pointer)
> > <1><BBB> DW_TAG_pointer_type
> >     DW_AT_type: <CCC>
> > <1><CCC> DW_TAG_array_type
> >     <> DW_AT_type: <DDD>
> >     <> DW_AT_data_location: (location of the array)
> > <2><EEE> DW_TAG_subrange_type
> 
> > Fortran:
> 
> > <1><AAA> DW_TAG_variable
> >     <>   DW_AT_type: <BBB>
> >     <>   DW_AT_location: (location of the pointer)
> > <1><BBB> DW_TAG_array_type
> >     <> DW_AT_type: <CCC>
> >     <> DW_AT_data_location: (location of the array)
> > <2><DDD> DW_TAG_subrange_type
> 
> > When printing this pointer to array in gfortran/ifx the pointer print function
> is
> > not actually called - since the DWARF does not emit a pointer here.
> Instead,
> > the normal array print is invoked which will use the DW_AT_data_location.
> > So we do not run into this same problem
> 
> I still do not understand, sorry.
> 
> For me this looks like two different types.  In one situation, the type
> is a pointer to an array.  In the second, it's just an array.  So, I
> would expect them to print differently.
> 
> Are you saying these should print identically?  If so, that's fine --
> but then it seems like something to do in the Fortran code, not in
> c-valprint.c.

Yes, that is true. So, in Fortran pointers are just aliases for the underlying objects
 
> Maybe in the code the check for TYPE_DATA_LOCATION is a proxy for
> checking that the pointer is a pointer to a Fortran array?  I guess I
> could understand that, but it doesn't seem like a good approach, because
> what if some C compiler starts emitting this?
> 
> >> I'm curious why the code added here is not also needed in f-valprint.c.
> 
> > The reason here is simply that Fortran does not have this implemented for
> > Itself.  Instead the fortran f_langugage inherits from language_defn (the all
> > languages base class) which simply delegates the value-print to
> > c_value_print / so changing it there suffices.
> 
> If Fortran-specific printing is needed, this will have to change.
> 
> 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] 10+ messages in thread

* RE: [PATCH 2/2] gdb: Resolve dynamic target types of pointers.
  2022-04-19  6:59         ` Kempke, Nils-Christian via Gdb-patches
@ 2022-05-13 14:45           ` Kempke, Nils-Christian via Gdb-patches
  0 siblings, 0 replies; 10+ messages in thread
From: Kempke, Nils-Christian via Gdb-patches @ 2022-05-13 14:45 UTC (permalink / raw)
  To: Kempke, Nils-Christian, Tom Tromey, Kempke,
	Nils-Christian via Gdb-patches

Hi,

Sorry for this first message, something went wrong there..
Again.

First of all, thanks for the feedback!

> -----Original Message-----
> From: Gdb-patches <gdb-patches-bounces+nils-
> christian.kempke=intel.com@sourceware.org> On Behalf Of Kempke, Nils-
> Christian via Gdb-patches
> Sent: Tuesday, April 19, 2022 8:59 AM
> To: Tom Tromey <tom@tromey.com>; Kempke, Nils-Christian via Gdb-
> patches <gdb-patches@sourceware.org>
> Subject: RE: [PATCH 2/2] gdb: Resolve dynamic target types of pointers.
> 
> >
> > >> > +	  if (type->code () == TYPE_CODE_PTR && is_dynamic_type
> (type))
> > >> > +	    {
> > >> > +	      CORE_ADDR addr;
> > >> > +	      if (nullptr != TYPE_DATA_LOCATION (TYPE_TARGET_TYPE
> (type)))
> > >> > +		addr = value_address (val);
> > >> > +	      else
> > >> > +		addr = value_as_address (val);
> > >>
> > >> This seems weird to me.  When does value_as_address fail?
> >
> > > This if else has been introduced because of the way ifort/icc describe
> > pointers
> > > vs. gcc/gfortran and icx/ifx.
> >
> > > <1><AAA> DW_TAG_variable
> > >     <> DW_AT_type: <BBB>
> > >     <> DW_AT_location: (location of the pointer)
> > > <1><BBB> DW_TAG_pointer_type
> > >     DW_AT_type: <CCC>
> > > <1><CCC> DW_TAG_array_type
> > >     <> DW_AT_type: <DDD>
> > >     <> DW_AT_data_location: (location of the array)
> > > <2><EEE> DW_TAG_subrange_type
> >
> > > Fortran:
> >
> > > <1><AAA> DW_TAG_variable
> > >     <>   DW_AT_type: <BBB>
> > >     <>   DW_AT_location: (location of the pointer)
> > > <1><BBB> DW_TAG_array_type
> > >     <> DW_AT_type: <CCC>
> > >     <> DW_AT_data_location: (location of the array)
> > > <2><DDD> DW_TAG_subrange_type
> >
> > > When printing this pointer to array in gfortran/ifx the pointer print
> function
> > is
> > > not actually called - since the DWARF does not emit a pointer here.
> > Instead,
> > > the normal array print is invoked which will use the
> DW_AT_data_location.
> > > So we do not run into this same problem
> >
> > I still do not understand, sorry.
> >
> > For me this looks like two different types.  In one situation, the type
> > is a pointer to an array.  In the second, it's just an array.  So, I
> > would expect them to print differently.
> >
> > Are you saying these should print identically?  If so, that's fine --
> > but then it seems like something to do in the Fortran code, not in
> > c-valprint.c.
> 
> Yes, that is true. So, in Fortran pointers are just aliases for the underlying
> objects

First about the pointers in general:

They are emitted as two different types but represent the same object in
the code. The fact that ifort emits pointer-to-array here is, I think, less
ideal than a plain array type.
In Fortran a pointer is more like an alias to the underlying object.  It should be
thought of as a type with the pointer attribute, rather than a c pointer. It is
handled the exact same way the as object itself.  E.g. printing an associated
pointer-to-array would just print the array - as if one had printed that
instead. There is some hidden dereferencing going on when using them
(and as compared To C pointers) - so yes, imo ideally the print should be
the same.

But this would not be addressed with the sent patch.  The changes in the
patch only affect the resolution of the target dynamic type (thus the cases
in the tests).

I think the explanation I gave above was quite bad as well and in my initial
submission of this patch I am not sure whether I had 100% understood this
part as I do think of it now.  I will try again:

In GDB when we end up at the point where the dynamic type is to be resolved,
the address we will get from

   CORE_ADDR addr = value_as_address (val);

will, if the underlying type had a DW_AT_data_location, be the result of
the evaluation of that DW_AT_data_location.  This is fine for pure arrays,
but with pointer to arrays this is problematic.  The reason for this is, as described
in the DWARF5 5.18.1:

1  5.18.1 Data location
   ...
5  The DW_AT_data_location attribute may be used with any type that provides
6  one or more levels of hidden indirection and/or run-time parameters in its
7  representation. ...
   ...
10 This location description will typically begin with DW_OP_push_object_address which
11 loads the address of the object which can then serve as a descriptor in subsequent
12 calculation. For an example using DW_AT_data_location for a Fortran 90 array, see
13 Appendix D.2.1 on page 292.

(the example on 292 I think is quite helpful)

In the example on page 292, not only the DW_TAG_data_location but also
the DW_TAG_subrange_type require the evaluation of an expression containing
the DW_OP_push_object_address.  The object address that should be used for
these evaluations is the one of the original variable, in the example the variable
'arrayvar', which has its own DW_AT_location attribute.  It is especially not the
result of the array type's DW_AT_data_location. 

The example describes this as

Page 295:
   7 For b), for each dimension of the array (only one in this case), go interpret the
   8 usual lower bound attribute. Again this is an expression, which again begins
   9 with DW_OP_push_object_address. This object is still arrayvar, from step 1,
   10 because we have not begun to actually perform any indexing yet

where b) is evaluation of the lower bounds of the array. Here the 'still arrayvar'
tells to use the DW_AT_location.

Going back to ifort:

<1><AAA> DW_TAG_variable
  <> DW_AT_type: <BBB>
  <> DW_AT_location: (location of the pointer)
<1><BBB> DW_TAG_pointer_type
  DW_AT_type: <CCC>
<1><CCC> DW_TAG_array_type
  <> DW_AT_type: <DDD>
  <> DW_AT_data_location: (location of the array using DW_OP_push_object_address)
  <> DW_AT_allocated: (using DW_OP_push_object_address) 
<2><EEE> DW_TAG_subrange_type
  <> DW_AT_TYPE: <FFF>
  <> DW_AT_lower_bound (using DW_OP_push_object_address)
  <> DW_AT_upper_bound (using DW_OP_push_object_address)

With such a location description we'd not be able to correctly evaluate the
DW_TAG_subrange_type as GDB would try to use the DW_AT_data_location
(gotten when evaluating value_as_address) instead of the DW_AT_location of
the original variable (which would be value_address (..)).  Thus, when we see
that the underlying type has a DW_AT_data_location we use the value_address
instead the value_as_address here.

I don't think this is a perfect solution really.. But I am not sure how to handle this
better atm.  I would be ok with removing this check from the patch for now, and
just always use value_as_address.. This part though for sure would need a way
better commit message I feel like.
Ifx, gfortran and flang do not emit these kind of types as far as I know (removing
the check for DW_AT_data_location will also only mess with the icc/ifort passrate
of the tests).  I guess, in order to properly resolve a target type of a pointer one
would have to got through all its dynamic attributes that are about to be resolved
and check, whether these need the address of the object or not.

> > Maybe in the code the check for TYPE_DATA_LOCATION is a proxy for
> > checking that the pointer is a pointer to a Fortran array?  I guess I
> > could understand that, but it doesn't seem like a good approach, because
> > what if some C compiler starts emitting this?

If a C compiler would start emitting this, I think we would also get problems
when trying to resolve the dynamic target type - as the location of the actual array
is not the location of the pointer. But that also assumes again that using
DW_AT_data_location for the target type will in general come with using
DW_ OP_push_object_address for the target type, too.

So, this checking is somehow a proxy, but for realizing that the resolving the
target type will need the object_address of the original pointer.
 
> > >> I'm curious why the code added here is not also needed in f-valprint.c.
> >
> > > The reason here is simply that Fortran does not have this implemented
> for
> > > Itself.  Instead the fortran f_langugage inherits from language_defn (the
> all
> > > languages base class) which simply delegates the value-print to
> > > c_value_print / so changing it there suffices.
> >
> > If Fortran-specific printing is needed, this will have to change.

That is true. Dereferencing pointers before printing them would be such a thing,
but I think handling the pointer-to-array DWARF construct above should apply to
more than just the ifort Fortran pointers - any compiler could start emitting the same
construct and we'd get problems at the same spots when trying to resolve the types.
At least I don't see how DWARF spec would stand in the way of compilers emitting
this.
Currently I only know of ifort emitting this construct where the resolving of the
subrange upper and lower bounds actually requires the original pointer address.
I know that icx also would emit pointers to arrays in the same way ifort does, but
here, the subrange (afaik) does not depend on the object address and does not
use DW_OP_push_object_address.

So overall, this currently only happens with ifrot and in Fortran.

Sorry for the bad first explanation.

Thanks!
Nils
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] 10+ messages in thread

end of thread, other threads:[~2022-05-13 14:46 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-18 13:26 [PATCH 0/2] Resolve dynamic types for pointers Nils-Christian Kempke via Gdb-patches
2022-01-18 13:26 ` [PATCH 1/2] gdb/types: Resolve dynamic properties of pointer types Nils-Christian Kempke via Gdb-patches
2022-01-28 18:30   ` Tom Tromey
2022-01-31  8:15     ` Kempke, Nils-Christian via Gdb-patches
2022-01-18 13:26 ` [PATCH 2/2] gdb: Resolve dynamic target types of pointers Nils-Christian Kempke via Gdb-patches
2022-02-10 19:47   ` Tom Tromey
2022-04-14 10:22     ` Kempke, Nils-Christian via Gdb-patches
2022-04-15 16:14       ` Tom Tromey
2022-04-19  6:59         ` Kempke, Nils-Christian via Gdb-patches
2022-05-13 14:45           ` Kempke, Nils-Christian via Gdb-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox