public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "vries at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug debug/94450] lto abstract variable emitted as concrete decl
Date: Thu, 02 Apr 2020 12:29:10 +0000	[thread overview]
Message-ID: <bug-94450-4-dqcKiIiCKI@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-94450-4@http.gcc.gnu.org/bugzilla/>

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94450

--- Comment #5 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #1)
> I guess the more correct DWARF would be to have the 13d DIE include
> DW_AT_declaration?

Well, currently the debug info contains two concrete symbols, one with and one
without location information. The things that makes the latter symbol concrete
are both the fact that it's contained in a CU (as opposed to in a PU), as well
as that it's imported into another CU (in fact, one could make an argument that
in fact three concrete symbols are present, but let's not go there). So, if
we'd mark the one without location information as declaration we'd still have
two concrete symbols.

It could be pedantically argued that tagging the symbol as declaration is
incorrect because there's in fact no declaration in the source that it
corresponds to. That could be fixed by marking the declaration with
DW_AT_artificial == 1 (and perhaps marking the def with DW_AT_artificial == 0
in order to make sure the artificial setting is not inherited, in case we go
the DW_AT_specification route). Btw, the dwarf5 standard lists DW_AT_artificial
as applicable to DW_TAG_variable, and the dwarf4 standard doesn't.  I'm not
sure yet whether that reflects improved documentation or an actual change.

But indeed, marking it as declaration would make the situation resemble more
non-lto code (for the case where the source has indeed both a decl and def).

I wonder even if the DW_AT_artificial marking itself (irrespective of a
possible DW_AT_declaration) is used or could be used in gdb to fix PR
gdb/25760.  I'll have to mock up a gdb testsuite dwarf assembly test-case
resembling the test-case and experiment a bit to see what works, and whether
gdb needs changes.

Anyway, the point I was trying to make is that the easiest way to make decls
abstract (rather than adding stuff to the decl itself), is by making the decl
not a top-level member of CU, in other words: declare it in a PU, and don't
import it into another CU.

> Then we could also stop the "abuse" of
> DW_AT_abstract_origin
> and instead have to use DW_AT_specification.  But I'm not sure whether
> DW_AT_specification allows cross CU references (technically yes but
> practically) especially since there's explicit wording that
> DW_AT_specification
> cannot refer to type unit entities.
>

Using DW_AT_specification sounds cleaner, agreed.

> Note I originally saw all early debug as abstract (but we're not consistently
> emitting DW_AT_inline to all early function DIEs either) but that concept
> doesn't apply to globals.
> 
> As you said the DW_TAG_imported_unit serve no useful purpose (I originally
> thought that it would provide proper name-lookup scopes but that works
> correct in other ways).  And I'm fine to simply drop those (also given
> consumers seem to handle references to CUs not explicitely imported just
> fine).  That could be done for GCC 10 already, I fear the rest needs more
> testing?
> 

Yeah, I think the part of dropping the imports should be safe, and the rest
should be decided once we have more info from playing with the above-mentioned
mockup example.

> Btw, thanks for sanity checking the LTO DWARF.

Sure. I'm working on trying to improve gdb speed for lto executables, and in
order to test gdb patches I need to regression test in lto mode, where I do run
into regressions, which need to be analyzed, and that's how I'm running into
this sort of issues.

  parent reply	other threads:[~2020-04-02 12:29 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-01 21:51 [Bug debug/94450] New: " vries at gcc dot gnu.org
2020-04-02  8:07 ` [Bug debug/94450] " rguenth at gcc dot gnu.org
2020-04-02 10:17 ` rguenth at gcc dot gnu.org
2020-04-02 10:22 ` rguenth at gcc dot gnu.org
2020-04-02 10:25 ` rguenth at gcc dot gnu.org
2020-04-02 12:29 ` vries at gcc dot gnu.org [this message]
2020-04-02 14:48 ` cvs-commit at gcc dot gnu.org
2020-04-02 14:48 ` rguenth at gcc dot gnu.org
2020-04-03 10:26 ` vries at gcc dot gnu.org
2020-04-03 11:10 ` rguenther at suse dot de
2020-04-03 12:52 ` vries at gcc dot gnu.org
2020-04-03 13:03 ` rguenther at suse dot de
2020-04-03 13:23 ` vries at gcc dot gnu.org
2020-04-03 16:43 ` vries at gcc dot gnu.org

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=bug-94450-4-dqcKiIiCKI@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.org \
    /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).