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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

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

David Jones via llvm-dev
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?

_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] DWARF: Ranges base address specifier entries & Gold's gdb-index 32 bit bug

David Jones via llvm-dev

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:[hidden email]]
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?


_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] DWARF: Ranges base address specifier entries & Gold's gdb-index 32 bit bug

David Jones via llvm-dev
In reply to this post by David Jones via llvm-dev
On Aug 7, 2017 6:58 PM, "David Blaikie" <[hidden email]> 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?


_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] DWARF: Ranges base address specifier entries & Gold's gdb-index 32 bit bug

David Jones via llvm-dev


On Tue, Aug 8, 2017 at 6:56 AM Nico Weber <[hidden email]> wrote:
On Aug 7, 2017 6:58 PM, "David Blaikie" <[hidden email]> 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?


_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] DWARF: Ranges base address specifier entries & Gold's gdb-index 32 bit bug

David Jones via llvm-dev
On Tue, Aug 8, 2017 at 10:41 AM, David Blaikie <[hidden email]> wrote:


On Tue, Aug 8, 2017 at 6:56 AM Nico Weber <[hidden email]> wrote:
On Aug 7, 2017 6:58 PM, "David Blaikie" <[hidden email]> 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?

'cause you asked me to test with that on IRC :-) We don't use it in our "real" build config.
 
 
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?



_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] DWARF: Ranges base address specifier entries & Gold's gdb-index 32 bit bug

David Jones via llvm-dev


On Tue, Aug 8, 2017 at 7:43 AM Nico Weber <[hidden email]> wrote:
On Tue, Aug 8, 2017 at 10:41 AM, David Blaikie <[hidden email]> wrote:


On Tue, Aug 8, 2017 at 6:56 AM Nico Weber <[hidden email]> wrote:
On Aug 7, 2017 6:58 PM, "David Blaikie" <[hidden email]> 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?

'cause you asked me to test with that on IRC :-) We don't use it in our "real" build config.

Ah, OK!

:/ 
 
 
 
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?


_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] DWARF: Ranges base address specifier entries & Gold's gdb-index 32 bit bug

David Jones via llvm-dev
In reply to this post by David Jones via llvm-dev
Adrian: any thoughts? Has LLDB been fixed to support this yet?

On Tue, Aug 8, 2017 at 6:33 AM Robinson, Paul <[hidden email]> 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:[hidden email]]
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?


_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] DWARF: Ranges base address specifier entries & Gold's gdb-index 32 bit bug

David Jones via llvm-dev

Can gdb handle these?  i.e. is it just gold that has the problem?  Conditioning on debugger tuning when it's not the debugger that has the problem… icky.  If that's the case then asking people to add –ggnu-pubnames seems okay.  Assuming that gets handed down through IR/metadata then the LTO problem goes away too.

--paulr

 

From: David Blaikie [mailto:[hidden email]]
Sent: Tuesday, August 08, 2017 10:59 AM
To: Robinson, Paul; llvm-dev; Adrian Prantl; Eric Christopher; Nico Weber
Subject: Re: DWARF: Ranges base address specifier entries & Gold's gdb-index 32 bit bug

 

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

On Tue, Aug 8, 2017 at 6:33 AM Robinson, Paul <[hidden email]> 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:[hidden email]]
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?


_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] DWARF: Ranges base address specifier entries & Gold's gdb-index 32 bit bug

David Jones via llvm-dev


On Tue, Aug 8, 2017 at 10:50 AM Robinson, Paul <[hidden email]> wrote:

Can gdb handle these?  i.e. is it just gold that has the problem?


Yep, it's just gold when it's building the gdb-index (an accelerator table for GDB)
 

  Conditioning on debugger tuning when it's not the debugger that has the problem… icky.


It does. Though to a lesser degree even gnu-pubnames isn't a perfect signal. One, I think, could use that even without a linker-generated gdb-index - presumably GDB's still faster by reading gnu-pubnames than by reading the raw DIEs to do lookup.
 

  If that's the case then asking people to add –ggnu-pubnames seems okay.  Assuming that gets handed down through IR/metadata then the LTO problem goes away too.


gnu-pubnames has such problems (it's done as a backend option, passed down from the clang driver via -mllvm -generate-gnu-dwarf-pub-sections, but they're already there... - mostly the discussion around flags was "If we're adding a new one, what's the right tradeoff here for a workaround flag compared to the platonic ideals of CU-based flags as much as possible") but it's already that way... 

- Dave
 

--paulr

 

From: David Blaikie [mailto:[hidden email]]
Sent: Tuesday, August 08, 2017 10:59 AM
To: Robinson, Paul; llvm-dev; Adrian Prantl; Eric Christopher; Nico Weber
Subject: Re: DWARF: Ranges base address specifier entries & Gold's gdb-index 32 bit bug

 

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

On Tue, Aug 8, 2017 at 6:33 AM Robinson, Paul <[hidden email]> 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:[hidden email]]
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?


_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] DWARF: Ranges base address specifier entries & Gold's gdb-index 32 bit bug

