llvm dev - Aug 2017 - DWARF: Ranges base address specifier entries & Gold's gdb-index 32 bit bug

Context:

In r309526 (with a followup fix in r309529) I implemented the use of
DWARF's debug_ranges base address specifier entries to reduce the number of
object file relocations needed for debug_ranges*.

* in a particular binary internally, an optimized build had a 70% reduction
in debug_ranges.reloc, a 16% decrease in total object size (with compressed
debug info and fission)

Nico noted that this broke the Chromium build (
https://bugs.llvm.org/show_bug.cgi?id=34007 ). Turns out GNU Binutils Gold
has a bug (I've reported it as:
https://sourceware.org/bugzilla/show_bug.cgi?id=21894 - wouldn't expect
much action on it, though) where it fails to parse a base address specifier
entry when trying to build gdb-index (-Wl,-gdb-index) for a 32 bit build.

To address this in the short term, I added a flag that disables this
feature by default for now.

So, two questions:

1) What does everyone reckon the best solution to this is?
  * Keep the feature disabled by default - with a flag to enable it for
those who want it/can use it
  * Enable the feature everywhere - with a flag to disable it
  * Conditionally disable (or conditionally enable) the feature, with a
flag to enable/disable it
    * best condition we probably have is "disabled if 32 bit and
-ggnu-pubnames" (gnu-pubnames are necessary for efficient building of
gdb-index, and isn't the default)
2) How to implement the flag:
  * backend option as it is currently (-mllvm
-use-dwarf-ranges-base-address-specifier (maybe drop "-specifier" to
make
it a bit more terse))
  * clang option that maps to
    * backend option (as currently)
    * MCOption (programmatic/C++ API rather than command line of a backend
option)
    * LLVM IR attribute on the CU (this feature could be supported on a
per-CU basis easiyl enough, thus survive LTO, etc, exactly as the user
requested (but really if the user requested this on at least one CU, they
probably might as well have it on all their CUs))
    * LLVM IR module metadata


Any ideas/thoughts/other aspects?
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170807/9ec25734/attachment.html>

My inclination would be to use "disable if 32-bit and –ggnu-pubnames"
as the default, and have a flag.
This would be determined in DwarfDebug, right? If the check is in CodeGen then
I'd think it would be insensitive to LTO.
Setting it via IR/metadata is adding a bunch of complexity just to work around a
gold bug.  Not worth it IMO.
--paulr

From: David Blaikie [mailto:dblaikie at gmail.com]
Sent: Monday, August 07, 2017 6:58 PM
To: llvm-dev; Robinson, Paul; Adrian Prantl; Eric Christopher; Nico Weber
Subject: DWARF: Ranges base address specifier entries & Gold's gdb-index
32 bit bug

Context:

In r309526 (with a followup fix in r309529) I implemented the use of DWARF's
debug_ranges base address specifier entries to reduce the number of object file
relocations needed for debug_ranges*.

* in a particular binary internally, an optimized build had a 70% reduction in
debug_ranges.reloc, a 16% decrease in total object size (with compressed debug
info and fission)

Nico noted that this broke the Chromium build (
https://bugs.llvm.org/show_bug.cgi?id=34007 ). Turns out GNU Binutils Gold has a
bug (I've reported it as:
https://sourceware.org/bugzilla/show_bug.cgi?id=21894 - wouldn't expect much
action on it, though) where it fails to parse a base address specifier entry
when trying to build gdb-index (-Wl,-gdb-index) for a 32 bit build.

To address this in the short term, I added a flag that disables this feature by
default for now.

So, two questions:

1) What does everyone reckon the best solution to this is?
  * Keep the feature disabled by default - with a flag to enable it for those
who want it/can use it
  * Enable the feature everywhere - with a flag to disable it
  * Conditionally disable (or conditionally enable) the feature, with a flag to
enable/disable it
    * best condition we probably have is "disabled if 32 bit and
-ggnu-pubnames" (gnu-pubnames are necessary for efficient building of
gdb-index, and isn't the default)
2) How to implement the flag:
  * backend option as it is currently (-mllvm
-use-dwarf-ranges-base-address-specifier (maybe drop "-specifier" to
make it a bit more terse))
  * clang option that maps to
    * backend option (as currently)
    * MCOption (programmatic/C++ API rather than command line of a backend
option)
    * LLVM IR attribute on the CU (this feature could be supported on a per-CU
basis easiyl enough, thus survive LTO, etc, exactly as the user requested (but
really if the user requested this on at least one CU, they probably might as
well have it on all their CUs))
    * LLVM IR module metadata


Any ideas/thoughts/other aspects?
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170808/aa584afb/attachment.html>

On Aug 7, 2017 6:58 PM, "David Blaikie" <dblaikie at gmail.com>
wrote:

Context:

In r309526 (with a followup fix in r309529) I implemented the use of
DWARF's debug_ranges base address specifier entries to reduce the number of
object file relocations needed for debug_ranges*.

