public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Nils-Christian Kempke <nils-christian.kempke@intel.com>
To: gdb-patches@sourceware.org
Cc: tom@tromey.com, Nils-Christian Kempke <nils-christian.kempke@intel.com>
Subject: [PATCH 3/4] gdb, typeprint: fix pointer/reference typeprint for icc/ifort
Date: Tue, 20 Sep 2022 09:26:28 +0200	[thread overview]
Message-ID: <20220920072629.2736207-4-nils-christian.kempke@intel.com> (raw)
In-Reply-To: <20220920072629.2736207-1-nils-christian.kempke@intel.com>

Intel classic compilers (icc/icpc/ifort) for references and 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_type)
    <215>   DW_AT_type        : <0x219>
 <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

(for pointers replace the DW_TAG_reference_type with a
DW_TAG_pointer_type).  This is, to my knowledge, allowed and corret DWARF
but posed a problem in GDB.  Usually, GDB would deal with gcc references
(or pointers) that look like

 <2><96>: Abbrev Number: 8 (DW_TAG_variable)
    <97>   DW_AT_location    : 2 byte block: 91 50
      (DW_OP_fbreg: -48)
    <9a>   DW_AT_name        : (indirect string, offset: 0x2aa): vlaref
    <9e>   DW_AT_decl_file   : 3
    <9f>   DW_AT_decl_line   : 41
    <a0>   DW_AT_type        : <0x1de>
 ...
 <1><1de>: Abbrev Number: 17 (DW_TAG_reference_type)
    <1df>   DW_AT_type        : <0x1e3>
 <1><1e3>: Abbrev Number: 22 (DW_TAG_array_type)
    <1e4>   DW_AT_type        : <0x1bf>
 <2><1e8>: Abbrev Number: 23 (DW_TAG_subrange_type)
    <1e9>   DW_AT_type        : <0x1f2>
    <1ed>   DW_AT_count       : <0x8a>
 <2><1f1>: Abbrev Number: 0

The DWARF above describes a reference with an address.  At the address
of this reference, so evaluating what lies at the DW_AT_location of the
DW_TAG_variable, we find the address we need to use to resolve the
array under the reference and print the array's values.
This is also exactly what GDB does per default when printing a
reference-to-array type.  It evaluates the reference, and then takes that
address to resolve the array (implicitly assuming that the value of the
reference coincides with the address of the array).

The difference to the icc output is icc's usage of DW_AT_data_location.
This attribute can be used to specify a location for the actual data of
an array that is different from the object's address.  If it is present
in addition to a DW_AT_location (as is the case above) the array values
are actually located at whatever DW_AT_data_location is evaluated to
and not at the address DW_AT_location of the variable points to.

When dealing with the icc output this posed a problem in GDB.  It would
still evaluate the reference.  It would then forget all about the reference
and use the address obtained that way to resolve the array type (which,
as mentioned, works fine with the gcc output).  It would then discover the
DW_AT_data_location attribute in the array's DIE and it would try to
resolve it.
Here is where GDB ran into a problem: according to DWARF5 and as seen in
this example as well, DW_AT_data_location usually starts with a
DW_OP_push_object_address.  This operation pushes the address of the
object currently being evaluated on the stack.  The object currently being
evaluated however in this case is the reference, not the
array.  But as GDB already evaluated the reference and as it would only
know about the array's address anymore it would use that address in order
to resolve the DW_AT_data_location.  This failed and GDB could not print
any references emitted that way, as it would usually try to access illegal
memory addresses.

