public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug debug/94450] New: lto abstract variable emitted as concrete decl
@ 2020-04-01 21:51 vries at gcc dot gnu.org
  2020-04-02  8:07 ` [Bug debug/94450] " rguenth at gcc dot gnu.org
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: vries at gcc dot gnu.org @ 2020-04-01 21:51 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 94450
           Summary: lto abstract variable emitted as concrete decl
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: debug
          Assignee: unassigned at gcc dot gnu.org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

Consider test-case test.c:
...
int aaa;

int
main (void)
{
  return aaa;
}
...

Compiled with -flto:
...
$ gcc-10 -O0 test.c -g -flto -flto-partition=none -ffat-lto-objects
...

The debug info for the variable aaa looks like this:
...
 <0><d2>: Abbrev Number: 1 (DW_TAG_compile_unit)
    <d8>   DW_AT_name        : <artificial>
 <1><f4>: Abbrev Number: 2 (DW_TAG_imported_unit)
    <f5>   DW_AT_import      : <0x12b>  [Abbrev Number: 1]
 <1><f9>: Abbrev Number: 3 (DW_TAG_variable)
    <fa>   DW_AT_abstract_origin: <0x13d>
    <fe>   DW_AT_location    : 9 byte block: 3 2c 10 60 0 0 0 0 0      
(DW_OP_addr: 60102c)
 <0><12b>: Abbrev Number: 1 (DW_TAG_compile_unit)
    <131>   DW_AT_name        : test.c
 <1><13d>: Abbrev Number: 2 (DW_TAG_variable)
    <13e>   DW_AT_name        : aaa
    <142>   DW_AT_decl_file   : 1
    <143>   DW_AT_decl_line   : 1
    <144>   DW_AT_decl_column : 5
    <145>   DW_AT_type        : <0x149>
    <149>   DW_AT_external    : 1
...

When printing the symbol tables in gdb:
...
$ gdb -readnow -batch a.out -ex "maint print symbols"
...
it turns out we have two symbols aaa, one here:
...
Symtab for file test.c at 0x23aeeb0
Compilation directory is /home/vries
Read from object file /home/vries/a.out (0x238cc90)
Language: c

Blockvector:

block #000, object at 0x23af030, 1 syms/buckets in 0x0..0x0
 int aaa; unresolved
  block #001, object at 0x23aef80 under 0x23af030, 1 syms/buckets in 0x0..0x0
   typedef int int; 

Compunit user: 0x23aedf0
...
and one here:
...
Symtab for file <artificial> at 0x23aedf0
Compilation directory is /home/vries
Read from object file /home/vries/a.out (0x238cc90)
Language: c

Blockvector:

block #000, object at 0x23aecb0, 1 syms/buckets in 0x400492..0x40049e
 int aaa; static at 0x60102c section .bss
 int main(void); block object 0x23aebf0, 0x400492..0x40049e section .text
  block #001, object at 0x23aec50 under 0x23aecb0, 0 syms/buckets in
0x400492..0x40049e
    block #002, object at 0x23aebf0 under 0x23aec50, 0 syms/buckets in
0x400492..0x40049e, function main

Compunit include: 0x23aeeb0
...

If we do "print aaa" in gdb and gdb finds the 'static' aaa symbol first, it
uses the DW_AT_location to find the address of the variable.

If we do "print aaa" in gdb and gdb finds the 'unresolved' aaa symbol first, it
looks in the minimal symbols for a symbol aaa and uses that address.

In both cases, we'd find the same address and print the same value, so there's
no correctness problem for this example.

But with ada, we run into PR gdb/25760 - "[gcc -flto] FAIL:
gdb.ada/call_pn.exp: print last_node_id after calling pn (timeout)", which
means that there is a correctness problem.

It's an idea to try to ignore these useless decls in gdb (filed as PR gdb/25759
- "Remove useless decls from symtab"), but I'm not sure yet how easy it is to
do this efficiently.

So, it would be good if gcc could make it explicit in the DWARF that there's
only one symbol to be considered, rather than having gdb spent time to ignore
the abstract one.

ISTM the only way to do this is to make the test.c CU a partial unit (using
DW_TAG_partial_unit) and drop the import.

[ Having said that, I'm not sure that gdb in its current state would correctly
interpret such dwarf and only create one symbol, so that might require an
additional gdb fix. ]

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

* [Bug debug/94450] lto abstract variable emitted as concrete decl
  2020-04-01 21:51 [Bug debug/94450] New: lto abstract variable emitted as concrete decl vries at gcc dot gnu.org
@ 2020-04-02  8:07 ` rguenth at gcc dot gnu.org
  2020-04-02 10:17 ` rguenth at gcc dot gnu.org
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-04-02  8:07 UTC (permalink / raw)
  To: gcc-bugs

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2020-04-02
           Keywords|                            |lto
                 CC|                            |rguenth at gcc dot gnu.org
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
I guess the more correct DWARF would be to have the 13d DIE include
DW_AT_declaration?  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.

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?

Btw, thanks for sanity checking the LTO DWARF.

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

* [Bug debug/94450] lto abstract variable emitted as concrete decl
  2020-04-01 21:51 [Bug debug/94450] New: lto abstract variable emitted as concrete decl 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
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-04-02 10:17 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
So the current situation is similar to that of

static inline int foo(int i)
{
  static int j;
  j = i + 1;
  return j;
}

int bar(int i)
{
  return foo(i);
}

int baz(int i)
{
  return foo(i);
}

here we get

 <1><2d>: Abbrev Number: 2 (DW_TAG_subprogram)
    <2e>   DW_AT_name        : foo
    <32>   DW_AT_decl_file   : 1
    <33>   DW_AT_decl_line   : 1
    <34>   DW_AT_prototyped  : 1
    <34>   DW_AT_type        : <0x50>
    <38>   DW_AT_inline      : 3        (declared as inline and inlined)
    <39>   DW_AT_sibling     : <0x50>
...
 <2><46>: Abbrev Number: 4 (DW_TAG_variable)
    <47>   DW_AT_name        : j
    <49>   DW_AT_decl_file   : 1
    <4a>   DW_AT_decl_line   : 3
    <4b>   DW_AT_type        : <0x50>

...
 <1><57>: Abbrev Number: 6 (DW_TAG_subprogram)
    <58>   DW_AT_external    : 1
    <58>   DW_AT_name        : bar
...
 <2><83>: Abbrev Number: 8 (DW_TAG_inlined_subroutine)
    <84>   DW_AT_abstract_origin: <0x2d>
    <88>   DW_AT_low_pc      : 0x0
    <90>   DW_AT_high_pc     : 0x9
    <98>   DW_AT_call_file   : 1
    <99>   DW_AT_call_line   : 10
 <3><9a>: Abbrev Number: 9 (DW_TAG_formal_parameter)
    <9b>   DW_AT_abstract_origin: <0x3d>
    <9f>   DW_AT_location    : 1 byte block: 55         (DW_OP_reg5 (rdi))
 <3><a1>: Abbrev Number: 10 (DW_TAG_lexical_block)
    <a2>   DW_AT_low_pc      : 0x0
    <aa>   DW_AT_high_pc     : 0x9
 <4><b2>: Abbrev Number: 11 (DW_TAG_variable)
    <b3>   DW_AT_abstract_origin: <0x46>
    <b7>   DW_AT_location    : 9 byte block: 3 0 0 0 0 0 0 0 0  (DW_OP_addr: 0)

...

 <1><c4>: Abbrev Number: 12 (DW_TAG_subprogram)
    <c5>   DW_AT_external    : 1
    <c5>   DW_AT_name        : baz
...
 <2><ec>: Abbrev Number: 8 (DW_TAG_inlined_subroutine)
    <ed>   DW_AT_abstract_origin: <0x2d>
    <f1>   DW_AT_low_pc      : 0x10
    <f9>   DW_AT_high_pc     : 0x9
    <101>   DW_AT_call_file   : 1
    <102>   DW_AT_call_line   : 15
 <3><103>: Abbrev Number: 9 (DW_TAG_formal_parameter)
    <104>   DW_AT_abstract_origin: <0x3d>
    <108>   DW_AT_location    : 1 byte block: 55        (DW_OP_reg5 (rdi))
 <3><10a>: Abbrev Number: 10 (DW_TAG_lexical_block)
    <10b>   DW_AT_low_pc      : 0x10
    <113>   DW_AT_high_pc     : 0x9
 <4><11b>: Abbrev Number: 11 (DW_TAG_variable)
    <11c>   DW_AT_abstract_origin: <0x46>
    <120>   DW_AT_location    : 9 byte block: 3 0 0 0 0 0 0 0 0        
(DW_OP_addr: 0)


which also means at least two (in the DW_TAG_inlined_subroutine instances)
concrete variables named 'j'.  Not sure if the <46> DIE is a concrete
variable - DWARF doesn't have linkage attributes and the containing
subroutine DIE is marked with DW_AT_inline.

Note we're not emitting a concrete instance of the function which would
contain the "main" instance of 'j'.  Not sure what the DWARF for a
standalone concrete instantiation of 'j' would look like.

So current LTO is modeled after this, the early debug contains abstract
instances only (also for non-functions, thus global variables), albeit
not marked as such in all cases.  The LTRANS debug contains all concrete
(and inline) instances.

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

* [Bug debug/94450] lto abstract variable emitted as concrete decl
  2020-04-01 21:51 [Bug debug/94450] New: lto abstract variable emitted as concrete decl 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
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-04-02 10:22 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #2)
> So the current situation is similar to that of

Modifying the testcase to C99

inline int foo(int i)
{
  static int j;
  j = i + 1;
  return j;
}

int bar(int i)
{
  return foo(i);
}

int baz(int i)
{
  return foo(i);
}

extern int foo(int i);

[...]

> Note we're not emitting a concrete instance of the function which would
> contain the "main" instance of 'j'.  Not sure what the DWARF for a
> standalone concrete instantiation of 'j' would look like.

The concrete instance is emitted:

 <1><57>: Abbrev Number: 6 (DW_TAG_subprogram)
    <58>   DW_AT_abstract_origin: <0x2d>
    <5c>   DW_AT_low_pc      : 0x0
    <64>   DW_AT_high_pc     : 0xa
    <6c>   DW_AT_frame_base  : 1 byte block: 9c         (DW_OP_call_frame_cfa)
    <6e>   DW_AT_GNU_all_call_sites: 1
    <6e>   DW_AT_sibling     : <0x89>
 <2><72>: Abbrev Number: 7 (DW_TAG_formal_parameter)
    <73>   DW_AT_abstract_origin: <0x3d>
    <77>   DW_AT_location    : 1 byte block: 55         (DW_OP_reg5 (rdi))
 <2><79>: Abbrev Number: 8 (DW_TAG_variable)
    <7a>   DW_AT_abstract_origin: <0x46>
    <7e>   DW_AT_location    : 9 byte block: 3 0 0 0 0 0 0 0 0  (DW_OP_addr: 0)

so here's another copy of the variable.

I guess for function local variables gdb isn't fooled to think it has
two copies?  And the bogus copy is not because of the abstract origin
link but because of the unit import but could be brought in by other
means of making gdb read the DWARF of it?

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

* [Bug debug/94450] lto abstract variable emitted as concrete decl
  2020-04-01 21:51 [Bug debug/94450] New: lto abstract variable emitted as concrete decl vries at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  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
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-04-02 10:25 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 48168
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48168&action=edit
patch to drop DW_TAG_imported_unit DIEs

I'm testing this patch.  Does it help?

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

* [Bug debug/94450] lto abstract variable emitted as concrete decl
  2020-04-01 21:51 [Bug debug/94450] New: lto abstract variable emitted as concrete decl vries at gcc dot gnu.org
                   ` (3 preceding siblings ...)
  2020-04-02 10:25 ` rguenth at gcc dot gnu.org
@ 2020-04-02 12:29 ` vries at gcc dot gnu.org
  2020-04-02 14:48 ` cvs-commit at gcc dot gnu.org
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: vries at gcc dot gnu.org @ 2020-04-02 12:29 UTC (permalink / raw)
  To: gcc-bugs

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.

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

* [Bug debug/94450] lto abstract variable emitted as concrete decl
  2020-04-01 21:51 [Bug debug/94450] New: lto abstract variable emitted as concrete decl vries at gcc dot gnu.org
                   ` (4 preceding siblings ...)
  2020-04-02 12:29 ` vries at gcc dot gnu.org
@ 2020-04-02 14:48 ` cvs-commit at gcc dot gnu.org
  2020-04-02 14:48 ` rguenth at gcc dot gnu.org
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-04-02 14:48 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:54af95767e887d63dc332731738e642536d87a48

commit r10-7521-g54af95767e887d63dc332731738e642536d87a48
Author: Richard Biener <rguenther@suse.de>
Date:   Thu Apr 2 16:45:28 2020 +0200

    debug/94450 - remove DW_TAG_imported_unit generated in LTRANS units

    This removes the DW_TAG_imported_unit we generate for each referenced
    early debug unit in LTRANS units.  They are more harmful than they
    do good and the semantics can be read in a way making it even wrong.

    2020-04-02  Richard Biener  <rguenther@suse.de>

            PR debug/94450
            * dwarf2out.c (dwarf2out_early_finish): Remove code emitting
            DW_TAG_imported_unit.

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

* [Bug debug/94450] lto abstract variable emitted as concrete decl
  2020-04-01 21:51 [Bug debug/94450] New: lto abstract variable emitted as concrete decl vries at gcc dot gnu.org
                   ` (5 preceding siblings ...)
  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
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-04-02 14:48 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
The DW_TAG_imported_unit are now gone for GCC 10.  So can we consider this
fixed?

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