* in a particular binary internally, an optimized build had a 70% reduction
in debug_ranges.reloc, a 16% decrease in total object size (with compressed
debug info and fission)

Nico noted that this broke the Chromium build ( https://bugs.llvm.org/show_
bug.cgi?id=34007 ). Turns out GNU Binutils Gold has a bug (I've reported it
as: https://sourceware.org/bugzilla/show_bug.cgi?id=21894 - wouldn't expect
much action on it, though) where it fails to parse a base address specifier
entry when trying to build gdb-index (-Wl,-gdb-index) for a 32 bit build.

To address this in the short term, I added a flag that disables this
feature by default for now.

So, two questions:

1) What does everyone reckon the best solution to this is?
  * Keep the feature disabled by default - with a flag to enable it for
those who want it/can use it
  * Enable the feature everywhere - with a flag to disable it
  * Conditionally disable (or conditionally enable) the feature, with a
flag to enable/disable it
    * best condition we probably have is "disabled if 32 bit and
-ggnu-pubnames" (gnu-pubnames are necessary for efficient building of
gdb-index, and isn't the default)


FWIW, we use -Wl,--gdb-index but had never heard of -ggnu-pubnames before
last week and don't use it (yet?). So maybe just "32-bit".

Either of these options works for us. From a usability perspective
independent of chromium, making your linker not crash by default seems kind
of nice to me. But either of these options works for me. Disabling the
feature everywhere by default seems a bit strange since it has no known
drawbacks on 64-bit (yet? :-) ).

2) How to implement the flag:
  * backend option as it is currently (-mllvm
-use-dwarf-ranges-base-address-specifier
(maybe drop "-specifier" to make it a bit more terse))
  * clang option that maps to
    * backend option (as currently)
    * MCOption (programmatic/C++ API rather than command line of a backend
option)
    * LLVM IR attribute on the CU (this feature could be supported on a
per-CU basis easiyl enough, thus survive LTO, etc, exactly as the user
requested (but really if the user requested this on at least one CU, they
probably might as well have it on all their CUs))
    * LLVM IR module metadata


Any ideas/thoughts/other aspects?
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170808/3bcf63f5/attachment.html>

On Tue, Aug 8, 2017 at 6:56 AM Nico Weber <thakis at chromium.org> wrote:
> On Aug 7, 2017 6:58 PM, "David Blaikie" <dblaikie at
gmail.com> wrote:
>
> Context:
>
> In r309526 (with a followup fix in r309529) I implemented the use of
> DWARF's debug_ranges base address specifier entries to reduce the
number of
> object file relocations needed for debug_ranges*.
>
> * in a particular binary internally, an optimized build had a 70%
> reduction in debug_ranges.reloc, a 16% decrease in total object size (with
> compressed debug info and fission)
>
> Nico noted that this broke the Chromium build (
> https://bugs.llvm.org/show_bug.cgi?id=34007 ). Turns out GNU Binutils
> Gold has a bug (I've reported it as:
> https://sourceware.org/bugzilla/show_bug.cgi?id=21894 - wouldn't expect
> much action on it, though) where it fails to parse a base address specifier
> entry when trying to build gdb-index (-Wl,-gdb-index) for a 32 bit build.
>
> To address this in the short term, I added a flag that disables this
> feature by default for now.
>
> So, two questions:
>
> 1) What does everyone reckon the best solution to this is?
>   * Keep the feature disabled by default - with a flag to enable it for
> those who want it/can use it
>   * Enable the feature everywhere - with a flag to disable it
>   * Conditionally disable (or conditionally enable) the feature, with a
> flag to enable/disable it
>     * best condition we probably have is "disabled if 32 bit and
> -ggnu-pubnames" (gnu-pubnames are necessary for efficient building of
> gdb-index, and isn't the default)
>
>
> FWIW, we use -Wl,--gdb-index but had never heard of -ggnu-pubnames before
> last week and don't use it (yet?).
>
PR34007 about the gold failure shows -ggnu-pubnames in the repro, though?

> So maybe just "32-bit".
>
> Either of these options works for us. From a usability perspective
> independent of chromium, making your linker not crash by default seems kind
> of nice to me. But either of these options works for me. Disabling the
> feature everywhere by default seems a bit strange since it has no known
> drawbacks on 64-bit (yet? :-) ).
>
> 2) How to implement the flag:
>   * backend option as it is currently (-mllvm
> -use-dwarf-ranges-base-address-specifier (maybe drop "-specifier"
to make
> it a bit more terse))
>   * clang option that maps to
>     * backend option (as currently)
>     * MCOption (programmatic/C++ API rather than command line of a backend
> option)
>     * LLVM IR attribute on the CU (this feature could be supported on a
> per-CU basis easiyl enough, thus survive LTO, etc, exactly as the user
> requested (but really if the user requested this on at least one CU, they
> probably might as well have it on all their CUs))
>     * LLVM IR module metadata
>
>
> Any ideas/thoughts/other aspects?
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170808/a2111b97/attachment.html>

