* [PATCH] gas: Add --force-compress-debug-sections @ 2023-02-23 12:45 Tom de Vries 2023-02-23 13:08 ` Jan Beulich 0 siblings, 1 reply; 24+ messages in thread From: Tom de Vries @ 2023-02-23 12:45 UTC (permalink / raw) To: binutils Gas has an option --compress-debug-sections that allows it to generate compressed debug sections. That does not guarantee that the debug sections are in fact compressed: ... $ gcc ~/hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd $ readelf -S -W hello.o | grep " .debug" [ 9] .debug_line PROGBITS 0000a8 000053 00 0 0 1 [11] .debug_line_str PROGBITS 0000fb 000025 01 MS 0 0 1 [12] .debug_info PROGBITS 000120 000039 00 0 0 1 [14] .debug_abbrev PROGBITS 000159 000028 00 0 0 1 [15] .debug_aranges PROGBITS 000190 000030 00 0 0 16 [17] .debug_str PROGBITS 0001c0 000039 01 MS 0 0 1 ... Sensibly so, they're only compressed if that provides a size benefit. However, for the purposes of testing components consuming dwarf we may want the sections to be compressed regardless. Add a new option --force-compress-debug-sections that ignores the size heuristic, such that we have instead: ... $ gcc ~/hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd \ -Wa,--force-compress-debug-sections $ readelf -S -W hello.o | grep " .debug" [ 9] .debug_line PROGBITS 0000a8 000064 00 C 0 0 8 [11] .debug_line_str PROGBITS 000110 000046 01 MSC 0 0 8 [12] .debug_info PROGBITS 000158 000046 00 C 0 0 8 [14] .debug_abbrev PROGBITS 0001a0 000049 00 C 0 0 8 [15] .debug_aranges PROGBITS 0001f0 000034 00 C 0 0 8 [17] .debug_str PROGBITS 000228 00005a 01 MSC 0 0 8 ... Advertised as: ... $ as --help 2>&1 | grep compress --compress-debug-sections[={none|zlib|zlib-gnu|zlib-gabi|zstd}] compress DWARF debug sections --nocompress-debug-sections don't compress DWARF debug sections --force-compress-debug-sections force compression of DWARF debug sections ... Tested on x86_64-linux. --- gas/as.c | 13 ++++++++++++- gas/as.h | 4 ++++ gas/doc/as.texi | 10 ++++++++-- gas/write.c | 4 ++-- 4 files changed, 26 insertions(+), 5 deletions(-) diff --git a/gas/as.c b/gas/as.c index 598bfd56cf5..835c51c609b 100644 --- a/gas/as.c +++ b/gas/as.c @@ -230,6 +230,8 @@ enum compressed_debug_section_type flag_compress_debug = DEFAULT_COMPRESSED_DEBUG_ALGORITHM; #endif +bool flag_force_compress_debug = false; + static void show_usage (FILE * stream) { @@ -262,6 +264,9 @@ Options:\n\ --nocompress-debug-sections\n\ don't compress DWARF debug sections\n")); fprintf (stream, _("\ + --force-compress-debug-sections\n\ + force compression of DWARF debug sections\n")), + fprintf (stream, _("\ -D produce assembler debugging messages\n")); fprintf (stream, _("\ --dump-config display how the assembler is configured and then exit\n")); @@ -508,7 +513,8 @@ parse_args (int * pargc, char *** pargv) OPTION_NOCOMPRESS_DEBUG, OPTION_NO_PAD_SECTIONS, OPTION_MULTIBYTE_HANDLING, /* = STD_BASE + 40 */ - OPTION_SFRAME + OPTION_SFRAME, + OPTION_FORCE_COMPRESS_DEBUG /* When you add options here, check that they do not collide with OPTION_MD_BASE. See as.h. */ }; @@ -528,6 +534,7 @@ parse_args (int * pargc, char *** pargv) ,{"al", optional_argument, NULL, OPTION_AL} ,{"compress-debug-sections", optional_argument, NULL, OPTION_COMPRESS_DEBUG} ,{"nocompress-debug-sections", no_argument, NULL, OPTION_NOCOMPRESS_DEBUG} + ,{"force-compress-debug-sections", no_argument, NULL, OPTION_FORCE_COMPRESS_DEBUG} ,{"debug-prefix-map", required_argument, NULL, OPTION_DEBUG_PREFIX_MAP} ,{"defsym", required_argument, NULL, OPTION_DEFSYM} ,{"dump-config", no_argument, NULL, OPTION_DUMPCONFIG} @@ -771,6 +778,10 @@ This program has absolutely no warranty.\n")); flag_compress_debug = COMPRESS_DEBUG_NONE; break; + case OPTION_FORCE_COMPRESS_DEBUG: + flag_force_compress_debug = true; + break; + case OPTION_DEBUG_PREFIX_MAP: add_debug_prefix_map (optarg); break; diff --git a/gas/as.h b/gas/as.h index 4c5fa9ecf7d..3f8935d2827 100644 --- a/gas/as.h +++ b/gas/as.h @@ -331,6 +331,10 @@ COMMON int flag_traditional_format; /* Type of compressed debug sections we should generate. */ COMMON enum compressed_debug_section_type flag_compress_debug; +/* True if we want to generate compressed debug sections, even if it + doesn't make them smaller. */ +COMMON bool flag_force_compress_debug; + /* TRUE if .note.GNU-stack section with SEC_CODE should be created */ COMMON int flag_execstack; diff --git a/gas/doc/as.texi b/gas/doc/as.texi index bbdfa4bfdca..c853cafbbe5 100644 --- a/gas/doc/as.texi +++ b/gas/doc/as.texi @@ -229,6 +229,7 @@ gcc(1), ld(1), and the Info entries for @file{binutils} and @file{ld}. @value{AS} [@b{-a}[@b{cdghlns}][=@var{file}]] [@b{--alternate}] [@b{--compress-debug-sections}] [@b{--nocompress-debug-sections}] + [@b{--force-compress-debug-sections}] [@b{-D}] [@b{--dump-config}] [@b{--debug-prefix-map} @var{old}=@var{new}] @@ -718,7 +719,8 @@ Begin in alternate macro mode. Compress DWARF debug sections using zlib with SHF_COMPRESSED from the ELF ABI. The resulting object file may not be compatible with older linkers and object file utilities. Note if compression would make a -given section @emph{larger} then it is not compressed. +given section @emph{larger} then it is not compressed, unless +@option{--force-compress-debug-section} is used. @ifset ELF @cindex @samp{--compress-debug-sections=} option @@ -738,7 +740,11 @@ using the obsoleted zlib-gnu format. The debug sections are renamed to begin with @samp{.zdebug}. @option{--compress-debug-sections=zstd} compresses DWARF debug sections using zstd. Note - if compression would actually make a section -@emph{larger}, then it is not compressed nor renamed. +@emph{larger}, then it is not compressed nor renamed, unless +@option{--force-compress-debug-section} is used. + +@item --force-compress-debug-sections +Compress DWARF debug sections, even if this does not reduce size. @end ifset diff --git a/gas/write.c b/gas/write.c index 8273b7a42f1..8fcbc326e3f 100644 --- a/gas/write.c +++ b/gas/write.c @@ -1465,7 +1465,7 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) flagword flags = bfd_section_flags (sec); if (seginfo == NULL - || uncompressed_size < 32 + || (!flag_force_compress_debug && uncompressed_size < 32) || (flags & SEC_HAS_CONTENTS) == 0) return; @@ -1582,7 +1582,7 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) /* PR binutils/18087: If compression didn't make the section smaller, just keep it uncompressed. */ - if (compressed_size >= uncompressed_size) + if (!flag_force_compress_debug && compressed_size >= uncompressed_size) return; /* Replace the uncompressed frag list with the compressed frag list. */ -- 2.35.3 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-23 12:45 [PATCH] gas: Add --force-compress-debug-sections Tom de Vries @ 2023-02-23 13:08 ` Jan Beulich 2023-02-23 13:27 ` Tom de Vries 0 siblings, 1 reply; 24+ messages in thread From: Jan Beulich @ 2023-02-23 13:08 UTC (permalink / raw) To: Tom de Vries; +Cc: binutils On 23.02.2023 13:45, Tom de Vries via Binutils wrote: > Gas has an option --compress-debug-sections that allows it to generate > compressed debug sections. > > That does not guarantee that the debug sections are in fact compressed: > ... > $ gcc ~/hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd > $ readelf -S -W hello.o | grep " .debug" > [ 9] .debug_line PROGBITS 0000a8 000053 00 0 0 1 > [11] .debug_line_str PROGBITS 0000fb 000025 01 MS 0 0 1 > [12] .debug_info PROGBITS 000120 000039 00 0 0 1 > [14] .debug_abbrev PROGBITS 000159 000028 00 0 0 1 > [15] .debug_aranges PROGBITS 000190 000030 00 0 0 16 > [17] .debug_str PROGBITS 0001c0 000039 01 MS 0 0 1 > ... > > Sensibly so, they're only compressed if that provides a size benefit. > > However, for the purposes of testing components consuming dwarf > we may want the sections to be compressed regardless. > > Add a new option --force-compress-debug-sections that ignores the size > heuristic, such that we have instead: > ... > $ gcc ~/hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd \ > -Wa,--force-compress-debug-sections > $ readelf -S -W hello.o | grep " .debug" > [ 9] .debug_line PROGBITS 0000a8 000064 00 C 0 0 8 > [11] .debug_line_str PROGBITS 000110 000046 01 MSC 0 0 8 > [12] .debug_info PROGBITS 000158 000046 00 C 0 0 8 > [14] .debug_abbrev PROGBITS 0001a0 000049 00 C 0 0 8 > [15] .debug_aranges PROGBITS 0001f0 000034 00 C 0 0 8 > [17] .debug_str PROGBITS 000228 00005a 01 MSC 0 0 8 > ... > > Advertised as: > ... > $ as --help 2>&1 | grep compress > --compress-debug-sections[={none|zlib|zlib-gnu|zlib-gabi|zstd}] > compress DWARF debug sections > --nocompress-debug-sections > don't compress DWARF debug sections > --force-compress-debug-sections > force compression of DWARF debug sections No objection in principle, but have you considered making this a new sub-option to --compress-debug-sections, i.e. compress-debug-sections=force? (I've actually been puzzled by --nocompress-debug-sections, which looks to be no different than --compress-debug-sections=none.) Jan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-23 13:08 ` Jan Beulich @ 2023-02-23 13:27 ` Tom de Vries 2023-02-23 13:44 ` Jan Beulich 2023-02-23 15:23 ` Michael Matz 0 siblings, 2 replies; 24+ messages in thread From: Tom de Vries @ 2023-02-23 13:27 UTC (permalink / raw) To: Jan Beulich; +Cc: binutils On 2/23/23 14:08, Jan Beulich wrote: > On 23.02.2023 13:45, Tom de Vries via Binutils wrote: >> Gas has an option --compress-debug-sections that allows it to generate >> compressed debug sections. >> >> That does not guarantee that the debug sections are in fact compressed: >> ... >> $ gcc ~/hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd >> $ readelf -S -W hello.o | grep " .debug" >> [ 9] .debug_line PROGBITS 0000a8 000053 00 0 0 1 >> [11] .debug_line_str PROGBITS 0000fb 000025 01 MS 0 0 1 >> [12] .debug_info PROGBITS 000120 000039 00 0 0 1 >> [14] .debug_abbrev PROGBITS 000159 000028 00 0 0 1 >> [15] .debug_aranges PROGBITS 000190 000030 00 0 0 16 >> [17] .debug_str PROGBITS 0001c0 000039 01 MS 0 0 1 >> ... >> >> Sensibly so, they're only compressed if that provides a size benefit. >> >> However, for the purposes of testing components consuming dwarf >> we may want the sections to be compressed regardless. >> >> Add a new option --force-compress-debug-sections that ignores the size >> heuristic, such that we have instead: >> ... >> $ gcc ~/hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd \ >> -Wa,--force-compress-debug-sections >> $ readelf -S -W hello.o | grep " .debug" >> [ 9] .debug_line PROGBITS 0000a8 000064 00 C 0 0 8 >> [11] .debug_line_str PROGBITS 000110 000046 01 MSC 0 0 8 >> [12] .debug_info PROGBITS 000158 000046 00 C 0 0 8 >> [14] .debug_abbrev PROGBITS 0001a0 000049 00 C 0 0 8 >> [15] .debug_aranges PROGBITS 0001f0 000034 00 C 0 0 8 >> [17] .debug_str PROGBITS 000228 00005a 01 MSC 0 0 8 >> ... >> >> Advertised as: >> ... >> $ as --help 2>&1 | grep compress >> --compress-debug-sections[={none|zlib|zlib-gnu|zlib-gabi|zstd}] >> compress DWARF debug sections >> --nocompress-debug-sections >> don't compress DWARF debug sections >> --force-compress-debug-sections >> force compression of DWARF debug sections > > No objection in principle, but have you considered making this a new > sub-option to --compress-debug-sections, i.e. compress-debug-sections=force? I did consider adding a "force-" prefix variant for all the non-none sub-options, but decided to go with the simplest solution first. Your suggestion, --compress-debug-sections=force is more orthogonal, though it breaks the pattern that all the sub-options are mutually exclusive. We could have it be standalone, so you'd do: --compress-debug-sections=zstd --compress-debug-sections=force. Or instead combined: --compress-debug-sections=force,zstd. Harder to parse though, I suppose. I guess this last one would be my preference, because it makes it clear force is in a different category than zlib/zstd. Thanks, - Tom > (I've actually been puzzled by --nocompress-debug-sections, which looks to > be no different than --compress-debug-sections=none.) ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-23 13:27 ` Tom de Vries @ 2023-02-23 13:44 ` Jan Beulich 2023-02-24 10:52 ` Tom de Vries 2023-02-23 15:23 ` Michael Matz 1 sibling, 1 reply; 24+ messages in thread From: Jan Beulich @ 2023-02-23 13:44 UTC (permalink / raw) To: Tom de Vries; +Cc: binutils On 23.02.2023 14:27, Tom de Vries wrote: > On 2/23/23 14:08, Jan Beulich wrote: >> On 23.02.2023 13:45, Tom de Vries via Binutils wrote: >>> Gas has an option --compress-debug-sections that allows it to generate >>> compressed debug sections. >>> >>> That does not guarantee that the debug sections are in fact compressed: >>> ... >>> $ gcc ~/hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd >>> $ readelf -S -W hello.o | grep " .debug" >>> [ 9] .debug_line PROGBITS 0000a8 000053 00 0 0 1 >>> [11] .debug_line_str PROGBITS 0000fb 000025 01 MS 0 0 1 >>> [12] .debug_info PROGBITS 000120 000039 00 0 0 1 >>> [14] .debug_abbrev PROGBITS 000159 000028 00 0 0 1 >>> [15] .debug_aranges PROGBITS 000190 000030 00 0 0 16 >>> [17] .debug_str PROGBITS 0001c0 000039 01 MS 0 0 1 >>> ... >>> >>> Sensibly so, they're only compressed if that provides a size benefit. >>> >>> However, for the purposes of testing components consuming dwarf >>> we may want the sections to be compressed regardless. >>> >>> Add a new option --force-compress-debug-sections that ignores the size >>> heuristic, such that we have instead: >>> ... >>> $ gcc ~/hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd \ >>> -Wa,--force-compress-debug-sections >>> $ readelf -S -W hello.o | grep " .debug" >>> [ 9] .debug_line PROGBITS 0000a8 000064 00 C 0 0 8 >>> [11] .debug_line_str PROGBITS 000110 000046 01 MSC 0 0 8 >>> [12] .debug_info PROGBITS 000158 000046 00 C 0 0 8 >>> [14] .debug_abbrev PROGBITS 0001a0 000049 00 C 0 0 8 >>> [15] .debug_aranges PROGBITS 0001f0 000034 00 C 0 0 8 >>> [17] .debug_str PROGBITS 000228 00005a 01 MSC 0 0 8 >>> ... >>> >>> Advertised as: >>> ... >>> $ as --help 2>&1 | grep compress >>> --compress-debug-sections[={none|zlib|zlib-gnu|zlib-gabi|zstd}] >>> compress DWARF debug sections >>> --nocompress-debug-sections >>> don't compress DWARF debug sections >>> --force-compress-debug-sections >>> force compression of DWARF debug sections >> >> No objection in principle, but have you considered making this a new >> sub-option to --compress-debug-sections, i.e. compress-debug-sections=force? > > I did consider adding a "force-" prefix variant for all the non-none > sub-options, but decided to go with the simplest solution first. > > Your suggestion, --compress-debug-sections=force is more orthogonal, > though it breaks the pattern that all the sub-options are mutually > exclusive. > > We could have it be standalone, so you'd do: > --compress-debug-sections=zstd --compress-debug-sections=force. > > Or instead combined: --compress-debug-sections=force,zstd. Harder to > parse though, I suppose. I think both should be allowed. In a complex build system it may be different entities setting "how" and "whether". (To me "none" falls in the "whether" category together with "force", and it also can be seen as falling in the "how" category together with "zlib" etc. In Linux Kconfig, for example, I'd see this being expressed as first a "whether" choice [yes/maybe/forced] and then a "how" choice dependent upon "whether != none".) Jan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-23 13:44 ` Jan Beulich @ 2023-02-24 10:52 ` Tom de Vries 2023-02-24 11:28 ` Jan Beulich 0 siblings, 1 reply; 24+ messages in thread From: Tom de Vries @ 2023-02-24 10:52 UTC (permalink / raw) To: Jan Beulich; +Cc: binutils, Michael Matz [-- Attachment #1: Type: text/plain, Size: 3597 bytes --] On 2/23/23 14:44, Jan Beulich wrote: > On 23.02.2023 14:27, Tom de Vries wrote: >> On 2/23/23 14:08, Jan Beulich wrote: >>> On 23.02.2023 13:45, Tom de Vries via Binutils wrote: >>>> Gas has an option --compress-debug-sections that allows it to generate >>>> compressed debug sections. >>>> >>>> That does not guarantee that the debug sections are in fact compressed: >>>> ... >>>> $ gcc ~/hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd >>>> $ readelf -S -W hello.o | grep " .debug" >>>> [ 9] .debug_line PROGBITS 0000a8 000053 00 0 0 1 >>>> [11] .debug_line_str PROGBITS 0000fb 000025 01 MS 0 0 1 >>>> [12] .debug_info PROGBITS 000120 000039 00 0 0 1 >>>> [14] .debug_abbrev PROGBITS 000159 000028 00 0 0 1 >>>> [15] .debug_aranges PROGBITS 000190 000030 00 0 0 16 >>>> [17] .debug_str PROGBITS 0001c0 000039 01 MS 0 0 1 >>>> ... >>>> >>>> Sensibly so, they're only compressed if that provides a size benefit. >>>> >>>> However, for the purposes of testing components consuming dwarf >>>> we may want the sections to be compressed regardless. >>>> >>>> Add a new option --force-compress-debug-sections that ignores the size >>>> heuristic, such that we have instead: >>>> ... >>>> $ gcc ~/hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd \ >>>> -Wa,--force-compress-debug-sections >>>> $ readelf -S -W hello.o | grep " .debug" >>>> [ 9] .debug_line PROGBITS 0000a8 000064 00 C 0 0 8 >>>> [11] .debug_line_str PROGBITS 000110 000046 01 MSC 0 0 8 >>>> [12] .debug_info PROGBITS 000158 000046 00 C 0 0 8 >>>> [14] .debug_abbrev PROGBITS 0001a0 000049 00 C 0 0 8 >>>> [15] .debug_aranges PROGBITS 0001f0 000034 00 C 0 0 8 >>>> [17] .debug_str PROGBITS 000228 00005a 01 MSC 0 0 8 >>>> ... >>>> >>>> Advertised as: >>>> ... >>>> $ as --help 2>&1 | grep compress >>>> --compress-debug-sections[={none|zlib|zlib-gnu|zlib-gabi|zstd}] >>>> compress DWARF debug sections >>>> --nocompress-debug-sections >>>> don't compress DWARF debug sections >>>> --force-compress-debug-sections >>>> force compression of DWARF debug sections >>> >>> No objection in principle, but have you considered making this a new >>> sub-option to --compress-debug-sections, i.e. compress-debug-sections=force? >> >> I did consider adding a "force-" prefix variant for all the non-none >> sub-options, but decided to go with the simplest solution first. >> >> Your suggestion, --compress-debug-sections=force is more orthogonal, >> though it breaks the pattern that all the sub-options are mutually >> exclusive. >> >> We could have it be standalone, so you'd do: >> --compress-debug-sections=zstd --compress-debug-sections=force. >> >> Or instead combined: --compress-debug-sections=force,zstd. Harder to >> parse though, I suppose. > > I think both should be allowed. In a complex build system it may be > different entities setting "how" and "whether". (To me "none" falls in > the "whether" category together with "force", and it also can be seen > as falling in the "how" category together with "zlib" etc. In Linux > Kconfig, for example, I'd see this being expressed as first a "whether" > choice [yes/maybe/forced] and then a "how" choice dependent upon > "whether != none".) > I gave this approach a try. Thanks, - Tom [-- Attachment #2: 0001-gas-Add-compress-debug-sections-force.patch --] [-- Type: text/x-patch, Size: 11188 bytes --] From 792f364b885126b83ad50619085426aca1bc308b Mon Sep 17 00:00:00 2001 From: Tom de Vries <tdevries@suse.de> Date: Thu, 23 Feb 2023 12:53:40 +0100 Subject: [PATCH] gas: Add --compress-debug-sections=force Gas has an option --compress-debug-sections that allows it to generate compressed debug sections. That does not guarantee that the debug sections are in fact compressed: ... $ gcc hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd $ readelf -S -W hello.o | grep " .debug" [ 9] .debug_line PROGBITS 0000a8 000053 00 0 0 1 [11] .debug_line_str PROGBITS 0000fb 000025 01 MS 0 0 1 [12] .debug_info PROGBITS 000120 000039 00 0 0 1 [14] .debug_abbrev PROGBITS 000159 000028 00 0 0 1 [15] .debug_aranges PROGBITS 000190 000030 00 0 0 16 [17] .debug_str PROGBITS 0001c0 000039 01 MS 0 0 1 ... Sensibly so, they're only compressed if that provides a size benefit. However, for the purpose of testing components consuming dwarf we may want the sections to be compressed regardless. Add a new suboption --compress-debug-sections=force that ignores the size heuristic, such that we have instead: ... $ gcc hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd+force $ readelf -S -W hello.o | grep " .debug" [ 9] .debug_line PROGBITS 0000a8 000064 00 C 0 0 8 [11] .debug_line_str PROGBITS 000110 000046 01 MSC 0 0 8 [12] .debug_info PROGBITS 000158 000046 00 C 0 0 8 [14] .debug_abbrev PROGBITS 0001a0 000049 00 C 0 0 8 [15] .debug_aranges PROGBITS 0001f0 000034 00 C 0 0 8 [17] .debug_str PROGBITS 000228 00005a 01 MSC 0 0 8 ... Advertised as: ... $ as --help ... --compress-debug-sections[={none|<format>|force|force+<format>}] where <format> is {zlib|zlib-gnu|zlib-gabi|zstd} compress DWARF debug sections Default: zstd ... Tested on x86_64-linux. --- gas/as.c | 158 ++++++++++++++++++++++++++++++++++++++++++------ gas/as.h | 4 ++ gas/doc/as.texi | 10 ++- gas/write.c | 4 +- 4 files changed, 153 insertions(+), 23 deletions(-) diff --git a/gas/as.c b/gas/as.c index 598bfd56cf5..3c38b7a45e9 100644 --- a/gas/as.c +++ b/gas/as.c @@ -230,6 +230,18 @@ enum compressed_debug_section_type flag_compress_debug = DEFAULT_COMPRESSED_DEBUG_ALGORITHM; #endif +enum compress_debug_action +{ + cda_default, + cda_none, + cda_force, + cda_yes, +}; +static enum compress_debug_action compress_debug_action + = cda_default; + +bool force_compress_debug = false; + static void show_usage (FILE * stream) { @@ -252,7 +264,8 @@ Options:\n\ fprintf (stream, _("\ --alternate initially turn on alternate macro syntax\n")); fprintf (stream, _("\ - --compress-debug-sections[={none|zlib|zlib-gnu|zlib-gabi|zstd}]\n\ + --compress-debug-sections[={none|<format>|force|force+<format>}]\n\ + where <format> is {zlib|zlib-gnu|zlib-gabi|zstd}\n\ compress DWARF debug sections\n")), fprintf (stream, _("\ Default: %s\n"), @@ -418,6 +431,126 @@ Options:\n\ fprintf (stream, _("Report bugs to %s\n"), REPORT_BUGS_TO); } +static void +parse_compress_debug_optarg_1 (const char *optarg, unsigned int *none_cnt, + unsigned int *force_cnt, + unsigned int *format_cnt, + enum compressed_debug_section_type *format) +{ + gas_assert (optarg != NULL); + + if (strcmp (optarg, "force") == 0) + { + ++*force_cnt; + return; + } + + enum compressed_debug_section_type tmp + = bfd_get_compression_algorithm (optarg); + +#ifndef HAVE_ZSTD + if (tmp == COMPRESS_DEBUG_ZSTD) + as_fatal (_ ("--compress-debug-sections=zstd: gas is not " + "built with zstd support")); +#endif + + if (tmp == COMPRESS_UNKNOWN) + as_fatal (_("Invalid --compress-debug-sections option: `%s'"), + optarg); + + if (tmp == COMPRESS_DEBUG_NONE) + { + ++*none_cnt; + return; + } + + *format = tmp; + ++*format_cnt; +} + +static void +parse_compress_debug_optarg (const char *optarg) +{ + const char *orig_optarg = optarg; + +#if !defined OBJ_ELF && !defined OBJ_MAYBE_ELF + as_fatal (_("--compress-debug-sections=%s is unsupported"), + optarg); +#endif + + /* Tokenize subopts seperated by '+' and pass to + parse_compress_debug_optarg_1. */ + unsigned int none_cnt = 0; + unsigned int force_cnt = 0; + unsigned int format_cnt = 0; + enum compressed_debug_section_type format = COMPRESS_UNKNOWN; + while (true) + { + const char *idx = optarg; + while (*idx != '\0' && *idx != '+') + idx++; + + size_t len = idx - optarg; + if (len == 0) + { + /* Generate error. */ + parse_compress_debug_optarg_1 ("", NULL, NULL, NULL, NULL); + break; + } + + char *tmp = xstrndup (optarg, len); + parse_compress_debug_optarg_1 (tmp, &none_cnt, &force_cnt, &format_cnt, + &format); + free (tmp); + + if (*idx == '\0') + break; + + /* Step over '+' and continue tokenizing. */ + gas_assert (*idx == '+'); + optarg = idx + 1; + } + + bool valid_p = true; + if (force_cnt >= 2 || none_cnt >= 2 || format_cnt >= 2) + /* Not allowed: force+force, none+none, zstd+zstd, zstd+zlib. */ + valid_p = false; + else if (none_cnt == 1 && (force_cnt > 0 || format_cnt > 0)) + /* Not allowed: none+force, none+zstd. */ + valid_p = false; + + if (!valid_p) + as_fatal (_("Invalid --compress-debug-sections option combination: `%s'"), + orig_optarg); + + if (none_cnt == 1) + /* Let --compress-debug-section=none override + --compress-debug-section and --compress-debug-section=force. */ + compress_debug_action = cda_none; + else if (force_cnt == 1) + /* Let --compress-debug-section=force override + --compress-debug-section and --compress-debug-section=none. */ + compress_debug_action = cda_force; + else + /* Let --compress-debug-section override + --compress-debug-section=none and --compress-debug-section=force. */ + compress_debug_action = cda_yes; + + if (format_cnt == 1) + /* Let --compress-debug-section=zlib override + --compress-debug-section=zstd. */ + flag_compress_debug = format; +} + +static void +finalize_parse_compress_debug_optarg (void) +{ + if (compress_debug_action == cda_none) + flag_compress_debug = COMPRESS_DEBUG_NONE; + else if (compress_debug_action == cda_force) + force_compress_debug = true; +} + /* Since it is easy to do here we interpret the special arg "-" to mean "use stdin" and we set that argv[] pointing to "". After we have munged argv[], the only things left are source file @@ -747,28 +880,13 @@ This program has absolutely no warranty.\n")); case OPTION_COMPRESS_DEBUG: if (optarg) - { -#if defined OBJ_ELF || defined OBJ_MAYBE_ELF - flag_compress_debug = bfd_get_compression_algorithm (optarg); -#ifndef HAVE_ZSTD - if (flag_compress_debug == COMPRESS_DEBUG_ZSTD) - as_fatal (_ ("--compress-debug-sections=zstd: gas is not " - "built with zstd support")); -#endif - if (flag_compress_debug == COMPRESS_UNKNOWN) - as_fatal (_("Invalid --compress-debug-sections option: `%s'"), - optarg); -#else - as_fatal (_("--compress-debug-sections=%s is unsupported"), - optarg); -#endif - } + parse_compress_debug_optarg (optarg); else - flag_compress_debug = COMPRESS_DEBUG_GABI_ZLIB; + parse_compress_debug_optarg ("zlib-gabi"); break; case OPTION_NOCOMPRESS_DEBUG: - flag_compress_debug = COMPRESS_DEBUG_NONE; + parse_compress_debug_optarg ("none"); break; case OPTION_DEBUG_PREFIX_MAP: @@ -1136,6 +1254,8 @@ This program has absolutely no warranty.\n")); *pargc = new_argc; *pargv = new_argv; + finalize_parse_compress_debug_optarg (); + #ifdef md_after_parse_args md_after_parse_args (); #endif diff --git a/gas/as.h b/gas/as.h index 4c5fa9ecf7d..115af019815 100644 --- a/gas/as.h +++ b/gas/as.h @@ -331,6 +331,10 @@ COMMON int flag_traditional_format; /* Type of compressed debug sections we should generate. */ COMMON enum compressed_debug_section_type flag_compress_debug; +/* True if we want to generate compressed debug sections, even if it + doesn't make them smaller. */ +COMMON bool force_compress_debug; + /* TRUE if .note.GNU-stack section with SEC_CODE should be created */ COMMON int flag_execstack; diff --git a/gas/doc/as.texi b/gas/doc/as.texi index bbdfa4bfdca..d25559141c7 100644 --- a/gas/doc/as.texi +++ b/gas/doc/as.texi @@ -718,7 +718,8 @@ Begin in alternate macro mode. Compress DWARF debug sections using zlib with SHF_COMPRESSED from the ELF ABI. The resulting object file may not be compatible with older linkers and object file utilities. Note if compression would make a -given section @emph{larger} then it is not compressed. +given section @emph{larger} then it is not compressed, unless +@option{--compress-debug-section=force} is used. @ifset ELF @cindex @samp{--compress-debug-sections=} option @@ -727,6 +728,7 @@ given section @emph{larger} then it is not compressed. @itemx --compress-debug-sections=zlib-gnu @itemx --compress-debug-sections=zlib-gabi @itemx --compress-debug-sections=zstd +@itemx --compress-debug-sections=force These options control how DWARF debug sections are compressed. @option{--compress-debug-sections=none} is equivalent to @option{--nocompress-debug-sections}. @@ -738,7 +740,11 @@ using the obsoleted zlib-gnu format. The debug sections are renamed to begin with @samp{.zdebug}. @option{--compress-debug-sections=zstd} compresses DWARF debug sections using zstd. Note - if compression would actually make a section -@emph{larger}, then it is not compressed nor renamed. +@emph{larger}, then it is not compressed nor renamed, unless +@option{--compress-debug-section=force} is used. +@option{--compress-debug-sections=force} compresses DWARF debug sections, +even if this does not reduce size. It can be used in conjunction with a format +selection, for instance @option{--compress-debug-section=zstd+force}. @end ifset diff --git a/gas/write.c b/gas/write.c index 8273b7a42f1..39bcea23fac 100644 --- a/gas/write.c +++ b/gas/write.c @@ -1465,7 +1465,7 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) flagword flags = bfd_section_flags (sec); if (seginfo == NULL - || uncompressed_size < 32 + || (!force_compress_debug && uncompressed_size < 32) || (flags & SEC_HAS_CONTENTS) == 0) return; @@ -1582,7 +1582,7 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) /* PR binutils/18087: If compression didn't make the section smaller, just keep it uncompressed. */ - if (compressed_size >= uncompressed_size) + if (!force_compress_debug && compressed_size >= uncompressed_size) return; /* Replace the uncompressed frag list with the compressed frag list. */ -- 2.35.3 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-24 10:52 ` Tom de Vries @ 2023-02-24 11:28 ` Jan Beulich 2023-02-24 12:21 ` Tom de Vries 0 siblings, 1 reply; 24+ messages in thread From: Jan Beulich @ 2023-02-24 11:28 UTC (permalink / raw) To: Tom de Vries; +Cc: binutils, Michael Matz On 24.02.2023 11:52, Tom de Vries wrote: > On 2/23/23 14:44, Jan Beulich wrote: >> On 23.02.2023 14:27, Tom de Vries wrote: >>> On 2/23/23 14:08, Jan Beulich wrote: >>>> On 23.02.2023 13:45, Tom de Vries via Binutils wrote: >>>>> Gas has an option --compress-debug-sections that allows it to generate >>>>> compressed debug sections. >>>>> >>>>> That does not guarantee that the debug sections are in fact compressed: >>>>> ... >>>>> $ gcc ~/hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd >>>>> $ readelf -S -W hello.o | grep " .debug" >>>>> [ 9] .debug_line PROGBITS 0000a8 000053 00 0 0 1 >>>>> [11] .debug_line_str PROGBITS 0000fb 000025 01 MS 0 0 1 >>>>> [12] .debug_info PROGBITS 000120 000039 00 0 0 1 >>>>> [14] .debug_abbrev PROGBITS 000159 000028 00 0 0 1 >>>>> [15] .debug_aranges PROGBITS 000190 000030 00 0 0 16 >>>>> [17] .debug_str PROGBITS 0001c0 000039 01 MS 0 0 1 >>>>> ... >>>>> >>>>> Sensibly so, they're only compressed if that provides a size benefit. >>>>> >>>>> However, for the purposes of testing components consuming dwarf >>>>> we may want the sections to be compressed regardless. >>>>> >>>>> Add a new option --force-compress-debug-sections that ignores the size >>>>> heuristic, such that we have instead: >>>>> ... >>>>> $ gcc ~/hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd \ >>>>> -Wa,--force-compress-debug-sections >>>>> $ readelf -S -W hello.o | grep " .debug" >>>>> [ 9] .debug_line PROGBITS 0000a8 000064 00 C 0 0 8 >>>>> [11] .debug_line_str PROGBITS 000110 000046 01 MSC 0 0 8 >>>>> [12] .debug_info PROGBITS 000158 000046 00 C 0 0 8 >>>>> [14] .debug_abbrev PROGBITS 0001a0 000049 00 C 0 0 8 >>>>> [15] .debug_aranges PROGBITS 0001f0 000034 00 C 0 0 8 >>>>> [17] .debug_str PROGBITS 000228 00005a 01 MSC 0 0 8 >>>>> ... >>>>> >>>>> Advertised as: >>>>> ... >>>>> $ as --help 2>&1 | grep compress >>>>> --compress-debug-sections[={none|zlib|zlib-gnu|zlib-gabi|zstd}] >>>>> compress DWARF debug sections >>>>> --nocompress-debug-sections >>>>> don't compress DWARF debug sections >>>>> --force-compress-debug-sections >>>>> force compression of DWARF debug sections >>>> >>>> No objection in principle, but have you considered making this a new >>>> sub-option to --compress-debug-sections, i.e. compress-debug-sections=force? >>> >>> I did consider adding a "force-" prefix variant for all the non-none >>> sub-options, but decided to go with the simplest solution first. >>> >>> Your suggestion, --compress-debug-sections=force is more orthogonal, >>> though it breaks the pattern that all the sub-options are mutually >>> exclusive. >>> >>> We could have it be standalone, so you'd do: >>> --compress-debug-sections=zstd --compress-debug-sections=force. >>> >>> Or instead combined: --compress-debug-sections=force,zstd. Harder to >>> parse though, I suppose. >> >> I think both should be allowed. In a complex build system it may be >> different entities setting "how" and "whether". (To me "none" falls in >> the "whether" category together with "force", and it also can be seen >> as falling in the "how" category together with "zlib" etc. In Linux >> Kconfig, for example, I'd see this being expressed as first a "whether" >> choice [yes/maybe/forced] and then a "how" choice dependent upon >> "whether != none".) >> > > I gave this approach a try. Any specific reason you chose + as the separator instead of the more conventional , ? I also wouldn't see anything wrong with something like "...=force,zstd,none" - the last one(s) win. That's no different from specifying a second instance of the option. And without that it looks as if the parsing would end up simpler. Jan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-24 11:28 ` Jan Beulich @ 2023-02-24 12:21 ` Tom de Vries 2023-02-24 13:23 ` Jan Beulich 0 siblings, 1 reply; 24+ messages in thread From: Tom de Vries @ 2023-02-24 12:21 UTC (permalink / raw) To: Jan Beulich; +Cc: binutils, Michael Matz [-- Attachment #1: Type: text/plain, Size: 4531 bytes --] On 2/24/23 12:28, Jan Beulich wrote: > On 24.02.2023 11:52, Tom de Vries wrote: >> On 2/23/23 14:44, Jan Beulich wrote: >>> On 23.02.2023 14:27, Tom de Vries wrote: >>>> On 2/23/23 14:08, Jan Beulich wrote: >>>>> On 23.02.2023 13:45, Tom de Vries via Binutils wrote: >>>>>> Gas has an option --compress-debug-sections that allows it to generate >>>>>> compressed debug sections. >>>>>> >>>>>> That does not guarantee that the debug sections are in fact compressed: >>>>>> ... >>>>>> $ gcc ~/hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd >>>>>> $ readelf -S -W hello.o | grep " .debug" >>>>>> [ 9] .debug_line PROGBITS 0000a8 000053 00 0 0 1 >>>>>> [11] .debug_line_str PROGBITS 0000fb 000025 01 MS 0 0 1 >>>>>> [12] .debug_info PROGBITS 000120 000039 00 0 0 1 >>>>>> [14] .debug_abbrev PROGBITS 000159 000028 00 0 0 1 >>>>>> [15] .debug_aranges PROGBITS 000190 000030 00 0 0 16 >>>>>> [17] .debug_str PROGBITS 0001c0 000039 01 MS 0 0 1 >>>>>> ... >>>>>> >>>>>> Sensibly so, they're only compressed if that provides a size benefit. >>>>>> >>>>>> However, for the purposes of testing components consuming dwarf >>>>>> we may want the sections to be compressed regardless. >>>>>> >>>>>> Add a new option --force-compress-debug-sections that ignores the size >>>>>> heuristic, such that we have instead: >>>>>> ... >>>>>> $ gcc ~/hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd \ >>>>>> -Wa,--force-compress-debug-sections >>>>>> $ readelf -S -W hello.o | grep " .debug" >>>>>> [ 9] .debug_line PROGBITS 0000a8 000064 00 C 0 0 8 >>>>>> [11] .debug_line_str PROGBITS 000110 000046 01 MSC 0 0 8 >>>>>> [12] .debug_info PROGBITS 000158 000046 00 C 0 0 8 >>>>>> [14] .debug_abbrev PROGBITS 0001a0 000049 00 C 0 0 8 >>>>>> [15] .debug_aranges PROGBITS 0001f0 000034 00 C 0 0 8 >>>>>> [17] .debug_str PROGBITS 000228 00005a 01 MSC 0 0 8 >>>>>> ... >>>>>> >>>>>> Advertised as: >>>>>> ... >>>>>> $ as --help 2>&1 | grep compress >>>>>> --compress-debug-sections[={none|zlib|zlib-gnu|zlib-gabi|zstd}] >>>>>> compress DWARF debug sections >>>>>> --nocompress-debug-sections >>>>>> don't compress DWARF debug sections >>>>>> --force-compress-debug-sections >>>>>> force compression of DWARF debug sections >>>>> >>>>> No objection in principle, but have you considered making this a new >>>>> sub-option to --compress-debug-sections, i.e. compress-debug-sections=force? >>>> >>>> I did consider adding a "force-" prefix variant for all the non-none >>>> sub-options, but decided to go with the simplest solution first. >>>> >>>> Your suggestion, --compress-debug-sections=force is more orthogonal, >>>> though it breaks the pattern that all the sub-options are mutually >>>> exclusive. >>>> >>>> We could have it be standalone, so you'd do: >>>> --compress-debug-sections=zstd --compress-debug-sections=force. >>>> >>>> Or instead combined: --compress-debug-sections=force,zstd. Harder to >>>> parse though, I suppose. >>> >>> I think both should be allowed. In a complex build system it may be >>> different entities setting "how" and "whether". (To me "none" falls in >>> the "whether" category together with "force", and it also can be seen >>> as falling in the "how" category together with "zlib" etc. In Linux >>> Kconfig, for example, I'd see this being expressed as first a "whether" >>> choice [yes/maybe/forced] and then a "how" choice dependent upon >>> "whether != none".) >>> >> >> I gave this approach a try. > > Any specific reason you chose + as the separator instead of the more > conventional , ? Yes, I initially went for ',', but ran into: ... $ gcc ~/hello.c -Wa,-gdwarf-5 \ -Wa,--compress-debug-sections=zstd,force -c -v ... as -v --64 -gdwarf-5 --compress-debug-sections=zstd force -o hello.o \ /tmp/ccOUMqHL.s ... Assembler messages: Error: can't open force for reading: No such file or directory ... > I also wouldn't see anything wrong with something > like "...=force,zstd,none" - the last one(s) win. That's no different > from specifying a second instance of the option. And without that it > looks as if the parsing would end up simpler. OK, gave that a try. Thanks, - Tom [-- Attachment #2: 0001-gas-Add-compress-debug-sections-force.patch --] [-- Type: text/x-patch, Size: 10115 bytes --] From f23c41372cb7d48116ce51f99b9f265248b2d7d4 Mon Sep 17 00:00:00 2001 From: Tom de Vries <tdevries@suse.de> Date: Thu, 23 Feb 2023 12:53:40 +0100 Subject: [PATCH] gas: Add --compress-debug-sections=force Gas has an option --compress-debug-sections that allows it to generate compressed debug sections. That does not guarantee that the debug sections are in fact compressed: ... $ gcc hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd $ readelf -S -W hello.o | grep " .debug" [ 9] .debug_line PROGBITS 0000a8 000053 00 0 0 1 [11] .debug_line_str PROGBITS 0000fb 000025 01 MS 0 0 1 [12] .debug_info PROGBITS 000120 000039 00 0 0 1 [14] .debug_abbrev PROGBITS 000159 000028 00 0 0 1 [15] .debug_aranges PROGBITS 000190 000030 00 0 0 16 [17] .debug_str PROGBITS 0001c0 000039 01 MS 0 0 1 ... Sensibly so, they're only compressed if that provides a size benefit. However, for the purpose of testing components consuming dwarf we may want the sections to be compressed regardless. Add a new suboption --compress-debug-sections=force that ignores the size heuristic, such that we have instead: ... $ gcc hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd+force $ readelf -S -W hello.o | grep " .debug" [ 9] .debug_line PROGBITS 0000a8 000064 00 C 0 0 8 [11] .debug_line_str PROGBITS 000110 000046 01 MSC 0 0 8 [12] .debug_info PROGBITS 000158 000046 00 C 0 0 8 [14] .debug_abbrev PROGBITS 0001a0 000049 00 C 0 0 8 [15] .debug_aranges PROGBITS 0001f0 000034 00 C 0 0 8 [17] .debug_str PROGBITS 000228 00005a 01 MSC 0 0 8 ... Advertised as: ... $ as --help ... --compress-debug-sections[={none|<format>|force|force+<format>}] where <format> is {zlib|zlib-gnu|zlib-gabi|zstd} compress DWARF debug sections Default: zstd ... Tested on x86_64-linux. --- gas/as.c | 133 +++++++++++++++++++++++++++++++++++++++++------- gas/as.h | 4 ++ gas/doc/as.texi | 10 +++- gas/write.c | 4 +- 4 files changed, 128 insertions(+), 23 deletions(-) diff --git a/gas/as.c b/gas/as.c index 598bfd56cf5..0f4d43f256d 100644 --- a/gas/as.c +++ b/gas/as.c @@ -230,6 +230,18 @@ enum compressed_debug_section_type flag_compress_debug = DEFAULT_COMPRESSED_DEBUG_ALGORITHM; #endif +enum compress_debug_action +{ + cda_default, + cda_none, + cda_force, + cda_yes, +}; +static enum compress_debug_action compress_debug_action + = cda_default; + +bool force_compress_debug = false; + static void show_usage (FILE * stream) { @@ -252,7 +264,8 @@ Options:\n\ fprintf (stream, _("\ --alternate initially turn on alternate macro syntax\n")); fprintf (stream, _("\ - --compress-debug-sections[={none|zlib|zlib-gnu|zlib-gabi|zstd}]\n\ + --compress-debug-sections[={none|<format>|force|force+<format>}]\n\ + where <format> is {zlib|zlib-gnu|zlib-gabi|zstd}\n\ compress DWARF debug sections\n")), fprintf (stream, _("\ Default: %s\n"), @@ -418,6 +431,101 @@ Options:\n\ fprintf (stream, _("Report bugs to %s\n"), REPORT_BUGS_TO); } +static void +parse_compress_debug_optarg_1 (const char *optarg, bool *none, bool *force, + enum compressed_debug_section_type *format) +{ + gas_assert (optarg != NULL); + + if (strcmp (optarg, "force") == 0) + { + *force = true; + *none = false; + return; + } + + enum compressed_debug_section_type tmp + = bfd_get_compression_algorithm (optarg); + +#ifndef HAVE_ZSTD + if (tmp == COMPRESS_DEBUG_ZSTD) + as_fatal (_ ("--compress-debug-sections=zstd: gas is not " + "built with zstd support")); +#endif + + if (tmp == COMPRESS_UNKNOWN) + as_fatal (_("Invalid --compress-debug-sections option: `%s'"), + optarg); + + if (tmp == COMPRESS_DEBUG_NONE) + { + *none = true; + *force = false; + return; + } + + *format = tmp; +} + +static void +parse_compress_debug_optarg (const char *optarg) +{ +#if !defined OBJ_ELF && !defined OBJ_MAYBE_ELF + as_fatal (_("--compress-debug-sections=%s is unsupported"), + optarg); +#endif + + /* Tokenize subopts seperated by '+' and pass to + parse_compress_debug_optarg_1. */ + bool none = false; + bool force = false; + enum compressed_debug_section_type format = COMPRESS_UNKNOWN; + while (true) + { + const char *idx = optarg; + while (*idx != '\0' && *idx != '+') + idx++; + + size_t len = idx - optarg; + if (len == 0) + { + /* Generate error. */ + parse_compress_debug_optarg_1 ("", NULL, NULL, NULL); + break; + } + + char *tmp = xstrndup (optarg, len); + parse_compress_debug_optarg_1 (tmp, &none, &force, &format); + free (tmp); + + if (*idx == '\0') + break; + + /* Step over '+' and continue tokenizing. */ + gas_assert (*idx == '+'); + optarg = idx + 1; + } + + if (none) + compress_debug_action = cda_none; + else if (force) + compress_debug_action = cda_force; + else + compress_debug_action = cda_yes; + + if (format != COMPRESS_UNKNOWN) + flag_compress_debug = format; +} + +static void +finalize_parse_compress_debug_optarg (void) +{ + if (compress_debug_action == cda_none) + flag_compress_debug = COMPRESS_DEBUG_NONE; + else if (compress_debug_action == cda_force) + force_compress_debug = true; +} + /* Since it is easy to do here we interpret the special arg "-" to mean "use stdin" and we set that argv[] pointing to "". After we have munged argv[], the only things left are source file @@ -747,28 +855,13 @@ This program has absolutely no warranty.\n")); case OPTION_COMPRESS_DEBUG: if (optarg) - { -#if defined OBJ_ELF || defined OBJ_MAYBE_ELF - flag_compress_debug = bfd_get_compression_algorithm (optarg); -#ifndef HAVE_ZSTD - if (flag_compress_debug == COMPRESS_DEBUG_ZSTD) - as_fatal (_ ("--compress-debug-sections=zstd: gas is not " - "built with zstd support")); -#endif - if (flag_compress_debug == COMPRESS_UNKNOWN) - as_fatal (_("Invalid --compress-debug-sections option: `%s'"), - optarg); -#else - as_fatal (_("--compress-debug-sections=%s is unsupported"), - optarg); -#endif - } + parse_compress_debug_optarg (optarg); else - flag_compress_debug = COMPRESS_DEBUG_GABI_ZLIB; + parse_compress_debug_optarg ("zlib-gabi"); break; case OPTION_NOCOMPRESS_DEBUG: - flag_compress_debug = COMPRESS_DEBUG_NONE; + parse_compress_debug_optarg ("none"); break; case OPTION_DEBUG_PREFIX_MAP: @@ -1136,6 +1229,8 @@ This program has absolutely no warranty.\n")); *pargc = new_argc; *pargv = new_argv; + finalize_parse_compress_debug_optarg (); + #ifdef md_after_parse_args md_after_parse_args (); #endif diff --git a/gas/as.h b/gas/as.h index 4c5fa9ecf7d..115af019815 100644 --- a/gas/as.h +++ b/gas/as.h @@ -331,6 +331,10 @@ COMMON int flag_traditional_format; /* Type of compressed debug sections we should generate. */ COMMON enum compressed_debug_section_type flag_compress_debug; +/* True if we want to generate compressed debug sections, even if it + doesn't make them smaller. */ +COMMON bool force_compress_debug; + /* TRUE if .note.GNU-stack section with SEC_CODE should be created */ COMMON int flag_execstack; diff --git a/gas/doc/as.texi b/gas/doc/as.texi index bbdfa4bfdca..d25559141c7 100644 --- a/gas/doc/as.texi +++ b/gas/doc/as.texi @@ -718,7 +718,8 @@ Begin in alternate macro mode. Compress DWARF debug sections using zlib with SHF_COMPRESSED from the ELF ABI. The resulting object file may not be compatible with older linkers and object file utilities. Note if compression would make a -given section @emph{larger} then it is not compressed. +given section @emph{larger} then it is not compressed, unless +@option{--compress-debug-section=force} is used. @ifset ELF @cindex @samp{--compress-debug-sections=} option @@ -727,6 +728,7 @@ given section @emph{larger} then it is not compressed. @itemx --compress-debug-sections=zlib-gnu @itemx --compress-debug-sections=zlib-gabi @itemx --compress-debug-sections=zstd +@itemx --compress-debug-sections=force These options control how DWARF debug sections are compressed. @option{--compress-debug-sections=none} is equivalent to @option{--nocompress-debug-sections}. @@ -738,7 +740,11 @@ using the obsoleted zlib-gnu format. The debug sections are renamed to begin with @samp{.zdebug}. @option{--compress-debug-sections=zstd} compresses DWARF debug sections using zstd. Note - if compression would actually make a section -@emph{larger}, then it is not compressed nor renamed. +@emph{larger}, then it is not compressed nor renamed, unless +@option{--compress-debug-section=force} is used. +@option{--compress-debug-sections=force} compresses DWARF debug sections, +even if this does not reduce size. It can be used in conjunction with a format +selection, for instance @option{--compress-debug-section=zstd+force}. @end ifset diff --git a/gas/write.c b/gas/write.c index 8273b7a42f1..39bcea23fac 100644 --- a/gas/write.c +++ b/gas/write.c @@ -1465,7 +1465,7 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) flagword flags = bfd_section_flags (sec); if (seginfo == NULL - || uncompressed_size < 32 + || (!force_compress_debug && uncompressed_size < 32) || (flags & SEC_HAS_CONTENTS) == 0) return; @@ -1582,7 +1582,7 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) /* PR binutils/18087: If compression didn't make the section smaller, just keep it uncompressed. */ - if (compressed_size >= uncompressed_size) + if (!force_compress_debug && compressed_size >= uncompressed_size) return; /* Replace the uncompressed frag list with the compressed frag list. */ -- 2.35.3 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-24 12:21 ` Tom de Vries @ 2023-02-24 13:23 ` Jan Beulich 2023-02-24 14:11 ` Tom de Vries 0 siblings, 1 reply; 24+ messages in thread From: Jan Beulich @ 2023-02-24 13:23 UTC (permalink / raw) To: Tom de Vries; +Cc: binutils, Michael Matz, Nick Clifton, Alan Modra On 24.02.2023 13:21, Tom de Vries wrote: > On 2/24/23 12:28, Jan Beulich wrote: >> On 24.02.2023 11:52, Tom de Vries wrote: >>> On 2/23/23 14:44, Jan Beulich wrote: >>>> I think both should be allowed. In a complex build system it may be >>>> different entities setting "how" and "whether". (To me "none" falls in >>>> the "whether" category together with "force", and it also can be seen >>>> as falling in the "how" category together with "zlib" etc. In Linux >>>> Kconfig, for example, I'd see this being expressed as first a "whether" >>>> choice [yes/maybe/forced] and then a "how" choice dependent upon >>>> "whether != none".) >>>> >>> >>> I gave this approach a try. >> >> Any specific reason you chose + as the separator instead of the more >> conventional , ? > > Yes, I initially went for ',', but ran into: > ... > $ gcc ~/hello.c -Wa,-gdwarf-5 \ > -Wa,--compress-debug-sections=zstd,force -c -v > ... > as -v --64 -gdwarf-5 --compress-debug-sections=zstd force -o hello.o \ > /tmp/ccOUMqHL.s > ... > Assembler messages: > Error: can't open force for reading: No such file or directory > ... Hmm. I have to admit that I'm not happy with +, irrespective of this issue. I wonder what other maintainers think - Nick, Alan? >> I also wouldn't see anything wrong with something >> like "...=force,zstd,none" - the last one(s) win. That's no different >> from specifying a second instance of the option. And without that it >> looks as if the parsing would end up simpler. > > OK, gave that a try. That's still accumulating none and force across the entire sequence (and then giving none priority over force, no matter that force may have been specified last), rather than handling things the same as when multiple options are specified. With accumulation partially removed parsing became less involved, but it can be yet more simple when that accumulation is dropped. In case of contention maybe best to not allow a sequence and hence require (in certain cases) two instances of the option to be passed? At the very least that's then easier to parse. Jan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-24 13:23 ` Jan Beulich @ 2023-02-24 14:11 ` Tom de Vries 2023-02-24 14:26 ` Jan Beulich 0 siblings, 1 reply; 24+ messages in thread From: Tom de Vries @ 2023-02-24 14:11 UTC (permalink / raw) To: Jan Beulich; +Cc: binutils, Michael Matz, Nick Clifton, Alan Modra On 2/24/23 14:23, Jan Beulich wrote: > On 24.02.2023 13:21, Tom de Vries wrote: >> On 2/24/23 12:28, Jan Beulich wrote: >>> On 24.02.2023 11:52, Tom de Vries wrote: >>>> On 2/23/23 14:44, Jan Beulich wrote: >>>>> I think both should be allowed. In a complex build system it may be >>>>> different entities setting "how" and "whether". (To me "none" falls in >>>>> the "whether" category together with "force", and it also can be seen >>>>> as falling in the "how" category together with "zlib" etc. In Linux >>>>> Kconfig, for example, I'd see this being expressed as first a "whether" >>>>> choice [yes/maybe/forced] and then a "how" choice dependent upon >>>>> "whether != none".) >>>>> >>>> >>>> I gave this approach a try. >>> >>> Any specific reason you chose + as the separator instead of the more >>> conventional , ? >> >> Yes, I initially went for ',', but ran into: >> ... >> $ gcc ~/hello.c -Wa,-gdwarf-5 \ >> -Wa,--compress-debug-sections=zstd,force -c -v >> ... >> as -v --64 -gdwarf-5 --compress-debug-sections=zstd force -o hello.o \ >> /tmp/ccOUMqHL.s >> ... >> Assembler messages: >> Error: can't open force for reading: No such file or directory >> ... > > Hmm. I have to admit that I'm not happy with +, irrespective of this > issue. I wonder what other maintainers think - Nick, Alan? > AFAIU you're proposing to use "-Xassembler --compress-debug-sections=zstd,force" in this case instead of -Wa. >>> I also wouldn't see anything wrong with something >>> like "...=force,zstd,none" - the last one(s) win. That's no different >>> from specifying a second instance of the option. And without that it >>> looks as if the parsing would end up simpler. >> >> OK, gave that a try. > > That's still accumulating none and force across the entire sequence > (and then giving none priority over force, no matter that force may > have been specified last), Um, so you're saying that none+zstd+force is currently interpreted as none? Lets try: ... $ gcc ~/hello.c -c -Wa,-gdwarf-5 -Xassembler --compress-debug-sections=none+zstd+force $ readelf -S -W hello.o | grep " .debug" [ 9] .debug_line PROGBITS 0000a8 000064 00 C 0 0 8 [11] .debug_line_str PROGBITS 000110 000046 01 MSC 0 0 8 [12] .debug_info PROGBITS 000158 000046 00 C 0 0 8 [14] .debug_abbrev PROGBITS 0001a0 000049 00 C 0 0 8 [15] .debug_aranges PROGBITS 0001f0 000034 00 C 0 0 8 [17] .debug_str PROGBITS 000228 00005a 01 MSC 0 0 8 ... So, that doesn't seem to be the case, compression is done, as expected. Thanks, - Tom > rather than handling things the same as > when multiple options are specified. With accumulation partially > removed parsing became less involved, but it can be yet more simple > when that accumulation is dropped. > > In case of contention maybe best to not allow a sequence and hence > require (in certain cases) two instances of the option to be passed? > At the very least that's then easier to parse. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-24 14:11 ` Tom de Vries @ 2023-02-24 14:26 ` Jan Beulich 2023-02-24 14:57 ` Tom de Vries 0 siblings, 1 reply; 24+ messages in thread From: Jan Beulich @ 2023-02-24 14:26 UTC (permalink / raw) To: Tom de Vries; +Cc: binutils, Michael Matz, Nick Clifton, Alan Modra On 24.02.2023 15:11, Tom de Vries wrote: > On 2/24/23 14:23, Jan Beulich wrote: >> On 24.02.2023 13:21, Tom de Vries wrote: >>> On 2/24/23 12:28, Jan Beulich wrote: >>>> I also wouldn't see anything wrong with something >>>> like "...=force,zstd,none" - the last one(s) win. That's no different >>>> from specifying a second instance of the option. And without that it >>>> looks as if the parsing would end up simpler. >>> >>> OK, gave that a try. >> >> That's still accumulating none and force across the entire sequence >> (and then giving none priority over force, no matter that force may >> have been specified last), > > Um, so you're saying that none+zstd+force is currently interpreted as none? > > Lets try: > ... > $ gcc ~/hello.c -c -Wa,-gdwarf-5 -Xassembler > --compress-debug-sections=none+zstd+force > $ readelf -S -W hello.o | grep " .debug" > [ 9] .debug_line PROGBITS 0000a8 000064 00 C 0 0 8 > [11] .debug_line_str PROGBITS 000110 000046 01 MSC 0 0 8 > [12] .debug_info PROGBITS 000158 000046 00 C 0 0 8 > [14] .debug_abbrev PROGBITS 0001a0 000049 00 C 0 0 8 > [15] .debug_aranges PROGBITS 0001f0 000034 00 C 0 0 8 > [17] .debug_str PROGBITS 000228 00005a 01 MSC 0 0 8 > > ... > > So, that doesn't seem to be the case, compression is done, as expected. Oh, I've overlooked that you explicitly clear *none when you set *force (my attention was mainly on the bottom of parse_compress_debug_optarg()). I think that's more involved than necessary (possibly merely a result of you having worked incrementally from your earlier version), and less obviously doing the same as would happen when multiple separate options were parsed. Jan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-24 14:26 ` Jan Beulich @ 2023-02-24 14:57 ` Tom de Vries 2023-02-27 9:03 ` Jan Beulich 0 siblings, 1 reply; 24+ messages in thread From: Tom de Vries @ 2023-02-24 14:57 UTC (permalink / raw) To: Jan Beulich; +Cc: binutils, Michael Matz, Nick Clifton, Alan Modra [-- Attachment #1: Type: text/plain, Size: 1934 bytes --] On 2/24/23 15:26, Jan Beulich wrote: > On 24.02.2023 15:11, Tom de Vries wrote: >> On 2/24/23 14:23, Jan Beulich wrote: >>> On 24.02.2023 13:21, Tom de Vries wrote: >>>> On 2/24/23 12:28, Jan Beulich wrote: >>>>> I also wouldn't see anything wrong with something >>>>> like "...=force,zstd,none" - the last one(s) win. That's no different >>>>> from specifying a second instance of the option. And without that it >>>>> looks as if the parsing would end up simpler. >>>> >>>> OK, gave that a try. >>> >>> That's still accumulating none and force across the entire sequence >>> (and then giving none priority over force, no matter that force may >>> have been specified last), >> >> Um, so you're saying that none+zstd+force is currently interpreted as none? >> >> Lets try: >> ... >> $ gcc ~/hello.c -c -Wa,-gdwarf-5 -Xassembler >> --compress-debug-sections=none+zstd+force >> $ readelf -S -W hello.o | grep " .debug" >> [ 9] .debug_line PROGBITS 0000a8 000064 00 C 0 0 8 >> [11] .debug_line_str PROGBITS 000110 000046 01 MSC 0 0 8 >> [12] .debug_info PROGBITS 000158 000046 00 C 0 0 8 >> [14] .debug_abbrev PROGBITS 0001a0 000049 00 C 0 0 8 >> [15] .debug_aranges PROGBITS 0001f0 000034 00 C 0 0 8 >> [17] .debug_str PROGBITS 000228 00005a 01 MSC 0 0 8 >> >> ... >> >> So, that doesn't seem to be the case, compression is done, as expected. > > Oh, I've overlooked that you explicitly clear *none when you set *force > (my attention was mainly on the bottom of parse_compress_debug_optarg()). > I think that's more involved than necessary (possibly merely a result of > you having worked incrementally from your earlier version), and less > obviously doing the same as would happen when multiple separate options > were parsed. I've tried to simplify further. Is this more how you want it? Thanks, - Tom [-- Attachment #2: 0001-gas-Add-compress-debug-sections-force.patch --] [-- Type: text/x-patch, Size: 9694 bytes --] From ae0cdfec9c854d89cc87f88c496a3b6e60539115 Mon Sep 17 00:00:00 2001 From: Tom de Vries <tdevries@suse.de> Date: Thu, 23 Feb 2023 12:53:40 +0100 Subject: [PATCH] gas: Add --compress-debug-sections=force Gas has an option --compress-debug-sections that allows it to generate compressed debug sections. That does not guarantee that the debug sections are in fact compressed: ... $ gcc hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd $ readelf -S -W hello.o | grep " .debug" [ 9] .debug_line PROGBITS 0000a8 000053 00 0 0 1 [11] .debug_line_str PROGBITS 0000fb 000025 01 MS 0 0 1 [12] .debug_info PROGBITS 000120 000039 00 0 0 1 [14] .debug_abbrev PROGBITS 000159 000028 00 0 0 1 [15] .debug_aranges PROGBITS 000190 000030 00 0 0 16 [17] .debug_str PROGBITS 0001c0 000039 01 MS 0 0 1 ... Sensibly so, they're only compressed if that provides a size benefit. However, for the purpose of testing components consuming dwarf we may want the sections to be compressed regardless. Add a new suboption --compress-debug-sections=force that ignores the size heuristic, such that we have instead: ... $ gcc hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd+force $ readelf -S -W hello.o | grep " .debug" [ 9] .debug_line PROGBITS 0000a8 000064 00 C 0 0 8 [11] .debug_line_str PROGBITS 000110 000046 01 MSC 0 0 8 [12] .debug_info PROGBITS 000158 000046 00 C 0 0 8 [14] .debug_abbrev PROGBITS 0001a0 000049 00 C 0 0 8 [15] .debug_aranges PROGBITS 0001f0 000034 00 C 0 0 8 [17] .debug_str PROGBITS 000228 00005a 01 MSC 0 0 8 ... Advertised as: ... $ as --help ... --compress-debug-sections[={none|<format>|force|force+<format>}] where <format> is {zlib|zlib-gnu|zlib-gabi|zstd} compress DWARF debug sections Default: zstd ... Tested on x86_64-linux. --- gas/as.c | 118 ++++++++++++++++++++++++++++++++++++++++-------- gas/as.h | 4 ++ gas/doc/as.texi | 10 +++- gas/write.c | 4 +- 4 files changed, 113 insertions(+), 23 deletions(-) diff --git a/gas/as.c b/gas/as.c index 598bfd56cf5..3b2d286befd 100644 --- a/gas/as.c +++ b/gas/as.c @@ -230,6 +230,18 @@ enum compressed_debug_section_type flag_compress_debug = DEFAULT_COMPRESSED_DEBUG_ALGORITHM; #endif +enum compress_debug_action +{ + cda_default, + cda_none, + cda_force, + cda_yes, +}; +static enum compress_debug_action compress_debug_action + = cda_default; + +bool force_compress_debug = false; + static void show_usage (FILE * stream) { @@ -252,7 +264,8 @@ Options:\n\ fprintf (stream, _("\ --alternate initially turn on alternate macro syntax\n")); fprintf (stream, _("\ - --compress-debug-sections[={none|zlib|zlib-gnu|zlib-gabi|zstd}]\n\ + --compress-debug-sections[={none|<format>|force|force+<format>}]\n\ + where <format> is {zlib|zlib-gnu|zlib-gabi|zstd}\n\ compress DWARF debug sections\n")), fprintf (stream, _("\ Default: %s\n"), @@ -418,6 +431,86 @@ Options:\n\ fprintf (stream, _("Report bugs to %s\n"), REPORT_BUGS_TO); } +static void +parse_compress_debug_optarg_1 (const char *optarg) +{ + gas_assert (optarg != NULL); + + if (strcmp (optarg, "force") == 0) + { + compress_debug_action = cda_force; + return; + } + + enum compressed_debug_section_type tmp + = bfd_get_compression_algorithm (optarg); + +#ifndef HAVE_ZSTD + if (tmp == COMPRESS_DEBUG_ZSTD) + as_fatal (_ ("--compress-debug-sections=zstd: gas is not " + "built with zstd support")); +#endif + + if (tmp == COMPRESS_UNKNOWN) + as_fatal (_("Invalid --compress-debug-sections option: `%s'"), + optarg); + + if (tmp == COMPRESS_DEBUG_NONE) + { + compress_debug_action = cda_none; + return; + } + + compress_debug_action = cda_yes; + flag_compress_debug = tmp; +} + +static void +parse_compress_debug_optarg (const char *optarg) +{ +#if !defined OBJ_ELF && !defined OBJ_MAYBE_ELF + as_fatal (_("--compress-debug-sections=%s is unsupported"), + optarg); +#endif + + /* Tokenize subopts seperated by '+' and pass to + parse_compress_debug_optarg_1. */ + while (true) + { + const char *idx = optarg; + while (*idx != '\0' && *idx != '+') + idx++; + + size_t len = idx - optarg; + if (len == 0) + { + /* Generate error. */ + parse_compress_debug_optarg_1 (""); + break; + } + + char *tmp = xstrndup (optarg, len); + parse_compress_debug_optarg_1 (tmp); + free (tmp); + + if (*idx == '\0') + break; + + /* Step over '+' and continue tokenizing. */ + gas_assert (*idx == '+'); + optarg = idx + 1; + } +} + +static void +finalize_parse_compress_debug_optarg (void) +{ + if (compress_debug_action == cda_none) + flag_compress_debug = COMPRESS_DEBUG_NONE; + else if (compress_debug_action == cda_force) + force_compress_debug = true; +} + /* Since it is easy to do here we interpret the special arg "-" to mean "use stdin" and we set that argv[] pointing to "". After we have munged argv[], the only things left are source file @@ -747,28 +840,13 @@ This program has absolutely no warranty.\n")); case OPTION_COMPRESS_DEBUG: if (optarg) - { -#if defined OBJ_ELF || defined OBJ_MAYBE_ELF - flag_compress_debug = bfd_get_compression_algorithm (optarg); -#ifndef HAVE_ZSTD - if (flag_compress_debug == COMPRESS_DEBUG_ZSTD) - as_fatal (_ ("--compress-debug-sections=zstd: gas is not " - "built with zstd support")); -#endif - if (flag_compress_debug == COMPRESS_UNKNOWN) - as_fatal (_("Invalid --compress-debug-sections option: `%s'"), - optarg); -#else - as_fatal (_("--compress-debug-sections=%s is unsupported"), - optarg); -#endif - } + parse_compress_debug_optarg (optarg); else - flag_compress_debug = COMPRESS_DEBUG_GABI_ZLIB; + parse_compress_debug_optarg ("zlib-gabi"); break; case OPTION_NOCOMPRESS_DEBUG: - flag_compress_debug = COMPRESS_DEBUG_NONE; + parse_compress_debug_optarg ("none"); break; case OPTION_DEBUG_PREFIX_MAP: @@ -1136,6 +1214,8 @@ This program has absolutely no warranty.\n")); *pargc = new_argc; *pargv = new_argv; + finalize_parse_compress_debug_optarg (); + #ifdef md_after_parse_args md_after_parse_args (); #endif diff --git a/gas/as.h b/gas/as.h index 4c5fa9ecf7d..115af019815 100644 --- a/gas/as.h +++ b/gas/as.h @@ -331,6 +331,10 @@ COMMON int flag_traditional_format; /* Type of compressed debug sections we should generate. */ COMMON enum compressed_debug_section_type flag_compress_debug; +/* True if we want to generate compressed debug sections, even if it + doesn't make them smaller. */ +COMMON bool force_compress_debug; + /* TRUE if .note.GNU-stack section with SEC_CODE should be created */ COMMON int flag_execstack; diff --git a/gas/doc/as.texi b/gas/doc/as.texi index bbdfa4bfdca..d25559141c7 100644 --- a/gas/doc/as.texi +++ b/gas/doc/as.texi @@ -718,7 +718,8 @@ Begin in alternate macro mode. Compress DWARF debug sections using zlib with SHF_COMPRESSED from the ELF ABI. The resulting object file may not be compatible with older linkers and object file utilities. Note if compression would make a -given section @emph{larger} then it is not compressed. +given section @emph{larger} then it is not compressed, unless +@option{--compress-debug-section=force} is used. @ifset ELF @cindex @samp{--compress-debug-sections=} option @@ -727,6 +728,7 @@ given section @emph{larger} then it is not compressed. @itemx --compress-debug-sections=zlib-gnu @itemx --compress-debug-sections=zlib-gabi @itemx --compress-debug-sections=zstd +@itemx --compress-debug-sections=force These options control how DWARF debug sections are compressed. @option{--compress-debug-sections=none} is equivalent to @option{--nocompress-debug-sections}. @@ -738,7 +740,11 @@ using the obsoleted zlib-gnu format. The debug sections are renamed to begin with @samp{.zdebug}. @option{--compress-debug-sections=zstd} compresses DWARF debug sections using zstd. Note - if compression would actually make a section -@emph{larger}, then it is not compressed nor renamed. +@emph{larger}, then it is not compressed nor renamed, unless +@option{--compress-debug-section=force} is used. +@option{--compress-debug-sections=force} compresses DWARF debug sections, +even if this does not reduce size. It can be used in conjunction with a format +selection, for instance @option{--compress-debug-section=zstd+force}. @end ifset diff --git a/gas/write.c b/gas/write.c index 8273b7a42f1..39bcea23fac 100644 --- a/gas/write.c +++ b/gas/write.c @@ -1465,7 +1465,7 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) flagword flags = bfd_section_flags (sec); if (seginfo == NULL - || uncompressed_size < 32 + || (!force_compress_debug && uncompressed_size < 32) || (flags & SEC_HAS_CONTENTS) == 0) return; @@ -1582,7 +1582,7 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) /* PR binutils/18087: If compression didn't make the section smaller, just keep it uncompressed. */ - if (compressed_size >= uncompressed_size) + if (!force_compress_debug && compressed_size >= uncompressed_size) return; /* Replace the uncompressed frag list with the compressed frag list. */ -- 2.35.3 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-24 14:57 ` Tom de Vries @ 2023-02-27 9:03 ` Jan Beulich 2023-02-27 11:43 ` [PATCH] gas: Add --compress-debug-sections=force Tom de Vries 2023-02-27 13:44 ` [PATCH] gas: Add --force-compress-debug-sections Pedro Alves 0 siblings, 2 replies; 24+ messages in thread From: Jan Beulich @ 2023-02-27 9:03 UTC (permalink / raw) To: Tom de Vries; +Cc: binutils, Michael Matz, Nick Clifton, Alan Modra On 24.02.2023 15:57, Tom de Vries wrote: > On 2/24/23 15:26, Jan Beulich wrote: >> On 24.02.2023 15:11, Tom de Vries wrote: >>> On 2/24/23 14:23, Jan Beulich wrote: >>>> On 24.02.2023 13:21, Tom de Vries wrote: >>>>> On 2/24/23 12:28, Jan Beulich wrote: >>>>>> I also wouldn't see anything wrong with something >>>>>> like "...=force,zstd,none" - the last one(s) win. That's no different >>>>>> from specifying a second instance of the option. And without that it >>>>>> looks as if the parsing would end up simpler. >>>>> >>>>> OK, gave that a try. >>>> >>>> That's still accumulating none and force across the entire sequence >>>> (and then giving none priority over force, no matter that force may >>>> have been specified last), >>> >>> Um, so you're saying that none+zstd+force is currently interpreted as none? >>> >>> Lets try: >>> ... >>> $ gcc ~/hello.c -c -Wa,-gdwarf-5 -Xassembler >>> --compress-debug-sections=none+zstd+force >>> $ readelf -S -W hello.o | grep " .debug" >>> [ 9] .debug_line PROGBITS 0000a8 000064 00 C 0 0 8 >>> [11] .debug_line_str PROGBITS 000110 000046 01 MSC 0 0 8 >>> [12] .debug_info PROGBITS 000158 000046 00 C 0 0 8 >>> [14] .debug_abbrev PROGBITS 0001a0 000049 00 C 0 0 8 >>> [15] .debug_aranges PROGBITS 0001f0 000034 00 C 0 0 8 >>> [17] .debug_str PROGBITS 000228 00005a 01 MSC 0 0 8 >>> >>> ... >>> >>> So, that doesn't seem to be the case, compression is done, as expected. >> >> Oh, I've overlooked that you explicitly clear *none when you set *force >> (my attention was mainly on the bottom of parse_compress_debug_optarg()). >> I think that's more involved than necessary (possibly merely a result of >> you having worked incrementally from your earlier version), and less >> obviously doing the same as would happen when multiple separate options >> were parsed. > > I've tried to simplify further. > > Is this more how you want it? I have to admit that I'm still puzzled by the presence of finalize_parse_compress_debug_optarg() as well as you needing both a new static variable and a new global one. But I guess whether that's really needed first of all depends on the semantics we want e.g. --nocompress-debug-sections --compress-debug-sections=force to have (which, with how you have it presently, could also be expressed as --compress-debug-sections=none+force or --compress-debug-sections=none --compress-debug-sections=force afaict). I view the present meaning as one sensible one, but I could also see "none" (or equivalent) simply zapping the compression type (and hence rendering "force" meaningless) as another sensible one. A change in meaning may then also result in the three option combinations above possibly not all doing the same. As an aside: As you update the patch, please try to keep the title in line with what the patch actually does. Also, ftaod, I don't mean to stand in the way of another maintainer approving any of the forms proposed so far. This specifically also includes the use of '+' as a separator, which I personally don't (currently) intend to approve. Jan ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] gas: Add --compress-debug-sections=force 2023-02-27 9:03 ` Jan Beulich @ 2023-02-27 11:43 ` Tom de Vries 2023-02-27 11:51 ` Jan Beulich 2023-02-27 13:44 ` [PATCH] gas: Add --force-compress-debug-sections Pedro Alves 1 sibling, 1 reply; 24+ messages in thread From: Tom de Vries @ 2023-02-27 11:43 UTC (permalink / raw) To: Jan Beulich; +Cc: binutils, Michael Matz, Nick Clifton, Alan Modra [-- Attachment #1: Type: text/plain, Size: 4169 bytes --] [ was: Re: [PATCH] gas: Add --force-compress-debug-sections ] On 2/27/23 10:03, Jan Beulich wrote: > On 24.02.2023 15:57, Tom de Vries wrote: >> On 2/24/23 15:26, Jan Beulich wrote: >>> On 24.02.2023 15:11, Tom de Vries wrote: >>>> On 2/24/23 14:23, Jan Beulich wrote: >>>>> On 24.02.2023 13:21, Tom de Vries wrote: >>>>>> On 2/24/23 12:28, Jan Beulich wrote: >>>>>>> I also wouldn't see anything wrong with something >>>>>>> like "...=force,zstd,none" - the last one(s) win. That's no different >>>>>>> from specifying a second instance of the option. And without that it >>>>>>> looks as if the parsing would end up simpler. >>>>>> >>>>>> OK, gave that a try. >>>>> >>>>> That's still accumulating none and force across the entire sequence >>>>> (and then giving none priority over force, no matter that force may >>>>> have been specified last), >>>> >>>> Um, so you're saying that none+zstd+force is currently interpreted as none? >>>> >>>> Lets try: >>>> ... >>>> $ gcc ~/hello.c -c -Wa,-gdwarf-5 -Xassembler >>>> --compress-debug-sections=none+zstd+force >>>> $ readelf -S -W hello.o | grep " .debug" >>>> [ 9] .debug_line PROGBITS 0000a8 000064 00 C 0 0 8 >>>> [11] .debug_line_str PROGBITS 000110 000046 01 MSC 0 0 8 >>>> [12] .debug_info PROGBITS 000158 000046 00 C 0 0 8 >>>> [14] .debug_abbrev PROGBITS 0001a0 000049 00 C 0 0 8 >>>> [15] .debug_aranges PROGBITS 0001f0 000034 00 C 0 0 8 >>>> [17] .debug_str PROGBITS 000228 00005a 01 MSC 0 0 8 >>>> >>>> ... >>>> >>>> So, that doesn't seem to be the case, compression is done, as expected. >>> >>> Oh, I've overlooked that you explicitly clear *none when you set *force >>> (my attention was mainly on the bottom of parse_compress_debug_optarg()). >>> I think that's more involved than necessary (possibly merely a result of >>> you having worked incrementally from your earlier version), and less >>> obviously doing the same as would happen when multiple separate options >>> were parsed. >> >> I've tried to simplify further. >> >> Is this more how you want it? > > I have to admit that I'm still puzzled by the presence of > finalize_parse_compress_debug_optarg() as well as you needing both a new > static variable and a new global one. The latter I've fixed, by exporting the static variable. That eliminates one half of finalize_parse_compress_debug_optarg(). The remaining half is necessary because we accumulate "none" into compress_debug_action instead of into flag_compress_debug, so after parsing is over we need to assign the accumulation result, if it is indeed cda_none, to flag_compress_debug. And the reason we have that complicated setup is related to preventing none from zapping compression type. > But I guess whether that's really > needed first of all depends on the semantics we want e.g. > > --nocompress-debug-sections --compress-debug-sections=force > > to have (which, with how you have it presently, could also be expressed > as > > --compress-debug-sections=none+force > > or > > --compress-debug-sections=none --compress-debug-sections=force > > afaict). I view the present meaning as one sensible one, but I could > also see "none" (or equivalent) simply zapping the compression type > (and hence rendering "force" meaningless) as another sensible one. Sure, that's another possible semantics. Should I implement that instead? > A > change in meaning may then also result in the three option combinations > above possibly not all doing the same. > > As an aside: As you update the patch, please try to keep the title in > line with what the patch actually does. > I'm assuming you mean the email title (the patch title looks ok to me), and I've updated it. > Also, ftaod, I don't mean to stand in the way of another maintainer > approving any of the forms proposed so far. This specifically also > includes the use of '+' as a separator, which I personally don't > (currently) intend to approve. Well, in case that's blocking you from giving approval, I've updated the patch to use the ',' separator. Thanks, - Tom [-- Attachment #2: 0001-gas-Add-compress-debug-sections-force.patch --] [-- Type: text/x-patch, Size: 9714 bytes --] From 0fb4ed6d648a69799cdbf6b19fd36c6125c5fc41 Mon Sep 17 00:00:00 2001 From: Tom de Vries <tdevries@suse.de> Date: Thu, 23 Feb 2023 12:53:40 +0100 Subject: [PATCH] gas: Add --compress-debug-sections=force Gas has an option --compress-debug-sections that allows it to generate compressed debug sections. That does not guarantee that the debug sections are in fact compressed: ... $ gcc hello.c -Wa,-gdwarf-5 -c -Wa,--compress-debug-sections=zstd $ readelf -S -W hello.o | grep " .debug" [ 9] .debug_line PROGBITS 0000a8 000053 00 0 0 1 [11] .debug_line_str PROGBITS 0000fb 000025 01 MS 0 0 1 [12] .debug_info PROGBITS 000120 000039 00 0 0 1 [14] .debug_abbrev PROGBITS 000159 000028 00 0 0 1 [15] .debug_aranges PROGBITS 000190 000030 00 0 0 16 [17] .debug_str PROGBITS 0001c0 000039 01 MS 0 0 1 ... Sensibly so, they're only compressed if that provides a size benefit. However, for the purpose of testing components consuming dwarf we may want the sections to be compressed regardless. Add a new suboption --compress-debug-sections=force that ignores the size heuristic, such that we have instead: ... $ gcc hello.c -Wa,-gdwarf-5 -c -Xassembler \ --compress-debug-sections=zstd,force $ readelf -S -W hello.o | grep " .debug" [ 9] .debug_line PROGBITS 0000a8 000064 00 C 0 0 8 [11] .debug_line_str PROGBITS 000110 000046 01 MSC 0 0 8 [12] .debug_info PROGBITS 000158 000046 00 C 0 0 8 [14] .debug_abbrev PROGBITS 0001a0 000049 00 C 0 0 8 [15] .debug_aranges PROGBITS 0001f0 000034 00 C 0 0 8 [17] .debug_str PROGBITS 000228 00005a 01 MSC 0 0 8 ... Advertised as: ... $ as --help ... --compress-debug-sections[={none|<format>|force|force,<format>}] where <format> is {zlib|zlib-gnu|zlib-gabi|zstd} compress DWARF debug sections Default: zstd ... Tested on x86_64-linux. --- gas/as.c | 106 +++++++++++++++++++++++++++++++++++++++--------- gas/as.h | 10 +++++ gas/doc/as.texi | 10 ++++- gas/write.c | 5 ++- 4 files changed, 108 insertions(+), 23 deletions(-) diff --git a/gas/as.c b/gas/as.c index 598bfd56cf5..acbccfb5dce 100644 --- a/gas/as.c +++ b/gas/as.c @@ -230,6 +230,8 @@ enum compressed_debug_section_type flag_compress_debug = DEFAULT_COMPRESSED_DEBUG_ALGORITHM; #endif +enum compress_debug_action compress_debug_action = cda_default; + static void show_usage (FILE * stream) { @@ -252,7 +254,8 @@ Options:\n\ fprintf (stream, _("\ --alternate initially turn on alternate macro syntax\n")); fprintf (stream, _("\ - --compress-debug-sections[={none|zlib|zlib-gnu|zlib-gabi|zstd}]\n\ + --compress-debug-sections[={none|<format>|force|force,<format>}]\n\ + where <format> is {zlib|zlib-gnu|zlib-gabi|zstd}\n\ compress DWARF debug sections\n")), fprintf (stream, _("\ Default: %s\n"), @@ -418,6 +421,84 @@ Options:\n\ fprintf (stream, _("Report bugs to %s\n"), REPORT_BUGS_TO); } +static void +parse_compress_debug_optarg_1 (const char *optarg) +{ + gas_assert (optarg != NULL); + + if (strcmp (optarg, "force") == 0) + { + compress_debug_action = cda_force; + return; + } + + enum compressed_debug_section_type tmp + = bfd_get_compression_algorithm (optarg); + +#ifndef HAVE_ZSTD + if (tmp == COMPRESS_DEBUG_ZSTD) + as_fatal (_ ("--compress-debug-sections=zstd: gas is not " + "built with zstd support")); +#endif + + if (tmp == COMPRESS_UNKNOWN) + as_fatal (_("Invalid --compress-debug-sections option: `%s'"), + optarg); + + if (tmp == COMPRESS_DEBUG_NONE) + { + compress_debug_action = cda_none; + return; + } + + compress_debug_action = cda_yes; + flag_compress_debug = tmp; +} + +static void +parse_compress_debug_optarg (const char *optarg) +{ +#if !defined OBJ_ELF && !defined OBJ_MAYBE_ELF + as_fatal (_("--compress-debug-sections=%s is unsupported"), + optarg); +#endif + + /* Tokenize subopts pass to parse_compress_debug_optarg_1. */ + char sep = ','; + while (true) + { + const char *idx = optarg; + while (*idx != '\0' && *idx != sep) + idx++; + + size_t len = idx - optarg; + if (len == 0) + { + /* Generate error. */ + parse_compress_debug_optarg_1 (""); + break; + } + + char *tmp = xstrndup (optarg, len); + parse_compress_debug_optarg_1 (tmp); + free (tmp); + + if (*idx == '\0') + break; + + /* Step over separator and continue tokenizing. */ + gas_assert (*idx == sep); + optarg = idx + 1; + } +} + +static void +finalize_parse_compress_debug_optarg (void) +{ + if (compress_debug_action == cda_none) + flag_compress_debug = COMPRESS_DEBUG_NONE; +} + /* Since it is easy to do here we interpret the special arg "-" to mean "use stdin" and we set that argv[] pointing to "". After we have munged argv[], the only things left are source file @@ -747,28 +828,13 @@ This program has absolutely no warranty.\n")); case OPTION_COMPRESS_DEBUG: if (optarg) - { -#if defined OBJ_ELF || defined OBJ_MAYBE_ELF - flag_compress_debug = bfd_get_compression_algorithm (optarg); -#ifndef HAVE_ZSTD - if (flag_compress_debug == COMPRESS_DEBUG_ZSTD) - as_fatal (_ ("--compress-debug-sections=zstd: gas is not " - "built with zstd support")); -#endif - if (flag_compress_debug == COMPRESS_UNKNOWN) - as_fatal (_("Invalid --compress-debug-sections option: `%s'"), - optarg); -#else - as_fatal (_("--compress-debug-sections=%s is unsupported"), - optarg); -#endif - } + parse_compress_debug_optarg (optarg); else - flag_compress_debug = COMPRESS_DEBUG_GABI_ZLIB; + parse_compress_debug_optarg ("zlib-gabi"); break; case OPTION_NOCOMPRESS_DEBUG: - flag_compress_debug = COMPRESS_DEBUG_NONE; + parse_compress_debug_optarg ("none"); break; case OPTION_DEBUG_PREFIX_MAP: @@ -1136,6 +1202,8 @@ This program has absolutely no warranty.\n")); *pargc = new_argc; *pargv = new_argv; + finalize_parse_compress_debug_optarg (); + #ifdef md_after_parse_args md_after_parse_args (); #endif diff --git a/gas/as.h b/gas/as.h index 4c5fa9ecf7d..bc0fe7f5cd9 100644 --- a/gas/as.h +++ b/gas/as.h @@ -331,6 +331,16 @@ COMMON int flag_traditional_format; /* Type of compressed debug sections we should generate. */ COMMON enum compressed_debug_section_type flag_compress_debug; +/* Whether we should compress debug sections. */ +enum compress_debug_action +{ + cda_default, + cda_none, + cda_force, + cda_yes, +}; +COMMON enum compress_debug_action compress_debug_action; + /* TRUE if .note.GNU-stack section with SEC_CODE should be created */ COMMON int flag_execstack; diff --git a/gas/doc/as.texi b/gas/doc/as.texi index bbdfa4bfdca..f0140c09779 100644 --- a/gas/doc/as.texi +++ b/gas/doc/as.texi @@ -718,7 +718,8 @@ Begin in alternate macro mode. Compress DWARF debug sections using zlib with SHF_COMPRESSED from the ELF ABI. The resulting object file may not be compatible with older linkers and object file utilities. Note if compression would make a -given section @emph{larger} then it is not compressed. +given section @emph{larger} then it is not compressed, unless +@option{--compress-debug-section=force} is used. @ifset ELF @cindex @samp{--compress-debug-sections=} option @@ -727,6 +728,7 @@ given section @emph{larger} then it is not compressed. @itemx --compress-debug-sections=zlib-gnu @itemx --compress-debug-sections=zlib-gabi @itemx --compress-debug-sections=zstd +@itemx --compress-debug-sections=force These options control how DWARF debug sections are compressed. @option{--compress-debug-sections=none} is equivalent to @option{--nocompress-debug-sections}. @@ -738,7 +740,11 @@ using the obsoleted zlib-gnu format. The debug sections are renamed to begin with @samp{.zdebug}. @option{--compress-debug-sections=zstd} compresses DWARF debug sections using zstd. Note - if compression would actually make a section -@emph{larger}, then it is not compressed nor renamed. +@emph{larger}, then it is not compressed nor renamed, unless +@option{--compress-debug-section=force} is used. +@option{--compress-debug-sections=force} compresses DWARF debug sections, +even if this does not reduce size. It can be used in conjunction with a format +selection, for instance @option{--compress-debug-section=zstd,force}. @end ifset diff --git a/gas/write.c b/gas/write.c index 8273b7a42f1..1fa3b54a03b 100644 --- a/gas/write.c +++ b/gas/write.c @@ -1463,9 +1463,10 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) segment_info_type *seginfo = seg_info (sec); bfd_size_type uncompressed_size = sec->size; flagword flags = bfd_section_flags (sec); + bool force_compress_debug = compress_debug_action == cda_force; if (seginfo == NULL - || uncompressed_size < 32 + || (!force_compress_debug && uncompressed_size < 32) || (flags & SEC_HAS_CONTENTS) == 0) return; @@ -1582,7 +1583,7 @@ compress_debug (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED) /* PR binutils/18087: If compression didn't make the section smaller, just keep it uncompressed. */ - if (compressed_size >= uncompressed_size) + if (!force_compress_debug && compressed_size >= uncompressed_size) return; /* Replace the uncompressed frag list with the compressed frag list. */ -- 2.35.3 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --compress-debug-sections=force 2023-02-27 11:43 ` [PATCH] gas: Add --compress-debug-sections=force Tom de Vries @ 2023-02-27 11:51 ` Jan Beulich 0 siblings, 0 replies; 24+ messages in thread From: Jan Beulich @ 2023-02-27 11:51 UTC (permalink / raw) To: Tom de Vries; +Cc: binutils, Michael Matz, Nick Clifton, Alan Modra On 27.02.2023 12:43, Tom de Vries wrote: > On 2/27/23 10:03, Jan Beulich wrote: >> But I guess whether that's really >> needed first of all depends on the semantics we want e.g. >> >> --nocompress-debug-sections --compress-debug-sections=force >> >> to have (which, with how you have it presently, could also be expressed >> as >> >> --compress-debug-sections=none+force >> >> or >> >> --compress-debug-sections=none --compress-debug-sections=force >> >> afaict). I view the present meaning as one sensible one, but I could >> also see "none" (or equivalent) simply zapping the compression type >> (and hence rendering "force" meaningless) as another sensible one. > > Sure, that's another possible semantics. > > Should I implement that instead? Not sure - as said, depends on what exactly is wanted. I find both variants leaving too much room for ambiguity, so I can't really decide (for myself). >> A >> change in meaning may then also result in the three option combinations >> above possibly not all doing the same. >> > >> As an aside: As you update the patch, please try to keep the title in >> line with what the patch actually does. >> > > I'm assuming you mean the email title (the patch title looks ok to me), > and I've updated it. Hmm, yes, sorry - I'm normally implying new patch versions to be at the root of new threads (and with increased version number), not in reply to earlier versions. Hence I didn't pay attention to the Re: ... still being there from the initial patch. Jan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-27 9:03 ` Jan Beulich 2023-02-27 11:43 ` [PATCH] gas: Add --compress-debug-sections=force Tom de Vries @ 2023-02-27 13:44 ` Pedro Alves 2023-02-27 14:07 ` Jan Beulich 1 sibling, 1 reply; 24+ messages in thread From: Pedro Alves @ 2023-02-27 13:44 UTC (permalink / raw) To: Jan Beulich, Tom de Vries Cc: binutils, Michael Matz, Nick Clifton, Alan Modra On 2023-02-27 9:03 a.m., Jan Beulich via Binutils wrote: > On 24.02.2023 15:57, Tom de Vries wrote: >> >> Is this more how you want it? > > I have to admit that I'm still puzzled by the presence of > finalize_parse_compress_debug_optarg() as well as you needing both a new > static variable and a new global one. But I guess whether that's really > needed first of all depends on the semantics we want e.g. > > --nocompress-debug-sections --compress-debug-sections=force > > to have (which, with how you have it presently, could also be expressed > as > > --compress-debug-sections=none+force > > or > > --compress-debug-sections=none --compress-debug-sections=force > > afaict). I view the present meaning as one sensible one, but I could > also see "none" (or equivalent) simply zapping the compression type > (and hence rendering "force" meaningless) as another sensible one. A > change in meaning may then also result in the three option combinations > above possibly not all doing the same. ISTM that these confusions (which no doubt users will have too) would go away if we did not try to put orthogonal settings into one option. Witness how the implementation uses two different enums: This new one: +enum compress_debug_action +{ + cda_default, + cda_none, + cda_force, + cda_yes, +}; And this preexisting one: /* Types of compressed DWARF debug sections. */ enum compressed_debug_section_type { COMPRESS_DEBUG_NONE = 0, COMPRESS_DEBUG_GNU_ZLIB = 1 << 1, COMPRESS_DEBUG_GABI_ZLIB = 1 << 2, COMPRESS_DEBUG_ZSTD = 1 << 3, COMPRESS_UNKNOWN = 1 << 4 }; Imagine we started over from scratch, and had these two orthogonal options, matching internal enums (enum compress_debug_action would be slightly different): --compressed-debug-sections-format=zlib|zstd|...|none # Iff we're compressing, what format shall we use? --compress-debug-sections=no|yes|sizewin # Are we compressing debug sections? When? # # - "no" - never, we're not compressing. # # - "yes" - always, we're compressing, using the format specified # by --compressed-debug-sections-format (and if that is "none", well, # you still get what you asked for). # # - "sizewin" - compress if there's a size win. Like "yes", if the format is # "none", well, this becomes a nop. ... then the semantics of mixing these options, or what happens when you repeat them would be much more obvious: - You can specify '--compressed-debug-sections-format' multiple times. The last wins. - You can specify '--compress-debug-sections' multiple times. The last wins. - Changing '--compressed-debug-sections-format' does not affect the value of '--compress-debug-sections'. - Changing '--compress-debug-sections' does not affect the value of '--compressed-debug-sections-format' Now, we can't use those option names with that meaning, though, because '--compress-debug-sections' today already has the meaning of selecting the compression format. But that just means we would need to pick different names, like for example: --compress-debug-sections=zlib|zstd|none # Iff we're compressing, what format shall we use? --nocompress-debug-sections # Shorthand for --compress-debug-sections=none --compress-debug-sections-when=never|always|sizewin # Are we compressing debug sections? When? # # - "never" - never, we're not compressing. # # - "always" - always compress, using the format specified by # by --compress-debug-sections (and if that is "none", well, # you still get what you asked for). # # - "sizewin", compress if there's a size win. Like "always", respects # the format specified by --compress-debug-sections. The semantics of mixing these options, or what happens when you repeat them would be obvious in the same way in the other options naming earlier: - You can specify '--compress-debug-sections' multiple times. The last wins. - You can specify '--compress-debug-sections-when' multiple times. The last wins. - Changing '--compress-debug-sections' does not affect the value of '--compress-debug-sections-when'. - Changing '--compress-debug-sections-when' does not affect the value of '--compressed-debug-sections' You end up with two different ways to disable compressing debug sections, but that seems OK to me. All that would be left would be bikeshed on the new option name. Pedro Alves > > As an aside: As you update the patch, please try to keep the title in > line with what the patch actually does. > > Also, ftaod, I don't mean to stand in the way of another maintainer > approving any of the forms proposed so far. This specifically also > includes the use of '+' as a separator, which I personally don't > (currently) intend to approve. > > Jan > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-27 13:44 ` [PATCH] gas: Add --force-compress-debug-sections Pedro Alves @ 2023-02-27 14:07 ` Jan Beulich 2023-02-27 23:24 ` Tom de Vries 2023-02-28 12:49 ` Pedro Alves 0 siblings, 2 replies; 24+ messages in thread From: Jan Beulich @ 2023-02-27 14:07 UTC (permalink / raw) To: Pedro Alves Cc: binutils, Michael Matz, Nick Clifton, Alan Modra, Tom de Vries On 27.02.2023 14:44, Pedro Alves wrote: > On 2023-02-27 9:03 a.m., Jan Beulich via Binutils wrote: >> On 24.02.2023 15:57, Tom de Vries wrote: >>> >>> Is this more how you want it? >> >> I have to admit that I'm still puzzled by the presence of >> finalize_parse_compress_debug_optarg() as well as you needing both a new >> static variable and a new global one. But I guess whether that's really >> needed first of all depends on the semantics we want e.g. >> >> --nocompress-debug-sections --compress-debug-sections=force >> >> to have (which, with how you have it presently, could also be expressed >> as >> >> --compress-debug-sections=none+force >> >> or >> >> --compress-debug-sections=none --compress-debug-sections=force >> >> afaict). I view the present meaning as one sensible one, but I could >> also see "none" (or equivalent) simply zapping the compression type >> (and hence rendering "force" meaningless) as another sensible one. A >> change in meaning may then also result in the three option combinations >> above possibly not all doing the same. > > > ISTM that these confusions (which no doubt users will have too) would go away if > we did not try to put orthogonal settings into one option. > > Witness how the implementation uses two different enums: > > This new one: > > +enum compress_debug_action > +{ > + cda_default, > + cda_none, > + cda_force, > + cda_yes, > +}; > > And this preexisting one: > > /* Types of compressed DWARF debug sections. */ > enum compressed_debug_section_type > { > COMPRESS_DEBUG_NONE = 0, > COMPRESS_DEBUG_GNU_ZLIB = 1 << 1, > COMPRESS_DEBUG_GABI_ZLIB = 1 << 2, > COMPRESS_DEBUG_ZSTD = 1 << 3, > COMPRESS_UNKNOWN = 1 << 4 > }; > > Imagine we started over from scratch, and had these two orthogonal options, > matching internal enums (enum compress_debug_action would be slightly different): > > --compressed-debug-sections-format=zlib|zstd|...|none > > # Iff we're compressing, what format shall we use? > > --compress-debug-sections=no|yes|sizewin > > # Are we compressing debug sections? When? > # > # - "no" - never, we're not compressing. > # > # - "yes" - always, we're compressing, using the format specified > # by --compressed-debug-sections-format (and if that is "none", well, > # you still get what you asked for). > # > # - "sizewin" - compress if there's a size win. Like "yes", if the format is > # "none", well, this becomes a nop. > > ... then the semantics of mixing these options, or what happens when you repeat them > would be much more obvious: > > - You can specify '--compressed-debug-sections-format' multiple times. The last wins. > > - You can specify '--compress-debug-sections' multiple times. The last wins. > > - Changing '--compressed-debug-sections-format' does not affect the value of '--compress-debug-sections'. > > - Changing '--compress-debug-sections' does not affect the value of '--compressed-debug-sections-format' > > > Now, we can't use those option names with that meaning, though, because > '--compress-debug-sections' today already has the meaning of selecting > the compression format. But that just means we would need to pick different > names, like for example: > > --compress-debug-sections=zlib|zstd|none > > # Iff we're compressing, what format shall we use? > > --nocompress-debug-sections > > # Shorthand for --compress-debug-sections=none > > --compress-debug-sections-when=never|always|sizewin > > # Are we compressing debug sections? When? > # > # - "never" - never, we're not compressing. > # > # - "always" - always compress, using the format specified by > # by --compress-debug-sections (and if that is "none", well, > # you still get what you asked for). > # > # - "sizewin", compress if there's a size win. Like "always", respects > # the format specified by --compress-debug-sections. > > > The semantics of mixing these options, or what happens when you repeat them > would be obvious in the same way in the other options naming earlier: > > - You can specify '--compress-debug-sections' multiple times. The last wins. > > - You can specify '--compress-debug-sections-when' multiple times. The last wins. > > - Changing '--compress-debug-sections' does not affect the value of '--compress-debug-sections-when'. > > - Changing '--compress-debug-sections-when' does not affect the value of '--compressed-debug-sections' > > You end up with two different ways to disable compressing debug sections, > but that seems OK to me. That would be okay with me as well, but (in part to avoid ... > All that would be left would be bikeshed on the new option name. ... this aspect) I fail to see why a single option won't do: --compress-debug-sections=zlib|zstd|no|yes|sizewin Since the sub-options are distinct sets (alongside "no" one could add "none" if wanted, to separate "method: no compression" from "don't compress", which I think would then also eliminate the ambiguity referred to earlier on [still visible in context above]), within each set your "last wins" would apply equally. And in fact that was the main reason for my original response back to Tom: I view the proliferation of command line options (not just here, but in general) as a scalability issue. Therefore my rule of thumb is that things which belong together are best grouped under a single top-level option. Jan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-27 14:07 ` Jan Beulich @ 2023-02-27 23:24 ` Tom de Vries 2023-02-28 0:19 ` Tom de Vries 2023-02-28 13:21 ` Pedro Alves 2023-02-28 12:49 ` Pedro Alves 1 sibling, 2 replies; 24+ messages in thread From: Tom de Vries @ 2023-02-27 23:24 UTC (permalink / raw) To: Jan Beulich, Pedro Alves; +Cc: binutils, Michael Matz, Nick Clifton, Alan Modra On 2/27/23 15:07, Jan Beulich wrote: > On 27.02.2023 14:44, Pedro Alves wrote: >> On 2023-02-27 9:03 a.m., Jan Beulich via Binutils wrote: >>> On 24.02.2023 15:57, Tom de Vries wrote: >>>> >>>> Is this more how you want it? >>> >>> I have to admit that I'm still puzzled by the presence of >>> finalize_parse_compress_debug_optarg() as well as you needing both a new >>> static variable and a new global one. But I guess whether that's really >>> needed first of all depends on the semantics we want e.g. >>> >>> --nocompress-debug-sections --compress-debug-sections=force >>> >>> to have (which, with how you have it presently, could also be expressed >>> as >>> >>> --compress-debug-sections=none+force >>> >>> or >>> >>> --compress-debug-sections=none --compress-debug-sections=force >>> >>> afaict). I view the present meaning as one sensible one, but I could >>> also see "none" (or equivalent) simply zapping the compression type >>> (and hence rendering "force" meaningless) as another sensible one. A >>> change in meaning may then also result in the three option combinations >>> above possibly not all doing the same. >> >> >> ISTM that these confusions (which no doubt users will have too) would go away if >> we did not try to put orthogonal settings into one option. >> >> Witness how the implementation uses two different enums: >> >> This new one: >> >> +enum compress_debug_action >> +{ >> + cda_default, >> + cda_none, >> + cda_force, >> + cda_yes, >> +}; >> >> And this preexisting one: >> >> /* Types of compressed DWARF debug sections. */ >> enum compressed_debug_section_type >> { >> COMPRESS_DEBUG_NONE = 0, >> COMPRESS_DEBUG_GNU_ZLIB = 1 << 1, >> COMPRESS_DEBUG_GABI_ZLIB = 1 << 2, >> COMPRESS_DEBUG_ZSTD = 1 << 3, >> COMPRESS_UNKNOWN = 1 << 4 >> }; >> >> Imagine we started over from scratch, and had these two orthogonal options, >> matching internal enums (enum compress_debug_action would be slightly different): >> >> --compressed-debug-sections-format=zlib|zstd|...|none >> >> # Iff we're compressing, what format shall we use? >> >> --compress-debug-sections=no|yes|sizewin >> >> # Are we compressing debug sections? When? >> # >> # - "no" - never, we're not compressing. >> # >> # - "yes" - always, we're compressing, using the format specified >> # by --compressed-debug-sections-format (and if that is "none", well, >> # you still get what you asked for). >> # >> # - "sizewin" - compress if there's a size win. Like "yes", if the format is >> # "none", well, this becomes a nop. >> >> ... then the semantics of mixing these options, or what happens when you repeat them >> would be much more obvious: >> >> - You can specify '--compressed-debug-sections-format' multiple times. The last wins. >> >> - You can specify '--compress-debug-sections' multiple times. The last wins. >> >> - Changing '--compressed-debug-sections-format' does not affect the value of '--compress-debug-sections'. >> >> - Changing '--compress-debug-sections' does not affect the value of '--compressed-debug-sections-format' >> >> >> Now, we can't use those option names with that meaning, though, because >> '--compress-debug-sections' today already has the meaning of selecting >> the compression format. But that just means we would need to pick different >> names, like for example: >> >> --compress-debug-sections=zlib|zstd|none >> >> # Iff we're compressing, what format shall we use? >> >> --nocompress-debug-sections >> >> # Shorthand for --compress-debug-sections=none >> >> --compress-debug-sections-when=never|always|sizewin >> >> # Are we compressing debug sections? When? >> # >> # - "never" - never, we're not compressing. >> # >> # - "always" - always compress, using the format specified by >> # by --compress-debug-sections (and if that is "none", well, >> # you still get what you asked for). >> # >> # - "sizewin", compress if there's a size win. Like "always", respects >> # the format specified by --compress-debug-sections. >> >> >> The semantics of mixing these options, or what happens when you repeat them >> would be obvious in the same way in the other options naming earlier: >> >> - You can specify '--compress-debug-sections' multiple times. The last wins. >> >> - You can specify '--compress-debug-sections-when' multiple times. The last wins. >> >> - Changing '--compress-debug-sections' does not affect the value of '--compress-debug-sections-when'. >> >> - Changing '--compress-debug-sections-when' does not affect the value of '--compressed-debug-sections' >> >> You end up with two different ways to disable compressing debug sections, >> but that seems OK to me. > > That would be okay with me as well, but (in part to avoid ... > >> All that would be left would be bikeshed on the new option name. > > ... this aspect) I fail to see why a single option won't do: > > --compress-debug-sections=zlib|zstd|no|yes|sizewin > > Since the sub-options are distinct sets (alongside "no" one could add "none" > if wanted, to separate "method: no compression" from "don't compress", which > I think would then also eliminate the ambiguity referred to earlier on > [still visible in context above]), within each set your "last wins" would > apply equally. And in fact that was the main reason for my original response > back to Tom: I view the proliferation of command line options (not just > here, but in general) as a scalability issue. Therefore my rule of thumb is > that things which belong together are best grouped under a single top-level > option. OK, so say we have --compress-debug-sections=none|zlib|zstd|no|yes|sizewin. We have the two existing aliases, --compress-debug-sections and --nocompress-debug-sections, how do we translate those? Currently --nocompress-debug-sections maps to --compress-debug-sections=none, but I suppose that would have to become --compress-debug-sections=no, otherwise things will get extremely confusing. Then --compress-debug-sections currently maps to --compress-debug-sections=zlib. So, should that become --compress-debug-sections=zlib,sizewin? Or just stay --compress-debug-sections=zlib? I'm not sure if we have made things clearer or more orthogonal this way. I'm starting to wonder if not a simple --compress-debug-sections=heuristic-sizewin|heuristic-always is the solution, in the sense that these two would only influence the heuristics and nothing else, which would work well with the existing options. Thanks, - Tom ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-27 23:24 ` Tom de Vries @ 2023-02-28 0:19 ` Tom de Vries 2023-02-28 13:21 ` Pedro Alves 1 sibling, 0 replies; 24+ messages in thread From: Tom de Vries @ 2023-02-28 0:19 UTC (permalink / raw) To: Jan Beulich, Pedro Alves; +Cc: binutils, Michael Matz, Nick Clifton, Alan Modra On 2/28/23 00:24, Tom de Vries wrote: > I'm starting to wonder if not a simple > --compress-debug-sections=heuristic-sizewin|heuristic-always is the > solution, in the sense that these two would only influence the > heuristics and nothing else, which would work well with the existing > options. Submitted a v2 patch series implementing this approach here ( https://sourceware.org/pipermail/binutils/2023-February/126335.html ). Thanks, - Tom ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-27 23:24 ` Tom de Vries 2023-02-28 0:19 ` Tom de Vries @ 2023-02-28 13:21 ` Pedro Alves 1 sibling, 0 replies; 24+ messages in thread From: Pedro Alves @ 2023-02-28 13:21 UTC (permalink / raw) To: Tom de Vries, Jan Beulich Cc: binutils, Michael Matz, Nick Clifton, Alan Modra On 2023-02-27 11:24 p.m., Tom de Vries wrote: > On 2/27/23 15:07, Jan Beulich wrote: > OK, so say we have --compress-debug-sections=none|zlib|zstd|no|yes|sizewin. > > We have the two existing aliases, --compress-debug-sections and --nocompress-debug-sections, how do we translate those? > > Currently --nocompress-debug-sections maps to --compress-debug-sections=none, but I suppose that would have to become --compress-debug-sections=no, otherwise things will get extremely confusing. Let's drop the new (when) "no" option, as it's not really needed, and rename "yes" -> when-always, and "sizewin" -> "when-always". So we end up with: --compress-debug-sections=none|zlib|zstd|when-always|when-sizewin ^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^ group1 group2 And then the "no" in nocompress-debug-sections is no longer ambiguous. > > Then --compress-debug-sections currently maps to --compress-debug-sections=zlib. So, should that become --compress-debug-sections=zlib,sizewin? Or just stay --compress-debug-sections=zlib? I'm not sure if we have made things clearer or more orthogonal this way. I think it's clearer to make it map to whatever are the defaults in all subgroups, so no subgroup is special cased. The default compression format is zlib, and the default "when" is "sizewin", so: "--compress-debug-sections" => "--compress-debug-sections=zlib,sizewin" And then: "--compress-debug-sections=always --compress-debug-sections" => "--compress-debug-sections=zlib,sizewin" > > I'm starting to wonder if not a simple --compress-debug-sections=heuristic-sizewin|heuristic-always is the solution, in the sense that these two would only influence the heuristics and nothing else, which would work well with the existing options. I prefer "when" (or no leading prefix) to "heuristic", as "always" is not an heuristic, there's no trial and error nor is it loosely defined what it does. But that's a nitpick, it's actually fine with me. FWIW. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-27 14:07 ` Jan Beulich 2023-02-27 23:24 ` Tom de Vries @ 2023-02-28 12:49 ` Pedro Alves 1 sibling, 0 replies; 24+ messages in thread From: Pedro Alves @ 2023-02-28 12:49 UTC (permalink / raw) To: Jan Beulich Cc: binutils, Michael Matz, Nick Clifton, Alan Modra, Tom de Vries On 2023-02-27 2:07 p.m., Jan Beulich wrote: > On 27.02.2023 14:44, Pedro Alves wrote: >> On 2023-02-27 9:03 a.m., Jan Beulich via Binutils wrote: >>> On 24.02.2023 15:57, Tom de Vries wrote: >>>> >>>> Is this more how you want it? >>> >>> I have to admit that I'm still puzzled by the presence of >>> finalize_parse_compress_debug_optarg() as well as you needing both a new >>> static variable and a new global one. But I guess whether that's really >>> needed first of all depends on the semantics we want e.g. >>> >>> --nocompress-debug-sections --compress-debug-sections=force >>> >>> to have (which, with how you have it presently, could also be expressed >>> as >>> >>> --compress-debug-sections=none+force >>> >>> or >>> >>> --compress-debug-sections=none --compress-debug-sections=force >>> >>> afaict). I view the present meaning as one sensible one, but I could >>> also see "none" (or equivalent) simply zapping the compression type >>> (and hence rendering "force" meaningless) as another sensible one. A >>> change in meaning may then also result in the three option combinations >>> above possibly not all doing the same. >> >> >> ISTM that these confusions (which no doubt users will have too) would go away if >> we did not try to put orthogonal settings into one option. >> >> Witness how the implementation uses two different enums: >> >> This new one: >> >> +enum compress_debug_action >> +{ >> + cda_default, >> + cda_none, >> + cda_force, >> + cda_yes, >> +}; >> >> And this preexisting one: >> >> /* Types of compressed DWARF debug sections. */ >> enum compressed_debug_section_type >> { >> COMPRESS_DEBUG_NONE = 0, >> COMPRESS_DEBUG_GNU_ZLIB = 1 << 1, >> COMPRESS_DEBUG_GABI_ZLIB = 1 << 2, >> COMPRESS_DEBUG_ZSTD = 1 << 3, >> COMPRESS_UNKNOWN = 1 << 4 >> }; >> >> Imagine we started over from scratch, and had these two orthogonal options, >> matching internal enums (enum compress_debug_action would be slightly different): >> >> --compressed-debug-sections-format=zlib|zstd|...|none >> >> # Iff we're compressing, what format shall we use? >> >> --compress-debug-sections=no|yes|sizewin >> >> # Are we compressing debug sections? When? >> # >> # - "no" - never, we're not compressing. >> # >> # - "yes" - always, we're compressing, using the format specified >> # by --compressed-debug-sections-format (and if that is "none", well, >> # you still get what you asked for). >> # >> # - "sizewin" - compress if there's a size win. Like "yes", if the format is >> # "none", well, this becomes a nop. >> >> ... then the semantics of mixing these options, or what happens when you repeat them >> would be much more obvious: >> >> - You can specify '--compressed-debug-sections-format' multiple times. The last wins. >> >> - You can specify '--compress-debug-sections' multiple times. The last wins. >> >> - Changing '--compressed-debug-sections-format' does not affect the value of '--compress-debug-sections'. >> >> - Changing '--compress-debug-sections' does not affect the value of '--compressed-debug-sections-format' >> >> >> Now, we can't use those option names with that meaning, though, because >> '--compress-debug-sections' today already has the meaning of selecting >> the compression format. But that just means we would need to pick different >> names, like for example: >> >> --compress-debug-sections=zlib|zstd|none >> >> # Iff we're compressing, what format shall we use? >> >> --nocompress-debug-sections >> >> # Shorthand for --compress-debug-sections=none >> >> --compress-debug-sections-when=never|always|sizewin >> >> # Are we compressing debug sections? When? >> # >> # - "never" - never, we're not compressing. >> # >> # - "always" - always compress, using the format specified by >> # by --compress-debug-sections (and if that is "none", well, >> # you still get what you asked for). >> # >> # - "sizewin", compress if there's a size win. Like "always", respects >> # the format specified by --compress-debug-sections. >> >> >> The semantics of mixing these options, or what happens when you repeat them >> would be obvious in the same way in the other options naming earlier: >> >> - You can specify '--compress-debug-sections' multiple times. The last wins. >> >> - You can specify '--compress-debug-sections-when' multiple times. The last wins. >> >> - Changing '--compress-debug-sections' does not affect the value of '--compress-debug-sections-when'. >> >> - Changing '--compress-debug-sections-when' does not affect the value of '--compressed-debug-sections' >> >> You end up with two different ways to disable compressing debug sections, >> but that seems OK to me. Actually, thinking back, we could just drop --compress-debug-sections-when=never, it's not needed. > > That would be okay with me as well, but (in part to avoid ... > >> All that would be left would be bikeshed on the new option name. > > ... this aspect) I fail to see why a single option won't do: > > --compress-debug-sections=zlib|zstd|no|yes|sizewin > > Since the sub-options are distinct sets (alongside "no" one could add "none" > if wanted, to separate "method: no compression" from "don't compress", which > I think would then also eliminate the ambiguity referred to earlier on > [still visible in context above]), within each set your "last wins" would > apply equally. Yes. As long as the sets are treated distinct, the same logic can be applied. > And in fact that was the main reason for my original response > back to Tom: I view the proliferation of command line options (not just > here, but in general) as a scalability issue. Therefore my rule of thumb is > that things which belong together are best grouped under a single top-level > option. I don't understand what you mean by scalability issue. Certainly adding more options to a tool increases the support burden as it adds more combinations to a support&test matrix. However, in this case, we're adding a new column to the support&test matrix, regardless of whether we add it as a new option, or as a distinct set within the preexisting option. The number of supported combinations is the same regardless. The separate options approach has the advantage of being clearer and easier to explain, and doesn't have the "+" vs "," parsing issue. But the single option with distinct sets also works for me. This seems to be what Tom ended up with in his latest revision, though I haven't read his patches. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-23 13:27 ` Tom de Vries 2023-02-23 13:44 ` Jan Beulich @ 2023-02-23 15:23 ` Michael Matz 2023-02-23 15:28 ` Tom de Vries 1 sibling, 1 reply; 24+ messages in thread From: Michael Matz @ 2023-02-23 15:23 UTC (permalink / raw) To: Tom de Vries; +Cc: Jan Beulich, binutils Hello, On Thu, 23 Feb 2023, Tom de Vries via Binutils wrote: > Or instead combined: --compress-debug-sections=force,zstd. Harder to parse > though, I suppose. > > I guess this last one would be my preference, because it makes it clear force > is in a different category than zlib/zstd. Regardless of the outcome how to spell this on the cmdline, can the force-mode please be explicitely documented as doing stuff "even if it increases section size", at least in the .info docu (not necessarily in --help output). Because, knowing users trying random options, they might be tempted to use the force-mode, because "it surely will be better" and the remark would hopefully deflect this. Ciao, Michael. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-23 15:23 ` Michael Matz @ 2023-02-23 15:28 ` Tom de Vries 2023-02-23 15:44 ` Michael Matz 0 siblings, 1 reply; 24+ messages in thread From: Tom de Vries @ 2023-02-23 15:28 UTC (permalink / raw) To: Michael Matz; +Cc: Jan Beulich, binutils On 2/23/23 16:23, Michael Matz wrote: > Hello, > > On Thu, 23 Feb 2023, Tom de Vries via Binutils wrote: > >> Or instead combined: --compress-debug-sections=force,zstd. Harder to parse >> though, I suppose. >> >> I guess this last one would be my preference, because it makes it clear force >> is in a different category than zlib/zstd. > > Regardless of the outcome how to spell this on the cmdline, can the > force-mode please be explicitely documented as doing stuff "even if it > increases section size", at least in the .info docu (not necessarily in > --help output). Because, knowing users trying random options, they might > be tempted to use the force-mode, because "it surely will be better" and > the remark would hopefully deflect this. > The submitted version ( https://sourceware.org/pipermail/binutils/2023-February/126284.html ) contains: ... @@ -718,7 +719,8 @@ Begin in alternate macro mode. Compress DWARF debug sections using zlib with SHF_COMPRESSED from the ELF ABI. The resulting object file may not be compatible with older linkers and object file utilities. Note if compression would make a -given section @emph{larger} then it is not compressed. +given section @emph{larger} then it is not compressed, unless +@option{--force-compress-debug-section} is used. @ifset ELF @cindex @samp{--compress-debug-sections=} option @@ -738,7 +740,11 @@ using the obsoleted zlib-gnu format. The debug sections are renamed to begin with @samp{.zdebug}. @option{--compress-debug-sections=zstd} compresses DWARF debug sections using zstd. Note - if compression would actually make a section -@emph{larger}, then it is not compressed nor renamed. +@emph{larger}, then it is not compressed nor renamed, unless +@option{--force-compress-debug-section} is used. + +@item --force-compress-debug-sections +Compress DWARF debug sections, even if this does not reduce size. @end ifset ... So, do you consider this insufficient? Thanks, - Tom ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-23 15:28 ` Tom de Vries @ 2023-02-23 15:44 ` Michael Matz 2023-02-23 15:46 ` Tom de Vries 0 siblings, 1 reply; 24+ messages in thread From: Michael Matz @ 2023-02-23 15:44 UTC (permalink / raw) To: Tom de Vries; +Cc: Jan Beulich, binutils Hey Tom, On Thu, 23 Feb 2023, Tom de Vries wrote: > The submitted version ( > https://sourceware.org/pipermail/binutils/2023-February/126284.html ) > contains: > ... > @@ -718,7 +719,8 @@ Begin in alternate macro mode. > Compress DWARF debug sections using zlib with SHF_COMPRESSED from the > ELF ABI. The resulting object file may not be compatible with older > linkers and object file utilities. Note if compression would make a > -given section @emph{larger} then it is not compressed. > +given section @emph{larger} then it is not compressed, unless > +@option{--force-compress-debug-section} is used. Gah! I overlooked this snippet and the one below that. > So, do you consider this insufficient? No, it's completely fine IMHO :) Ciao, Michael. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] gas: Add --force-compress-debug-sections 2023-02-23 15:44 ` Michael Matz @ 2023-02-23 15:46 ` Tom de Vries 0 siblings, 0 replies; 24+ messages in thread From: Tom de Vries @ 2023-02-23 15:46 UTC (permalink / raw) To: Michael Matz; +Cc: Jan Beulich, binutils On 2/23/23 16:44, Michael Matz wrote: > Hey Tom, > > On Thu, 23 Feb 2023, Tom de Vries wrote: > >> The submitted version ( >> https://sourceware.org/pipermail/binutils/2023-February/126284.html ) >> contains: >> ... >> @@ -718,7 +719,8 @@ Begin in alternate macro mode. >> Compress DWARF debug sections using zlib with SHF_COMPRESSED from the >> ELF ABI. The resulting object file may not be compatible with older >> linkers and object file utilities. Note if compression would make a >> -given section @emph{larger} then it is not compressed. >> +given section @emph{larger} then it is not compressed, unless >> +@option{--force-compress-debug-section} is used. > > Gah! I overlooked this snippet and the one below that. > >> So, do you consider this insufficient? > > No, it's completely fine IMHO :) Cool, thanks :) - Tom ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2023-02-28 13:21 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-02-23 12:45 [PATCH] gas: Add --force-compress-debug-sections Tom de Vries 2023-02-23 13:08 ` Jan Beulich 2023-02-23 13:27 ` Tom de Vries 2023-02-23 13:44 ` Jan Beulich 2023-02-24 10:52 ` Tom de Vries 2023-02-24 11:28 ` Jan Beulich 2023-02-24 12:21 ` Tom de Vries 2023-02-24 13:23 ` Jan Beulich 2023-02-24 14:11 ` Tom de Vries 2023-02-24 14:26 ` Jan Beulich 2023-02-24 14:57 ` Tom de Vries 2023-02-27 9:03 ` Jan Beulich 2023-02-27 11:43 ` [PATCH] gas: Add --compress-debug-sections=force Tom de Vries 2023-02-27 11:51 ` Jan Beulich 2023-02-27 13:44 ` [PATCH] gas: Add --force-compress-debug-sections Pedro Alves 2023-02-27 14:07 ` Jan Beulich 2023-02-27 23:24 ` Tom de Vries 2023-02-28 0:19 ` Tom de Vries 2023-02-28 13:21 ` Pedro Alves 2023-02-28 12:49 ` Pedro Alves 2023-02-23 15:23 ` Michael Matz 2023-02-23 15:28 ` Tom de Vries 2023-02-23 15:44 ` Michael Matz 2023-02-23 15:46 ` Tom de Vries
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).