* [Bug debug/94450] lto abstract variable emitted as concrete decl
  2020-04-01 21:51 [Bug debug/94450] New: lto abstract variable emitted as concrete decl vries at gcc dot gnu.org
                   ` (6 preceding siblings ...)
  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
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: vries at gcc dot gnu.org @ 2020-04-03 10:26 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #8 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #7)
> The DW_TAG_imported_unit are now gone for GCC 10.  So can we consider this
> fixed?

I'd like a PR to refer to at the to-be-added xfail in the gdb test-case (and
the PR should be open as long as that test fails with trunk gcc). It doesn't
matter for me whether that's this particular PR or a follow-up PR.

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

* [Bug debug/94450] lto abstract variable emitted as concrete decl
  2020-04-01 21:51 [Bug debug/94450] New: lto abstract variable emitted as concrete decl vries at gcc dot gnu.org
                   ` (7 preceding siblings ...)
  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
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: rguenther at suse dot de @ 2020-04-03 11:10 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #9 from rguenther at suse dot de <rguenther at suse dot de> ---
On Fri, 3 Apr 2020, vries at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94450
> 
> --- Comment #8 from Tom de Vries <vries at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #7)
> > The DW_TAG_imported_unit are now gone for GCC 10.  So can we consider this
> > fixed?
> 
> I'd like a PR to refer to at the to-be-added xfail in the gdb test-case (and
> the PR should be open as long as that test fails with trunk gcc). It doesn't
> matter for me whether that's this particular PR or a follow-up PR.