Adrian: any thoughts? Has LLDB been fixed to support this yet?

On Tue, Aug 8, 2017 at 6:33 AM Robinson, Paul <paul.robinson at sony.com>
wrote:
> My inclination would be to use "disable if 32-bit and
–ggnu-pubnames" as
> the default,
>
Unfortunately Nico points out that Chrome doesn't currently use
-ggnu-pubnames :/ So to continue to work "out of the box" we'd
have to
broaden the surface to "disable if 32 bit and targeting gdb".

-ggnu-pubnames makes gdb-index creation much more efficient by allowing
gdb-index generation not to have to parse most of the DWARF DIEs, and just
create the index straight from a table. (this is necessary for correctness
in -gsplit-dwarf mode (where the DIEs are not available at link time as
they're in the .dwo not the .o), and an efficiency gain otherwise)

If "disable if 32 bit and targeting gdb" is overly broad (I feel it
kind of
is, but I'm not wedded to it) we could require those working around the
gold bug to add -ggnu-pubnames to add a bit more of a specific hint that
they're intending to build gdb-index from these objects.

> and have a flag.
>
> This would be determined in DwarfDebug, right?
>
Yes

> If the check is in CodeGen then I'd think it would be insensitive to
LTO.
>
There's some desire that LTO works as-if it was non-LTO. So if you pass
flags (eg: debugger tuning flags, in this case) to each separate compile,
you get the same behavior if you LTO as when you didn't - so we preserve
things in the CU, and then do the specified thing per-CU. (eg: emission
kind (gmlt/limited/full), split-debug-inlining, debug-info-for-profiling,
etc)

In this case if, say, you're on an LLDB platform but prefer to use GDB
(&
happen to use Gold and gdb-index on 32 bit and LTO) - so you pass the
debugger tuning=gdb flag to your compiles, it wouldn't disable the feature,
because the flag would be consulted at CodeGen time and not at compile time
when you passed the tuning flag. (oh, actually the tuning is passed down
via TM Options anyway, not on the CU - so I guess you wouldn't be getitng
the tuning at all today because that flag already doesn't follow the per-CU
model... well I guess that makes it easier)

> Setting it via IR/metadata is adding a bunch of complexity just to work
> around a gold bug.  Not worth it IMO.
>
Fair enough. Any thoughts on whether it should be a frontend-visible option
or just a cc1 or even backend option? I'll probably just leave it as a
backend option at this point, could even implement the defaulting down in
LLVM without any changes to Clang, I suppose...

> --paulr
>
>
>
> *From:* David Blaikie [mailto:dblaikie at gmail.com]
> *Sent:* Monday, August 07, 2017 6:58 PM
> *To:* llvm-dev; Robinson, Paul; Adrian Prantl; Eric Christopher; Nico
> Weber
> *Subject:* DWARF: Ranges base address specifier entries & Gold's
> gdb-index 32 bit bug
>
>
>
> Context:
>
> In r309526 (with a followup fix in r309529) I implemented the use of
> DWARF's debug_ranges base address specifier entries to reduce the
number of
> object file relocations needed for debug_ranges*.
>
> * in a particular binary internally, an optimized build had a 70%
> reduction in debug_ranges.reloc, a 16% decrease in total object size (with
> compressed debug info and fission)
>
> Nico noted that this broke the Chromium build (
> https://bugs.llvm.org/show_bug.cgi?id=34007 ). Turns out GNU Binutils
> Gold has a bug (I've reported it as:
> https://sourceware.org/bugzilla/show_bug.cgi?id=21894 - wouldn't expect
> much action on it, though) where it fails to parse a base address specifier
> entry when trying to build gdb-index (-Wl,-gdb-index) for a 32 bit build.
>
> To address this in the short term, I added a flag that disables this
> feature by default for now.
>
> So, two questions:
>
> 1) What does everyone reckon the best solution to this is?
>   * Keep the feature disabled by default - with a flag to enable it for
> those who want it/can use it
>   * Enable the feature everywhere - with a flag to disable it
>   * Conditionally disable (or conditionally enable) the feature, with a
> flag to enable/disable it
>     * best condition we probably have is "disabled if 32 bit and
> -ggnu-pubnames" (gnu-pubnames are necessary for efficient building of
> gdb-index, and isn't the default)
> 2) How to implement the flag:
>   * backend option as it is currently (-mllvm
> -use-dwarf-ranges-base-address-specifier (maybe drop "-specifier"
to make
> it a bit more terse))
>   * clang option that maps to
>     * backend option (as currently)
>     * MCOption (programmatic/C++ API rather than command line of a backend
> option)
>     * LLVM IR attribute on the CU (this feature could be supported on a
> per-CU basis easiyl enough, thus survive LTO, etc, exactly as the user
> requested (but really if the user requested this on at least one CU, they
> probably might as well have it on all their CUs))
>     * LLVM IR module metadata
>
>
> Any ideas/thoughts/other aspects?
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170808/71766a66/attachment.html>