* [PATCH] compress .gnu.debuglto_.debug_* sections if requested @ 2022-09-29 13:35 Martin Liška 2022-09-30 0:20 ` Alan Modra 0 siblings, 1 reply; 6+ messages in thread From: Martin Liška @ 2022-09-29 13:35 UTC (permalink / raw) To: binutils; +Cc: H.J. Lu Right now, when using LTO, the intermediate object files do contain debug info in sections starting with .gnu.debuglto_ prefix and are not compressed when --compress-debug-sections is used. It's a mistake and we can save quite some disk space. The following example comes from tramp3d when the corresponding LTO sections are compressed with zlib: $ bloaty tramp3d-v4-v2.o -- tramp3d-v4.o FILE SIZE VM SIZE -------------- -------------- +83% +10 [ = ] 0 [Unmapped] -68.0% -441 [ = ] 0 .gnu.debuglto_.debug_line -52.3% -759 [ = ] 0 .gnu.debuglto_.debug_line_str -62.4% -3.24Ki [ = ] 0 .gnu.debuglto_.debug_abbrev -64.8% -1.12Mi [ = ] 0 .gnu.debuglto_.debug_info -88.8% -4.58Mi [ = ] 0 .gnu.debuglto_.debug_str -27.7% -5.70Mi [ = ] 0 TOTAL Survives tests on x86_64-linux-gnu. Ready to be installed? Thanks, Martin bfd/ChangeLog: * elf.c (_bfd_elf_make_section_from_shdr): Compress also ".gnu.debuglto_.debug_" and use startswith for all 3 prefixes. gas/ChangeLog: * write.c (compress_debug): Compress also ".gnu.debuglto_.debug_". --- bfd/elf.c | 9 +++++---- gas/write.c | 3 ++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/bfd/elf.c b/bfd/elf.c index d496c1b2983..5d67c6f0694 100644 --- a/bfd/elf.c +++ b/bfd/elf.c @@ -1199,12 +1199,13 @@ _bfd_elf_make_section_from_shdr (bfd *abfd, } } - /* Compress/decompress DWARF debug sections with names: .debug_* and - .zdebug_*, after the section flags is set. */ + /* Compress/decompress DWARF debug sections with names: .debug_*, + .zdebug_*, .gnu.debuglto_.debug_, after the section flags is set. */ if ((newsect->flags & SEC_DEBUGGING) != 0 && (newsect->flags & SEC_HAS_CONTENTS) != 0 - && ((name[1] == 'd' && name[6] == '_') - || (name[1] == 'z' && name[7] == '_'))) + && (startswith (name, ".debug_") + || startswith (name, ".zdebug_") + || startswith (name, ".gnu.debuglto_.debug_"))) { enum { nothing, compress, decompress } action = nothing; int compression_header_size; diff --git a/gas/write.c b/gas/write.c index 0e49df7c03f..7c6affd8d50 100644 --- a/gas/write.c +++ b/gas/write.c @@ -1481,7 +1481,8 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) return; section_name = bfd_section_name (sec); - if (!startswith (section_name, ".debug_")) + if (!startswith (section_name, ".debug_") + && !startswith (section_name, ".gnu.debuglto_.debug_")) return; bool use_zstd = abfd->flags & BFD_COMPRESS_ZSTD; -- 2.37.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] compress .gnu.debuglto_.debug_* sections if requested 2022-09-29 13:35 [PATCH] compress .gnu.debuglto_.debug_* sections if requested Martin Liška @ 2022-09-30 0:20 ` Alan Modra 2022-09-30 7:19 ` Martin Liška 0 siblings, 1 reply; 6+ messages in thread From: Alan Modra @ 2022-09-30 0:20 UTC (permalink / raw) To: Martin Liška; +Cc: binutils On Thu, Sep 29, 2022 at 03:35:15PM +0200, Martin Liška wrote: > + /* Compress/decompress DWARF debug sections with names: .debug_*, > + .zdebug_*, .gnu.debuglto_.debug_, after the section flags is set. */ > if ((newsect->flags & SEC_DEBUGGING) != 0 > && (newsect->flags & SEC_HAS_CONTENTS) != 0 > - && ((name[1] == 'd' && name[6] == '_') > - || (name[1] == 'z' && name[7] == '_'))) > + && (startswith (name, ".debug_") > + || startswith (name, ".zdebug_") > + || startswith (name, ".gnu.debuglto_.debug_"))) If you test "(newsect->flags & SEC_ELF_OCTETS) != 0" as well, I think you could omit any name tests here. This would also compress .gnu.linkonce.wi.* sections. (Not that they matter very much, I'm just pointing out a code simplification.) > --- a/gas/write.c > +++ b/gas/write.c > @@ -1481,7 +1481,8 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) > return; > > section_name = bfd_section_name (sec); > - if (!startswith (section_name, ".debug_")) > + if (!startswith (section_name, ".debug_") > + && !startswith (section_name, ".gnu.debuglto_.debug_")) > return; > > bool use_zstd = abfd->flags & BFD_COMPRESS_ZSTD; Hmm, it looks to me like this function will transform ".gnu.debuglto_.debug_*" to ".zgnu.debuglto_.debug_*" if --compress-debug-sections=zlib-gnu is used, which will break your elf.c change. Please check --compress-debug-sections=zlib-gnu. -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] compress .gnu.debuglto_.debug_* sections if requested 2022-09-30 0:20 ` Alan Modra @ 2022-09-30 7:19 ` Martin Liška 2022-10-04 8:07 ` Alan Modra 0 siblings, 1 reply; 6+ messages in thread From: Martin Liška @ 2022-09-30 7:19 UTC (permalink / raw) To: Alan Modra; +Cc: binutils [-- Attachment #1: Type: text/plain, Size: 1649 bytes --] On 9/30/22 02:20, Alan Modra wrote: > On Thu, Sep 29, 2022 at 03:35:15PM +0200, Martin Liška wrote: >> + /* Compress/decompress DWARF debug sections with names: .debug_*, >> + .zdebug_*, .gnu.debuglto_.debug_, after the section flags is set. */ >> if ((newsect->flags & SEC_DEBUGGING) != 0 >> && (newsect->flags & SEC_HAS_CONTENTS) != 0 >> - && ((name[1] == 'd' && name[6] == '_') >> - || (name[1] == 'z' && name[7] == '_'))) >> + && (startswith (name, ".debug_") >> + || startswith (name, ".zdebug_") >> + || startswith (name, ".gnu.debuglto_.debug_"))) > > If you test "(newsect->flags & SEC_ELF_OCTETS) != 0" as well, I think > you could omit any name tests here. This would also compress > .gnu.linkonce.wi.* sections. (Not that they matter very much, I'm > just pointing out a code simplification.) Yep, I like the code simplification! > >> --- a/gas/write.c >> +++ b/gas/write.c >> @@ -1481,7 +1481,8 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) >> return; >> >> section_name = bfd_section_name (sec); >> - if (!startswith (section_name, ".debug_")) >> + if (!startswith (section_name, ".debug_") >> + && !startswith (section_name, ".gnu.debuglto_.debug_")) >> return; >> >> bool use_zstd = abfd->flags & BFD_COMPRESS_ZSTD; > > Hmm, it looks to me like this function will transform > ".gnu.debuglto_.debug_*" to ".zgnu.debuglto_.debug_*" if > --compress-debug-sections=zlib-gnu is used, which will break your > elf.c change. Please check --compress-debug-sections=zlib-gnu. Again, great comment, fixed in v2. Ready for master now? Thanks, Martin [-- Attachment #2: 0001-compress-.gnu.debuglto_.debug_-sections-if-requested.patch --] [-- Type: text/x-patch, Size: 2750 bytes --] From 8cff32766302af863aa1ddf9ae30a18f7c801eb5 Mon Sep 17 00:00:00 2001 From: Martin Liska <mliska@suse.cz> Date: Thu, 29 Sep 2022 14:10:30 +0200 Subject: [PATCH] compress .gnu.debuglto_.debug_* sections if requested Right now, when using LTO, the intermediate object files do contain debug info in sections starting with .gnu.debuglto_ prefix and are not compressed when --compress-debug-sections is used. It's a mistake and we can save quite some disk space. The following example comes from tramp3d when the corresponding LTO sections are compressed with zlib: $ bloaty tramp3d-v4-v2.o -- tramp3d-v4.o FILE SIZE VM SIZE -------------- -------------- +83% +10 [ = ] 0 [Unmapped] -68.0% -441 [ = ] 0 .gnu.debuglto_.debug_line -52.3% -759 [ = ] 0 .gnu.debuglto_.debug_line_str -62.4% -3.24Ki [ = ] 0 .gnu.debuglto_.debug_abbrev -64.8% -1.12Mi [ = ] 0 .gnu.debuglto_.debug_info -88.8% -4.58Mi [ = ] 0 .gnu.debuglto_.debug_str -27.7% -5.70Mi [ = ] 0 TOTAL bfd/ChangeLog: * elf.c (_bfd_elf_make_section_from_shdr): Compress all debug info sections. gas/ChangeLog: * write.c (compress_debug): Compress also ".gnu.debuglto_.debug_" if the compression algorithm is different from zlib-gnu. --- bfd/elf.c | 7 +++---- gas/write.c | 4 +++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/bfd/elf.c b/bfd/elf.c index d496c1b2983..8cd257fc65c 100644 --- a/bfd/elf.c +++ b/bfd/elf.c @@ -1199,12 +1199,11 @@ _bfd_elf_make_section_from_shdr (bfd *abfd, } } - /* Compress/decompress DWARF debug sections with names: .debug_* and - .zdebug_*, after the section flags is set. */ + /* Compress/decompress DWARF debug sections with names: .debug_*, + .zdebug_*, .gnu.debuglto_.debug_, after the section flags is set. */ if ((newsect->flags & SEC_DEBUGGING) != 0 && (newsect->flags & SEC_HAS_CONTENTS) != 0 - && ((name[1] == 'd' && name[6] == '_') - || (name[1] == 'z' && name[7] == '_'))) + && (newsect->flags & SEC_ELF_OCTETS) != 0) { enum { nothing, compress, decompress } action = nothing; int compression_header_size; diff --git a/gas/write.c b/gas/write.c index 0e49df7c03f..b8d5253006c 100644 --- a/gas/write.c +++ b/gas/write.c @@ -1481,7 +1481,9 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) return; section_name = bfd_section_name (sec); - if (!startswith (section_name, ".debug_")) + if (!startswith (section_name, ".debug_") + && (!startswith (section_name, ".gnu.debuglto_.debug_") + || flag_compress_debug == COMPRESS_DEBUG_GNU_ZLIB)) return; bool use_zstd = abfd->flags & BFD_COMPRESS_ZSTD; -- 2.37.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] compress .gnu.debuglto_.debug_* sections if requested 2022-09-30 7:19 ` Martin Liška @ 2022-10-04 8:07 ` Alan Modra 2022-10-04 8:12 ` Martin Liška 0 siblings, 1 reply; 6+ messages in thread From: Alan Modra @ 2022-10-04 8:07 UTC (permalink / raw) To: Martin Liška; +Cc: binutils On Fri, Sep 30, 2022 at 09:19:49AM +0200, Martin Liška wrote: > * write.c (compress_debug): Compress also ".gnu.debuglto_.debug_" > if the compression algorithm is different from zlib-gnu. OK, choosing to skip compression for zlib-gnu is a reasonable approach given that zlib-gnu has the clunky .debug_* to .zdebug_* section renaming scheme. I'm going to commit your patch, but then enable zlib-gnu with a followup patch of mine that doesn't rename the lto debug sections. ld.bfd and the binutils work fine without renaming, but there may be some other reason why zlib-gnu must have .zdebug sections. If that turns out to be the case, my patch below can be reverted. bfd/ * elf.c (elf_fake_sections): Replace "." with ".z" in debug section names only when name was ".d*", ie. ".debug_*". (_bfd_elf_assign_file_positions_for_non_load): Likewise. gas/ * write.c (compress_debug): Compress .gnu.debuglto_.debug_* for zlib-gnu too. Compress .gnu.linkonce.wi.*. diff --git a/bfd/elf.c b/bfd/elf.c index 8cd257fc65c..420b524aae8 100644 --- a/bfd/elf.c +++ b/bfd/elf.c @@ -3234,7 +3234,8 @@ elf_fake_sections (bfd *abfd, asection *asect, void *fsarg) name = new_name; } } - else if (asect->compress_status == COMPRESS_SECTION_DONE) + else if (asect->compress_status == COMPRESS_SECTION_DONE + && name[1] == 'd') { /* PR binutils/18087: Compression does not always make a section smaller. So only rename the section when @@ -3246,7 +3247,6 @@ elf_fake_sections (bfd *abfd, asection *asect, void *fsarg) arg->failed = true; return; } - BFD_ASSERT (name[1] != 'z'); name = new_name; } } @@ -6689,7 +6689,8 @@ _bfd_elf_assign_file_positions_for_non_load (bfd *abfd) return false; if (sec->compress_status == COMPRESS_SECTION_DONE - && (abfd->flags & BFD_COMPRESS_GABI) == 0) + && (abfd->flags & BFD_COMPRESS_GABI) == 0 + && name[1] == 'd') { /* If section is compressed with zlib-gnu, convert section name from .debug_* to .zdebug_*. */ diff --git a/gas/write.c b/gas/write.c index b8d5253006c..30f6c110908 100644 --- a/gas/write.c +++ b/gas/write.c @@ -1473,7 +1473,7 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) char *header; int x; flagword flags = bfd_section_flags (sec); - unsigned int header_size, compression_header_size; + unsigned int header_size; if (seginfo == NULL || sec->size < 32 @@ -1482,8 +1482,8 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) section_name = bfd_section_name (sec); if (!startswith (section_name, ".debug_") - && (!startswith (section_name, ".gnu.debuglto_.debug_") - || flag_compress_debug == COMPRESS_DEBUG_GNU_ZLIB)) + && !startswith (section_name, ".gnu.debuglto_.debug_") + && !startswith (section_name, ".gnu.linkonce.wi.")) return; bool use_zstd = abfd->flags & BFD_COMPRESS_ZSTD; @@ -1492,16 +1492,9 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) return; if (flag_compress_debug == COMPRESS_DEBUG_GNU_ZLIB) - { - compression_header_size = 0; - header_size = 12; - } + header_size = 12; else - { - compression_header_size - = bfd_get_compression_header_size (stdoutput, NULL); - header_size = compression_header_size; - } + header_size = bfd_get_compression_header_size (stdoutput, NULL); /* Create a new frag to contain the compression header. */ first_newf = frag_alloc (ob); @@ -1609,7 +1602,8 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) bfd_update_compression_header (abfd, (bfd_byte *) header, sec); x = bfd_set_section_size (sec, compressed_size); gas_assert (x); - if (!compression_header_size) + if (flag_compress_debug == COMPRESS_DEBUG_GNU_ZLIB + && section_name[1] == 'd') { compressed_name = concat (".z", section_name + 1, (char *) NULL); bfd_rename_section (sec, compressed_name); -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] compress .gnu.debuglto_.debug_* sections if requested 2022-10-04 8:07 ` Alan Modra @ 2022-10-04 8:12 ` Martin Liška 2022-10-04 13:41 ` Alan Modra 0 siblings, 1 reply; 6+ messages in thread From: Martin Liška @ 2022-10-04 8:12 UTC (permalink / raw) To: Alan Modra; +Cc: binutils, Fangrui Song On 10/4/22 10:07, Alan Modra wrote: > On Fri, Sep 30, 2022 at 09:19:49AM +0200, Martin Liška wrote: >> * write.c (compress_debug): Compress also ".gnu.debuglto_.debug_" >> if the compression algorithm is different from zlib-gnu. > > OK, choosing to skip compression for zlib-gnu is a reasonable approach > given that zlib-gnu has the clunky .debug_* to .zdebug_* section > renaming scheme. I'm going to commit your patch, but then enable > zlib-gnu with a followup patch of mine that doesn't rename the lto > debug sections. ld.bfd and the binutils work fine without renaming, > but there may be some other reason why zlib-gnu must have .zdebug > sections. If that turns out to be the case, my patch below can be > reverted. Makes sense. On the other hand, I would somehow slowly (but surely) make zlib-gnu obsolete. What do you think? For the future, people will likely move to the more modern and faster zstd compression algorithm. We're planning doing that once new binutils is released in openSUSE Tumbleweed. Cheers, Martin > > bfd/ > * elf.c (elf_fake_sections): Replace "." with ".z" in debug > section names only when name was ".d*", ie. ".debug_*". > (_bfd_elf_assign_file_positions_for_non_load): Likewise. > gas/ > * write.c (compress_debug): Compress .gnu.debuglto_.debug_* > for zlib-gnu too. Compress .gnu.linkonce.wi.*. > > diff --git a/bfd/elf.c b/bfd/elf.c > index 8cd257fc65c..420b524aae8 100644 > --- a/bfd/elf.c > +++ b/bfd/elf.c > @@ -3234,7 +3234,8 @@ elf_fake_sections (bfd *abfd, asection *asect, void *fsarg) > name = new_name; > } > } > - else if (asect->compress_status == COMPRESS_SECTION_DONE) > + else if (asect->compress_status == COMPRESS_SECTION_DONE > + && name[1] == 'd') > { > /* PR binutils/18087: Compression does not always make a > section smaller. So only rename the section when > @@ -3246,7 +3247,6 @@ elf_fake_sections (bfd *abfd, asection *asect, void *fsarg) > arg->failed = true; > return; > } > - BFD_ASSERT (name[1] != 'z'); > name = new_name; > } > } > @@ -6689,7 +6689,8 @@ _bfd_elf_assign_file_positions_for_non_load (bfd *abfd) > return false; > > if (sec->compress_status == COMPRESS_SECTION_DONE > - && (abfd->flags & BFD_COMPRESS_GABI) == 0) > + && (abfd->flags & BFD_COMPRESS_GABI) == 0 > + && name[1] == 'd') > { > /* If section is compressed with zlib-gnu, convert > section name from .debug_* to .zdebug_*. */ > diff --git a/gas/write.c b/gas/write.c > index b8d5253006c..30f6c110908 100644 > --- a/gas/write.c > +++ b/gas/write.c > @@ -1473,7 +1473,7 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) > char *header; > int x; > flagword flags = bfd_section_flags (sec); > - unsigned int header_size, compression_header_size; > + unsigned int header_size; > > if (seginfo == NULL > || sec->size < 32 > @@ -1482,8 +1482,8 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) > > section_name = bfd_section_name (sec); > if (!startswith (section_name, ".debug_") > - && (!startswith (section_name, ".gnu.debuglto_.debug_") > - || flag_compress_debug == COMPRESS_DEBUG_GNU_ZLIB)) > + && !startswith (section_name, ".gnu.debuglto_.debug_") > + && !startswith (section_name, ".gnu.linkonce.wi.")) > return; > > bool use_zstd = abfd->flags & BFD_COMPRESS_ZSTD; > @@ -1492,16 +1492,9 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) > return; > > if (flag_compress_debug == COMPRESS_DEBUG_GNU_ZLIB) > - { > - compression_header_size = 0; > - header_size = 12; > - } > + header_size = 12; > else > - { > - compression_header_size > - = bfd_get_compression_header_size (stdoutput, NULL); > - header_size = compression_header_size; > - } > + header_size = bfd_get_compression_header_size (stdoutput, NULL); > > /* Create a new frag to contain the compression header. */ > first_newf = frag_alloc (ob); > @@ -1609,7 +1602,8 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) > bfd_update_compression_header (abfd, (bfd_byte *) header, sec); > x = bfd_set_section_size (sec, compressed_size); > gas_assert (x); > - if (!compression_header_size) > + if (flag_compress_debug == COMPRESS_DEBUG_GNU_ZLIB > + && section_name[1] == 'd') > { > compressed_name = concat (".z", section_name + 1, (char *) NULL); > bfd_rename_section (sec, compressed_name); > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] compress .gnu.debuglto_.debug_* sections if requested 2022-10-04 8:12 ` Martin Liška @ 2022-10-04 13:41 ` Alan Modra 0 siblings, 0 replies; 6+ messages in thread From: Alan Modra @ 2022-10-04 13:41 UTC (permalink / raw) To: Martin Liška; +Cc: binutils, Fangrui Song On Tue, Oct 04, 2022 at 10:12:46AM +0200, Martin Liška wrote: > On 10/4/22 10:07, Alan Modra wrote: > > On Fri, Sep 30, 2022 at 09:19:49AM +0200, Martin Liška wrote: > >> * write.c (compress_debug): Compress also ".gnu.debuglto_.debug_" > >> if the compression algorithm is different from zlib-gnu. > > > > OK, choosing to skip compression for zlib-gnu is a reasonable approach > > given that zlib-gnu has the clunky .debug_* to .zdebug_* section > > renaming scheme. I'm going to commit your patch, but then enable > > zlib-gnu with a followup patch of mine that doesn't rename the lto > > debug sections. ld.bfd and the binutils work fine without renaming, > > but there may be some other reason why zlib-gnu must have .zdebug > > sections. If that turns out to be the case, my patch below can be > > reverted. > > Makes sense. On the other hand, I would somehow slowly (but surely) make zlib-gnu > obsolete. What do you think? I don't see that happening in a hurry. We'd want to keep the ability to handle zlib-gnu compressed object files for as long as such objects are still in use. So decompression will stay supported for a long time, and since we need to test decompression support it is also necessary to support compression. > For the future, people will likely move to the more > modern and faster zstd compression algorithm. We're planning doing that once > new binutils is released in openSUSE Tumbleweed. Sure, that's fine. -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-10-04 13:41 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-09-29 13:35 [PATCH] compress .gnu.debuglto_.debug_* sections if requested Martin Liška 2022-09-30 0:20 ` Alan Modra 2022-09-30 7:19 ` Martin Liška 2022-10-04 8:07 ` Alan Modra 2022-10-04 8:12 ` Martin Liška 2022-10-04 13:41 ` Alan Modra
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).