OK, so if you have a (single?) specific testcase that's still affected
please duplicate that into a new bugzilla.  It's always better to
have something specific to track.

So did the patch not change anything?

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

* [Bug debug/94450] lto abstract variable emitted as concrete decl
  2020-04-01 21:51 [Bug debug/94450] New: lto abstract variable emitted as concrete decl vries at gcc dot gnu.org
                   ` (8 preceding siblings ...)
  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
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: vries at gcc dot gnu.org @ 2020-04-03 12:52 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #10 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #9)
> On Fri, 3 Apr 2020, vries at gcc dot gnu.org wrote:
> 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94450
> > 
> > --- Comment #8 from Tom de Vries <vries at gcc dot gnu.org> ---
> > (In reply to Richard Biener from comment #7)
> > > The DW_TAG_imported_unit are now gone for GCC 10.  So can we consider this
> > > fixed?
> > 
> > I'd like a PR to refer to at the to-be-added xfail in the gdb test-case (and
> > the PR should be open as long as that test fails with trunk gcc). It doesn't
> > matter for me whether that's this particular PR or a follow-up PR.
> 
> OK, so if you have a (single?) specific testcase that's still affected
> please duplicate that into a new bugzilla.  It's always better to
> have something specific to track.
> 
> So did the patch not change anything?

Well, the changes I asked for related to the example in comment 0 are:
- drop the import
- change the tag from DW_TAG_compile_unit to DW_TAG_partial unit.

AFAIU the patch only removes the import, so in that sense I do not consider the
test-case reported in comment 0 addressed.

I have not tried out the patch.  FWIW, I did try a quick dwarf-assembly
experiment (not the ada one, which will cost more time I expect, but modified
gdb testsuite test-case gdb.dwarf2/imported-unit.exp) and confirmed that
neither only removing the import nor only changing the tag is sufficient to get
only one entry in the symtab.

Anyway, I understand the example is somewhat abstract, and I'll file the actual
ada example.

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

* [Bug debug/94450] lto abstract variable emitted as concrete decl
  2020-04-01 21:51 [Bug debug/94450] New: lto abstract variable emitted as concrete decl vries at gcc dot gnu.org
                   ` (9 preceding siblings ...)
  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
  12 siblings, 0 replies; 14+ messages in thread
From: rguenther at suse dot de @ 2020-04-03 13:03 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #11 from rguenther at suse dot de <rguenther at suse dot de> ---
On Fri, 3 Apr 2020, vries at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94450
> 
> --- Comment #10 from Tom de Vries <vries at gcc dot gnu.org> ---
> (In reply to rguenther@suse.de from comment #9)
> > On Fri, 3 Apr 2020, vries at gcc dot gnu.org wrote:
> > 
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94450
> > > 
> > > --- Comment #8 from Tom de Vries <vries at gcc dot gnu.org> ---
> > > (In reply to Richard Biener from comment #7)
> > > > The DW_TAG_imported_unit are now gone for GCC 10.  So can we consider this
> > > > fixed?
> > > 
> > > I'd like a PR to refer to at the to-be-added xfail in the gdb test-case (and
> > > the PR should be open as long as that test fails with trunk gcc). It doesn't
> > > matter for me whether that's this particular PR or a follow-up PR.
> > 
> > OK, so if you have a (single?) specific testcase that's still affected
> > please duplicate that into a new bugzilla.  It's always better to
> > have something specific to track.
> > 
> > So did the patch not change anything?
> 
> Well, the changes I asked for related to the example in comment 0 are:
> - drop the import
> - change the tag from DW_TAG_compile_unit to DW_TAG_partial unit.
> 
> AFAIU the patch only removes the import, so in that sense I do not consider the
> test-case reported in comment 0 addressed.

OK, fair enough.  Note that as I read the DWARF spec changing the
early CUs to DW_TAG_partial_unit and then again importing those
(the spec suggests you need to import DW_TAG_partial_units) would
not reflect documented semantics either.  While the early CUs
represent the original TUs declarations the actual instantiation
is split into multiple late CUs.  Currently we import the early CUs
in each late CU that instantiates at least _one_ of the early CU
entities.  But as I read the SPEC importing a partial CU means
the importing CU will be an instantiation point for _all_ entities
in the imported partial CU.

So there may not be a good fit for the LTO usage where we have
"abstract" units for each TU and "concrete" units for each
LTRANS unit picking items to instantiate from various abstract CUs.

Splitting the early CUs into multiple DW_TAG_partial_units, one
for each object, and selectively only importing the instantiated
ones might make it fit better but we then likely have to force
the use of type units to carry type DIEs.  Note we'd still import
the abstract unit for a function when the function is only
instantiated inline and elsewhere instantiated concrete.

So somehow making the early CUs "abstract" might be a better
approach (not sure how gdb would like to have the DWARF represented
then).

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

* [Bug debug/94450] lto abstract variable emitted as concrete decl
  2020-04-01 21:51 [Bug debug/94450] New: lto abstract variable emitted as concrete decl vries at gcc dot gnu.org
                   ` (10 preceding siblings ...)
  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
  12 siblings, 0 replies; 14+ messages in thread
From: vries at gcc dot gnu.org @ 2020-04-03 13:23 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #12 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom de Vries from comment #10)
> I'll file the actual ada example.

PR94469 - "lto abstract variable emitted as concrete decl (ada test-case)"

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

* [Bug debug/94450] lto abstract variable emitted as concrete decl
  2020-04-01 21:51 [Bug debug/94450] New: lto abstract variable emitted as concrete decl vries at gcc dot gnu.org
                   ` (11 preceding siblings ...)
  2020-04-03 13:23 ` vries at gcc dot gnu.org
@ 2020-04-03 16:43 ` vries at gcc dot gnu.org
  12 siblings, 0 replies; 14+ messages in thread
From: vries at gcc dot gnu.org @ 2020-04-03 16:43 UTC (permalink / raw)
  To: gcc-bugs

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

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tromey at gcc dot gnu.org

--- Comment #13 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #11)

> Note that as I read the DWARF spec changing the
> early CUs to DW_TAG_partial_unit and then again importing those
> (the spec suggests you need to import DW_TAG_partial_units) would
> not reflect documented semantics either.

It's not my understanding from the spec that DW_TAG_partial_units are required
to be imported. AFAIU, it's just that in all the use-cases there using
DW_TAG_partial_unit, an import happens to be required.

OK, so lets look at the use cases described in the spec at E.1.

I. classic dwz: factor out into partial unit, add imports:
...
    DW_TAG_compile_unit
      DW_AT_name cu1
L1:   DIEx
      DIEy
        DW_AT_type L1
    DW_TAG_compile_unit
      DW_AT_name cu2
L2:   DIEx
      DIEz
        DW_AT_type L2

->

L3: DW_TAG_partial_unit
L4:   DIEx
    DW_TAG_compile_unit
      DW_TAG_imported_unit
        DW_AT_import L3
      DW_AT_name cu1
      DIEy
        DW_AT_type L4
    DW_TAG_compile_unit
      DW_TAG_imported_unit
        DW_AT_import L3
      DW_AT_name cu2
      DIEz
        DW_AT_type L4
...

II. dwz --devel-uni-lang --devel-gen-cu: factor out into DW_TAG_compile_unit,
no imports, exploit global namespace:
...
    DW_TAG_compile_unit
      DW_AT_name cu1
L1:   DIEx
      DIEy
        DW_AT_type L1
    DW_TAG_compile_unit
      DW_AT_name cu2
L2:   DIEx
      DIEz
        DW_AT_type L2

->

    DW_TAG_compile_unit
L4:   DIEx
    DW_TAG_compile_unit
      DW_AT_name cu1
      DIEy
        DW_AT_type L4
    DW_TAG_compile_unit
      DW_AT_name cu2
      DIEz
        DW_AT_type L4
...

III. #include in namespace:
...
    DW_TAG_compile_unit
      DW_AT_name cu1
      DW_TAG_namespace bla1
L1:     DIEx
        DIEy
          DW_AT_type L1
    DW_TAG_compile_unit
      DW_AT_name cu2
      DW_TAG_namespace bla2
L2:     DIEx
        DIEz
          DW_AT_type L2

->

L3: DW_TAG_partial_unit
L4:   DIEx
    DW_TAG_compile_unit
      DW_AT_name cu1
      DW_TAG_namespace bla1
        DW_TAG_imported_unit
          DW_AT_import L3
        DIEy
          DW_AT_type L4
    DW_TAG_compile_unit
      DW_AT_name cu2
      DW_TAG_namespace bla2
        DW_TAG_imported_unit
          DW_AT_import L3
        DIEz
          DW_AT_type L4
...

So indeed, in all cases where DW_TAG_partial_unit is used, we use an import,
but that's because it's applicable to the transformation, and we're just doing
an entirely different transformation here:
...
    DW_TAG_compile_unit
      DW_AT_name cu1
      DW_TAG_variable
        DW_AT_name var1
        DW_AT_location

->

    DW_TAG_partial_unit
L1:   DW_TAG_variable
        DW_AT_name var1
    DW_TAG_compile_unit
      DW_AT_name cu1
      DW_TAG_variable
        DW_AT_abstract_origin L1
        DW_AT_location
...
We fabricate a new abstract DW_TAG_variable DIE out of thin air, then try to
hide that fact by placing it in a DW_TAG_partial_unit, much like is done at
III. Only in contrast to III, we don't want to reintroduce it in another
context, we want it to keep hidden, so there's no import.

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

end of thread, other threads:[~2020-04-03 16:43 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-01 21:51 [Bug debug/94450] New: lto abstract variable emitted as concrete decl 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
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

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