GDB would usually display

  (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.

The problem is the exact same when dealing with pointers emitted by the
Intel classic compilers.

For resolving this problem, we make GDB check whether the pointer's
underlying type shows a DW_AT_data_location, and, if so, we assume
that, in order to resolve the type, not the value of the pointer (so the
value at DW_AT_location of the pointer) but the address of the pointer
(so the acutal value of DW_AT_location) is be required.

This patch implements the the fix for references and pointers.

With this patch the above example prints as

  (gdb) print vlaref
  $1 = (int (&)[3]) @0x7fffffffc4e0: {5, 7, 9}
---
 gdb/gdbtypes.c | 24 ++++++++++++++++++++++++
 gdb/valprint.c | 18 ++++++++++++++++--
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index a4d79a64e95..ce48b968f4b 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2809,6 +2809,18 @@ resolve_dynamic_pointer (struct type *type,
   if (addr_stack->valaddr.data () != NULL)
     pinfo.addr = extract_typed_address (addr_stack->valaddr.data (),
 					type);
+  else if (TYPE_DATA_LOCATION (TYPE_TARGET_TYPE (type)) != nullptr)
+    {
+      /* If the underlying type has a DW_AT_data_location attribute
+	 we assume that we need to take the reference's address (the
+	 value of its DW_AT_location) to resolve the type instead
+	 of the reference's value (the dereferenced DW_AT_location).
+	 This is because DW_AT_data_location usually uses
+	 DW_OP_push_obj_address which pushes the address of the
+	 currently observed object on the stack.  Here, this is the
+	 reference itself and not its underlying type.  */
+      pinfo.addr = addr_stack->addr;
+    }
   else
     pinfo.addr = read_memory_typed_address (addr_stack->addr, type);
   pinfo.next = addr_stack;
@@ -2874,6 +2886,18 @@ resolve_dynamic_type_internal (struct type *type,
 	    if (addr_stack->valaddr.data () != NULL)
 	      pinfo.addr = extract_typed_address (addr_stack->valaddr.data (),
 						  type);
+	    else if (TYPE_DATA_LOCATION (TYPE_TARGET_TYPE (type)) != nullptr)
+	      {
+		/* If the underlying type has a DW_AT_data_location attribute
+		   we assume that we need to take the reference's address (the
+		   value of its DW_AT_location) to resolve the type instead
+		   of the reference's value (the dereferenced DW_AT_location).
+		   This is because DW_AT_data_location usually uses
+		   DW_OP_push_obj_address which pushes the address of the
+		   currently observed object on the stack.  Here, this is the
+		   reference itself and not its underlying type.  */
+		pinfo.addr = addr_stack->addr;
+	      }
 	    else
 	      pinfo.addr = read_memory_typed_address (addr_stack->addr, type);
 	    pinfo.next = addr_stack;
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 0c3066602fe..7fee691cae0 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -565,8 +565,22 @@ generic_val_print_ref (struct type *type,
 	  gdb_assert (embedded_offset == 0);
 	}
       else
-	deref_val = value_at (TYPE_TARGET_TYPE (type),
-			      unpack_pointer (type, valaddr + embedded_offset));
+      {
+	/* If the underlying type has a DW_AT_data_location attribute we need
+	   to take the reference's address (so the value of its DW_AT_location)
+	   to resolve the type instead of the references value (the
+	   dereferenced DW_AT_location).  This is because
+	   DW_AT_data_location uses DW_OP_push_obj_address which pushes the
+	   address of the currently observed object on the stack, which is the
+	   reference itself here and not its underlying type.  */
+	if (TYPE_DATA_LOCATION (TYPE_TARGET_TYPE (type)) != nullptr)
+	  deref_val = value_at (TYPE_TARGET_TYPE (type),
+				value_address (original_value));
+	else
+	  deref_val = value_at (TYPE_TARGET_TYPE (type),
+				unpack_pointer (type,
+						valaddr + embedded_offset));
+      }
     }
   /* Else, original_value isn't a synthetic reference or we don't have to print
      the reference's contents.
-- 
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


  parent reply	other threads:[~2022-09-20  7:26 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-20  7:26 [PATCH 0/4] Dynamic properties of pointers Nils-Christian Kempke
2022-09-20  7:26 ` [PATCH 1/4] gdb, testsuite: handle icc and icpc deprecated remarks Nils-Christian Kempke
2022-09-26 14:32   ` Simon Marchi
2022-09-20  7:26 ` [PATCH 2/4] gdb/types: Resolve pointer types dynamically Nils-Christian Kempke
2022-09-26 15:33   ` Simon Marchi
2022-09-29 12:39     ` Kempke, Nils-Christian
2022-09-20  7:26 ` Nils-Christian Kempke [this message]
2022-09-26 16:02   ` [PATCH 3/4] gdb, typeprint: fix pointer/reference typeprint for icc/ifort Simon Marchi
2022-09-26 17:18     ` Kempke, Nils-Christian
2022-09-27  9:14       ` Zaric, Zoran (Zare)
2022-09-27 12:48         ` Simon Marchi
2022-09-20  7:26 ` [PATCH 4/4] gdb/fortran: Fix sizeof intrinsic for Fortran Nils-Christian Kempke
2022-09-26 17:06   ` Simon Marchi
2022-09-26 17:22     ` Kempke, Nils-Christian
2022-09-26 17:24     ` Kempke, Nils-Christian

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220920072629.2736207-4-nils-christian.kempke@intel.com \
    --to=nils-christian.kempke@intel.com \
    --cc=gdb-patches@sourceware.org \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).