[PATCH] gas: Add --force-compress-debug-sections

[PATCH] gas: Add --force-compress-debug-sections

[Tom de Vries]

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

Re: [PATCH] gas: Add --force-compress-debug-sections

[Jan Beulich]

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

Re: [PATCH] gas: Add --force-compress-debug-sections

[Tom de Vries]

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.)

Re: [PATCH] gas: Add --force-compress-debug-sections

[Jan Beulich]

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

Re: [PATCH] gas: Add --force-compress-debug-sections

[Tom de Vries]

[-- 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

Re: [PATCH] gas: Add --force-compress-debug-sections

[Jan Beulich]

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

Re: [PATCH] gas: Add --force-compress-debug-sections

[Tom de Vries]

[-- 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

Re: [PATCH] gas: Add --force-compress-debug-sections

[Jan Beulich]

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

Re: [PATCH] gas: Add --force-compress-debug-sections

[Tom de Vries]

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.

Re: [PATCH] gas: Add --force-compress-debug-sections

[Jan Beulich]

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

Re: [PATCH] gas: Add --force-compress-debug-sections

[Tom de Vries]

[-- 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

Re: [PATCH] gas: Add --force-compress-debug-sections

[Jan Beulich]

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

[PATCH] gas: Add --compress-debug-sections=force

[Tom de Vries]

[-- 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

Re: [PATCH] gas: Add --compress-debug-sections=force

[Jan Beulich]

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

Re: [PATCH] gas: Add --force-compress-debug-sections

[Pedro Alves]

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
> 

Re: [PATCH] gas: Add --force-compress-debug-sections

[Jan Beulich]

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

Re: [PATCH] gas: Add --force-compress-debug-sections

[Tom de Vries]

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

Re: [PATCH] gas: Add --force-compress-debug-sections

[Tom de Vries]

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

Re: [PATCH] gas: Add --force-compress-debug-sections

[Pedro Alves]

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.

Re: [PATCH] gas: Add --force-compress-debug-sections

[Pedro Alves]

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.

Re: [PATCH] gas: Add --force-compress-debug-sections

[Michael Matz]

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.

Re: [PATCH] gas: Add --force-compress-debug-sections

[Tom de Vries]

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

Re: [PATCH] gas: Add --force-compress-debug-sections

[Michael Matz]

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.

Re: [PATCH] gas: Add --force-compress-debug-sections

[Tom de Vries]

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