[Bug debug/91239] gcc generates invalid .debug_macro sections (according to lld folks)

[Bug debug/91239] gcc generates invalid .debug_macro sections (according to lld folks)

[tromey at gcc dot gnu.org]

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

Tom Tromey <tromey at gcc dot gnu.org> changed:

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

--- Comment #2 from Tom Tromey <tromey at gcc dot gnu.org> ---
I separately discovered this problem when debugging an apparent gdb
slowdown, which I tracked down to a pathological .debug_macro section --
but only when I switched to using 'mold' to link.

> So how does a testcase look like? 
[...]
> Can you possibly share two source files and instructions to reproduce the
> conflicting comdats?

I filed a trivial example (three very short files) with mold:
https://github.com/rui314/mold/issues/438

I'm taking the liberty of moving this out of "WAITING".

[Bug debug/91239] gcc generates invalid .debug_macro sections (according to lld folks)

[tromey at gcc dot gnu.org]

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

--- Comment #3 from Tom Tromey <tromey at gcc dot gnu.org> ---
Created attachment 52836
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52836&action=edit
test program

I thought I'd upload the sources.  You can just untar.
Compile with "gcc -g3 -O0 r.cc z.cc -o z"
If you link with mold or lld, you'll see 0x0 imports in .debug_macros.
If you link with gold or the bfd ld, that won't happen.

While I think this behavior seems unwanted on the part of mold/lld,
on the other hand perhaps it really is allowed by ELF, I don't know.

[Bug debug/91239] gcc generates invalid .debug_macro sections (according to lld folks)

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
It seems to work with lld (version 9 and 11) for me:

Contents of the .debug_macro section:

  Offset:                      0x0
  Version:                     4
  Offset size:                 4
  Offset into .debug_line:     0xe8

 DW_MACRO_import - offset : 0x23
 DW_MACRO_start_file - lineno: 0 filenum: 1 filename: r.cc
 DW_MACRO_start_file - lineno: 0 filenum: 2 filename:
/usr/include/stdc-predef.h
 DW_MACRO_import - offset : 0x9ed
 DW_MACRO_end_file
 DW_MACRO_start_file - lineno: 1 filenum: 3 filename: r.h
 DW_MACRO_import - offset : 0xa09
 DW_MACRO_end_file
 DW_MACRO_end_file

but I also only see section type relocations and obviously those have to
work and be redirected to the prevailing comdat group.  So - it doesn't
reproduce for me with your testcase.

[Bug debug/91239] gcc generates invalid .debug_macro sections (according to lld folks)

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
See also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99618 and
https://sourceware.org/bugzilla/show_bug.cgi?id=27590
>From DWARF5 it is clear what needs to happen and giving up on the
DW_MACRO_import deduplication would mean 10x growth of .debug_macro or how much
on average.
Furthermore, due to the ill-designed -fuse-ld= option which allows users to
easily switch to a linker which doesn't support what has been determined during
gcc configure time from the linker tested at that time we can't easily disable
DW_MACRO_import just for selected linkers like mold.
So, I think those linkers simply have to support this, whether by a hack keyed
on a particular section name or generally for non-allocatable sections is up to
them.

[Bug debug/91239] gcc generates invalid .debug_macro sections (according to lld folks)

[rui314 at gmail dot com]

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

Rui Ueyama <rui314 at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rui314 at gmail dot com

--- Comment #6 from Rui Ueyama <rui314 at gmail dot com> ---
I can add a support to mold so that mold works for this test case. I'm not 100%
sure what's the correct behavior though.

It looks like we should redirect references to dead comdat members to
prevailing ones. But is there any guarantee that all COMDAT members have the
exact same set of members? We deduplicate COMDAT groups just by looking at
their group names and don't compare their members.

Or, can we just leave `.debug_macros` sections even if they are in deduplicated
COMDAT groups?

[Bug debug/91239] gcc generates invalid .debug_macro sections (according to lld folks)

[rui314 at gmail dot com]

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

--- Comment #7 from Rui Ueyama <rui314 at gmail dot com> ---
Note that the latter should be easier to implement because finding a matching
section in a prevailing comdat group for a deduplicated section may not be
trivial.