David Jones via llvm-dev
Since it seems like there's unlikely to be a good heuristic that wouldn't potentially need to be overridden by some reasonable use cases, we'll need a flag one way or the thore - so let's add the flag & if someone wants to push on the defaults (I'll probably just leave it as default off without any heuristic for turning it on by default for any cases) they can.

So I've sent out a couple of reviews for the LLVM and Clang parts of adding a flag to opt-in to this functionality:

Clang: https://reviews.llvm.org/D54243 
LLVM: https://reviews.llvm.org/D54242 

Main thing is just to bikeshed the naming of the metadata and flag. 

Thanks everyone!
- Dave

On Tue, Aug 8, 2017 at 10:57 AM David Blaikie <[hidden email]> wrote:
On Tue, Aug 8, 2017 at 10:50 AM Robinson, Paul <[hidden email]> wrote:

Can gdb handle these?  i.e. is it just gold that has the problem?


Yep, it's just gold when it's building the gdb-index (an accelerator table for GDB)
 

  Conditioning on debugger tuning when it's not the debugger that has the problem… icky.


It does. Though to a lesser degree even gnu-pubnames isn't a perfect signal. One, I think, could use that even without a linker-generated gdb-index - presumably GDB's still faster by reading gnu-pubnames than by reading the raw DIEs to do lookup.
 

  If that's the case then asking people to add –ggnu-pubnames seems okay.  Assuming that gets handed down through IR/metadata then the LTO problem goes away too.


gnu-pubnames has such problems (it's done as a backend option, passed down from the clang driver via -mllvm -generate-gnu-dwarf-pub-sections, but they're already there... - mostly the discussion around flags was "If we're adding a new one, what's the right tradeoff here for a workaround flag compared to the platonic ideals of CU-based flags as much as possible") but it's already that way... 

- Dave
 

--paulr

 

From: David Blaikie [mailto:[hidden email]]
Sent: Tuesday, August 08, 2017 10:59 AM
To: Robinson, Paul; llvm-dev; Adrian Prantl; Eric Christopher; Nico Weber
Subject: Re: DWARF: Ranges base address specifier entries & Gold's gdb-index 32 bit bug

 

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

On Tue, Aug 8, 2017 at 6:33 AM Robinson, Paul <[hidden email]> 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:[hidden email]]
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?


_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] DWARF: Ranges base address specifier entries & Gold's gdb-index 32 bit bug

David Jones via llvm-dev
Flag's in, off by default, -fdebug-ranges-base-address - if anyone feels like taking on/advocating for making this on by default under some subset of conditions (non-32 bit builds (to avoid the gold 32 bit gdb-index bug), or some other slice), I'm happy to help with that, but adding a flag was the thing I was trying to avoid - so since it needs/has a flag, that's enough for me & I'll switch Google over to passing that flag once we pick up the changes.

- Dave

On Wed, Nov 7, 2018 at 5:13 PM David Blaikie <[hidden email]> wrote:
Since it seems like there's unlikely to be a good heuristic that wouldn't potentially need to be overridden by some reasonable use cases, we'll need a flag one way or the thore - so let's add the flag & if someone wants to push on the defaults (I'll probably just leave it as default off without any heuristic for turning it on by default for any cases) they can.

So I've sent out a couple of reviews for the LLVM and Clang parts of adding a flag to opt-in to this functionality:

Clang: https://reviews.llvm.org/D54243 
LLVM: https://reviews.llvm.org/D54242 

Main thing is just to bikeshed the naming of the metadata and flag. 

Thanks everyone!
- Dave


On Tue, Aug 8, 2017 at 10:57 AM David Blaikie <[hidden email]> wrote:
On Tue, Aug 8, 2017 at 10:50 AM Robinson, Paul <[hidden email]> wrote:

Can gdb handle these?  i.e. is it just gold that has the problem?


Yep, it's just gold when it's building the gdb-index (an accelerator table for GDB)
 

  Conditioning on debugger tuning when it's not the debugger that has the problem… icky.


It does. Though to a lesser degree even gnu-pubnames isn't a perfect signal. One, I think, could use that even without a linker-generated gdb-index - presumably GDB's still faster by reading gnu-pubnames than by reading the raw DIEs to do lookup.
 

  If that's the case then asking people to add –ggnu-pubnames seems okay.  Assuming that gets handed down through IR/metadata then the LTO problem goes away too.


gnu-pubnames has such problems (it's done as a backend option, passed down from the clang driver via -mllvm -generate-gnu-dwarf-pub-sections, but they're already there... - mostly the discussion around flags was "If we're adding a new one, what's the right tradeoff here for a workaround flag compared to the platonic ideals of CU-based flags as much as possible") but it's already that way... 

- Dave
 

--paulr

 

From: David Blaikie [mailto:[hidden email]]
Sent: Tuesday, August 08, 2017 10:59 AM
To: Robinson, Paul; llvm-dev; Adrian Prantl; Eric Christopher; Nico Weber
Subject: Re: DWARF: Ranges base address specifier entries & Gold's gdb-index 32 bit bug

 

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

On Tue, Aug 8, 2017 at 6:33 AM Robinson, Paul <[hidden email]> 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:[hidden email]]
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?


_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev