public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Simon Marchi <simark@simark.ca>
To: "Zaric, Zoran (Zare)" <Zoran.Zaric@amd.com>,
	"Kempke, Nils-Christian" <nils-christian.kempke@intel.com>,
	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Cc: "tom@tromey.com" <tom@tromey.com>
Subject: Re: [PATCH 3/4] gdb, typeprint: fix pointer/reference typeprint for icc/ifort
Date: Tue, 27 Sep 2022 08:48:10 -0400	[thread overview]
Message-ID: <55f9d383-76ff-4e2c-b5fe-495714bcbb8c@simark.ca> (raw)
In-Reply-To: <BN9PR12MB50653AC3FA2F8EF680A2106AF8559@BN9PR12MB5065.namprd12.prod.outlook.com>


> The key difference here is that the DW_TAG_reference_type is a "standalone" type and there is always an object or a member (a physical location description) behind it while the DW_TAG_subrange_type is a by definition tied to a "standalone" array types. Unfortunately, it is not really meaningful to have an object of a DW_TAG_subrange_type so the type is more there to allow factorizing of the debug information.
> 
> In the case of DW_TAG_reference_type, there is always an object that is the reference to something and if you dereference it, you get another object that the original object referenced, which is a very different case then the DW_TAG_subrange_type and the array type that references it.
> 
> That being said, I don't believe that there is anywhere in DWARF stating that if there is an actual object of a DW_TAG_subrange_type type that the DW_OP_push_object_address operation (in any of the attributes of that type) should not use it, it just that the way how this type is defined doesn't really allow you to have a separate object of that type.

Well, you can have a standaline DW_TAG_subrange_type object, in
languages where you can define types to be subranges of other types.  I
just tried this Ada example:

  https://learn.adacore.com/courses/intro-to-ada/chapters/strongly_typed_language.html#subtypes

And I got this, a variable with the subrange as its type:

0x00001a08:   DW_TAG_subprogram
                DW_AT_name [DW_FORM_strp]       ("greet")

0x00001a63:     DW_TAG_subrange_type
                  DW_AT_lower_bound [DW_FORM_data1]     (0x05)
                  DW_AT_upper_bound [DW_FORM_data1]     (0x06)
                  DW_AT_name [DW_FORM_strp]     ("greet__weekend_days")
                  DW_AT_type [DW_FORM_ref4]     (0x00001a2a "greet__days")

0x00001a7b:     DW_TAG_variable
                  DW_AT_name [DW_FORM_string]   ("s")
                  DW_AT_type [DW_FORM_ref4]     (0x00001a63 "greet__weekend_days")
                  DW_AT_location [DW_FORM_exprloc]      (DW_OP_fbreg -125)


> At the end of the day, the definition of DW_OP_push_object_address operation just states that the operation pushes the address of the object currently being evaluated, It even states that it could be a component of an object, but it still has to be something tangible (a real location).

I think that the confusion comes from the fact that DW_TAG_subrange_type
can serve two different and unrelated purposes.  One as a standard
standalone type, as shown above.  And two, as a direct child of a
DW_TAG_array_type, used to express the bounds.  Section 5.5 - Array Type
Entries says:

    Each array dimension is described by a debugging information entry
    with either the tag DW_TAG_subrange_type or the tag
    DW_TAG_enumeration_type. These entries are children of the array
    type entry and are ordered to reflect the appearance of the
    dimensions in the source program (that is, leftmost dimension first,
    next to leftmost second, and so on).

My interpretation of this text is that it describes that second special
use case where DW_TAG_subrange_type describes the bounds of the array,
but does not represent a concrete object itself, as it would with its
first purpose.  As Zoran said, maybe this helped "factorize" the debug
information and make the DWARF committee write less text, but I think it
also adds confusion.  I think it should be interpreted as if the
DW_TAG_array_type had the DW_AT_{lower,upper}_bound attributes directly.
I think this is what is intended by the standard, looking at the D.2.1
example.  The comments on the DW_AT_{lower,upper}_bound expressions show
that they consider DW_OP_push_object_address to have pushed the array's
address.

That said, assuming that this is indeed "broken" DWARF, icc/ifort
produce what they produce, and I am open to make GDB handle it.  We work
around broken DWARF all the time for different compilers.  However, the
commit message would need to explain this, the code should make it clear
that this is a workaround for a specific form of DWARF emitted by
icc/ifort, and ideally gated behind a compiler check.  This way, in 20
years, when we decide we no longer want to support 20 year old hacks,
it's easy to identify and remove them.  It would also be good to have a
gdb.dwarf2 test case for this, explaining that "this is not really valid
DWARF but we want to support it anyway".

Simon

  reply	other threads:[~2022-09-27 12:48 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 ` [PATCH 3/4] gdb, typeprint: fix pointer/reference typeprint for icc/ifort Nils-Christian Kempke
2022-09-26 16:02   ` 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 [this message]
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=55f9d383-76ff-4e2c-b5fe-495714bcbb8c@simark.ca \
    --to=simark@simark.ca \
    --cc=Zoran.Zaric@amd.com \
    --cc=gdb-patches@sourceware.org \
    --cc=nils-christian.kempke@intel.com \
    --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).