[Bug debug/91239] gcc generates invalid .debug_macro sections (according to lld folks)

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The .debug_macro deduplication works by computing a checksum of the ops coming
from certain headers (define, undef entries mostly) and not interrupted by
entries from other headers (e.g. if stdio.h has 10 define/undef directives,
then include some other header that has 20 define/undef directives, then
stdio.h has another 30 define/undef directives, there will be 3 comdat
sections,
2 with wm4.stdio.h.* name (with the hash after it) and one for the other
header).
The DW_MACRO_import operands always refer to the start of such comdat section.
Except for the ignored case of a hash collision, the intended behaviro is that
if  out of say 50 copies of wm4.stdio.h.24.5c1b97eef3c86b7a2549420f69f4f128 one
is kept and the 49 are left out, all the references are adjusted to the start
of the  copy that is left in the ELF object.

[Bug debug/91239] gcc generates invalid .debug_macro sections (according to lld folks)

[rui314 at gmail dot com]

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

--- Comment #9 from Rui Ueyama <rui314 at gmail dot com> ---
Thanks Jukub for the description. I'll try to implement it to mold.

One question though. Why doesn't gcc create a weak symbol
wm4.stdio.h.24.5c1b97eef3c86b7a2549420f69f4f128 at the beginning of each
wm4.stdio.h.24.5c1b97eef3c86b7a2549420f69f4f128 section and always use that
symbol instead of referring the beginning of the section using the section
symbol? The ELF spec says that no one should refer any comdat members directly
and instead refer them via symbols, so the current GCC's behavior violates the
spec if I understand it correctly.

[Bug debug/91239] gcc generates invalid .debug_macro sections (according to lld folks)

[jakub at gcc dot gnu.org]

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

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
See the PR I've referred to.
There is no ABI that would cover the names of such symbols and we definitely
don't want them to be visible out of the shared libraries or binaries.

[Bug debug/91239] gcc generates invalid .debug_macro sections (according to lld folks)

[rui314 at gmail dot com]

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

--- Comment #11 from Rui Ueyama <rui314 at gmail dot com> ---
You can make it a global, hidden symbol, can't you?

[Bug debug/91239] gcc generates invalid .debug_macro sections (according to lld folks)

[rui314 at gmail dot com]

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

--- Comment #12 from Rui Ueyama <rui314 at gmail dot com> ---
I mean setting STV_HIDDEN visibility to a symbol prevents it to be visible from
any ELF binaries than itself, which may resolve your concern. I may be missing
something though.

[Bug debug/91239] gcc generates invalid .debug_macro sections (according to lld folks)

[jakub at gcc dot gnu.org]

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

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
No, I can't.  For one, many targets GCC supports don't support visibility.
And, even for those that do, such a change would be effectively an ABI change,
as mixing *.o files from older gcc version and newer one could result in the
invalid DW_MACRO_import operands if the old version produced comdat wins.

[Bug debug/91239] gcc generates invalid .debug_macro sections (according to lld folks)

[rui314 at gmail dot com]

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

--- Comment #14 from Rui Ueyama <rui314 at gmail dot com> ---
I see. I think I'm convinced.

[Bug debug/91239] gcc generates invalid .debug_macro sections (according to lld folks)

[rguenther at suse dot de]

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

--- Comment #15 from rguenther at suse dot de <rguenther at suse dot de> ---
On Wed, 20 Apr 2022, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91239
> 
> --- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> No, I can't.  For one, many targets GCC supports don't support visibility.
> And, even for those that do, such a change would be effectively an ABI change,
> as mixing *.o files from older gcc version and newer one could result in the
> invalid DW_MACRO_import operands if the old version produced comdat wins.

To avoid that we'd need to change the mangling of the comdat symbol
(add a 'v2' suffix?), so I think it would be feasible.

I agree that it's unfortunate to not have a way to collect unused
symbols after link editing is done.  ELF is lacking there - we
get biten by this as well with LTO and hidden symbols used across
LTRANS units - they still consume symtab entries after link editing.