[RFC] add --enable-zstd-compressed-debug-sections configure option

Message ID 61355429-24b3-17d0-ab03-6fa57ee861d5@suse.cz
State Accepted, archived
Headers
Series [RFC] add --enable-zstd-compressed-debug-sections configure option |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Martin Liška Sept. 30, 2022, 9:48 a.m. UTC
  Hello.

The patch can set up default compression algorithm as zstd instead of the
default zlib-gabi.

The patch is lightly tested as readelf can't decompress debug sections:
https://sourceware.org/bugzilla/show_bug.cgi?id=29640

Thoughts?

ChangeLog:

	* configure.ac: Add --enable-zstd-compressed-debug-sections
	configure option.
	* configure: Regenerate.

gas/ChangeLog:

	* NEWS: Mention --enable-zstd-compressed-debug-sections.
	* as.c: Respect FLAG_ZSTD_COMPRESS_DEBUG and should it in
	--help.
	* configure.ac: Add --enable-zstd-compressed-debug-sections
	configure option.
	* configure: Regenerate.
	* config.in: Regenerate.

ld/ChangeLog:

	* NEWS: Mention --enable-zstd-compressed-debug-sections.
	* configure.ac: Add --enable-zstd-compressed-debug-sections
	configure option.
	* configure: Regenerate.
	* config.in: Regenerate.
	* ldmain.c: Respenct FLAG_ZSTD_COMPRESS_DEBUG.
	* lexsup.c: Likewise.
---
 configure        | 17 +++++++++++++++++
 configure.ac     |  6 ++++++
 gas/NEWS         |  3 +++
 gas/as.c         | 24 ++++++++++++++++--------
 gas/config.in    |  3 +++
 gas/configure    | 20 ++++++++++++++++++--
 gas/configure.ac | 10 ++++++++++
 ld/NEWS          |  3 +++
 ld/config.in     |  3 +++
 ld/configure     | 20 ++++++++++++++++++--
 ld/configure.ac  | 10 ++++++++++
 ld/ldmain.c      |  4 ++++
 ld/lexsup.c      |  5 +++++
 13 files changed, 116 insertions(+), 12 deletions(-)
  

Comments

Pedro Alves Sept. 30, 2022, 11:25 a.m. UTC | #1
On 2022-09-30 10:48 a.m., Martin Liška wrote:
> Hello.
> 
> The patch can set up default compression algorithm as zstd instead of the
> default zlib-gabi.
> 
> The patch is lightly tested as readelf can't decompress debug sections:
> https://sourceware.org/bugzilla/show_bug.cgi?id=29640
> 
> Thoughts?
> 
> ChangeLog:
> 
> 	* configure.ac: Add --enable-zstd-compressed-debug-sections
> 	configure option.
> 	* configure: Regenerate.

This may become a bit awkward in the future when other better format appears, and you want to
switch to use it by default.  Like, imagine zstd2 is invented.  At that point we'd have to decide
whether to add code to error out if the user specifies both
   "--enable-zstd-compressed-debug-sections --enable-zstd2-compressed-debug-sections",
or always pick the latter option, or some such.  

IMHO, it seems cleaner and more future proof to add instead:

  --enable-default-compressed-debug-sections=zlib|zlib-gnu|zlib-gabi|zstd

WDYT?

Pedro Alves
  
Martin Liška Sept. 30, 2022, 12:42 p.m. UTC | #2
On 9/30/22 13:25, Pedro Alves wrote:
> On 2022-09-30 10:48 a.m., Martin Liška wrote:
>> Hello.
>>
>> The patch can set up default compression algorithm as zstd instead of the
>> default zlib-gabi.
>>
>> The patch is lightly tested as readelf can't decompress debug sections:
>> https://sourceware.org/bugzilla/show_bug.cgi?id=29640
>>
>> Thoughts?
>>
>> ChangeLog:
>>
>> 	* configure.ac: Add --enable-zstd-compressed-debug-sections
>> 	configure option.
>> 	* configure: Regenerate.
> 
> This may become a bit awkward in the future when other better format appears, and you want to
> switch to use it by default.  Like, imagine zstd2 is invented.  At that point we'd have to decide
> whether to add code to error out if the user specifies both
>    "--enable-zstd-compressed-debug-sections --enable-zstd2-compressed-debug-sections",
> or always pick the latter option, or some such.  

Understood.

> 
> IMHO, it seems cleaner and more future proof to add instead:
> 
>   --enable-default-compressed-debug-sections=zlib|zlib-gnu|zlib-gabi|zstd
> 
> WDYT?

Yep, it's much nicer. Anyway, lemme try preparing a patch that does a bit of refactoring
before I introduce such a patch.

Martin

> 
> Pedro Alves
  
Fangrui Song Oct. 1, 2022, 7:31 a.m. UTC | #3
On 2022-09-30, Martin Liška wrote:
>On 9/30/22 13:25, Pedro Alves wrote:
>> On 2022-09-30 10:48 a.m., Martin Liška wrote:
>>> Hello.
>>>
>>> The patch can set up default compression algorithm as zstd instead of the
>>> default zlib-gabi.
>>>
>>> The patch is lightly tested as readelf can't decompress debug sections:
>>> https://sourceware.org/bugzilla/show_bug.cgi?id=29640
>>>
>>> Thoughts?
>>>
>>> ChangeLog:
>>>
>>> 	* configure.ac: Add --enable-zstd-compressed-debug-sections
>>> 	configure option.
>>> 	* configure: Regenerate.
>>
>> This may become a bit awkward in the future when other better format appears, and you want to
>> switch to use it by default.  Like, imagine zstd2 is invented.  At that point we'd have to decide
>> whether to add code to error out if the user specifies both
>>    "--enable-zstd-compressed-debug-sections --enable-zstd2-compressed-debug-sections",
>> or always pick the latter option, or some such.
>
>Understood.
>
>>
>> IMHO, it seems cleaner and more future proof to add instead:
>>
>>   --enable-default-compressed-debug-sections=zlib|zlib-gnu|zlib-gabi|zstd
>>
>> WDYT?
>
>Yep, it's much nicer. Anyway, lemme try preparing a patch that does a bit of refactoring
>before I introduce such a patch.
>
>Martin

Personally I'll prefer that the compiler drivers (GCC and Clang) pass
the default options to ld/as, and they have good infrastructure for
customization...  (Clang recently got better configuration file
(https://clang.llvm.org/docs/UsersManual.html#configuration-files)
support which enables more convenient default driver options.)

Setting default options in ld/as allow very few projects which bypass
the compiler driver to use zstd compressed debug sections but the
benefit is probably low...
  
Martin Liška Oct. 3, 2022, 7:49 a.m. UTC | #4
On 10/1/22 09:31, Fangrui Song wrote:
> On 2022-09-30, Martin Liška wrote:
>> On 9/30/22 13:25, Pedro Alves wrote:
>>> On 2022-09-30 10:48 a.m., Martin Liška wrote:
>>>> Hello.
>>>>
>>>> The patch can set up default compression algorithm as zstd instead of the
>>>> default zlib-gabi.
>>>>
>>>> The patch is lightly tested as readelf can't decompress debug sections:
>>>> https://sourceware.org/bugzilla/show_bug.cgi?id=29640
>>>>
>>>> Thoughts?
>>>>
>>>> ChangeLog:
>>>>
>>>>     * configure.ac: Add --enable-zstd-compressed-debug-sections
>>>>     configure option.
>>>>     * configure: Regenerate.
>>>
>>> This may become a bit awkward in the future when other better format appears, and you want to
>>> switch to use it by default.  Like, imagine zstd2 is invented.  At that point we'd have to decide
>>> whether to add code to error out if the user specifies both
>>>    "--enable-zstd-compressed-debug-sections --enable-zstd2-compressed-debug-sections",
>>> or always pick the latter option, or some such.
>>
>> Understood.
>>
>>>
>>> IMHO, it seems cleaner and more future proof to add instead:
>>>
>>>   --enable-default-compressed-debug-sections=zlib|zlib-gnu|zlib-gabi|zstd
>>>
>>> WDYT?
>>
>> Yep, it's much nicer. Anyway, lemme try preparing a patch that does a bit of refactoring
>> before I introduce such a patch.
>>
>> Martin
> 
> Personally I'll prefer that the compiler drivers (GCC and Clang) pass
> the default options to ld/as, and they have good infrastructure for
> customization...  (Clang recently got better configuration file
> (https://clang.llvm.org/docs/UsersManual.html#configuration-files)
> support which enables more convenient default driver options.)
> 
> Setting default options in ld/as allow very few projects which bypass
> the compiler driver to use zstd compressed debug sections but the
> benefit is probably low...

Well, I would prefer having the configure option, similar to 
--enable-compressed-debug-sections, we as openSUSE rely on that where
we select the corresponding option value in binutils package.

Martin
  

Patch

diff --git a/configure b/configure
index f14e0efd675..03c26c455c7 100755
--- a/configure
+++ b/configure
@@ -792,6 +792,7 @@  enable_gold
 enable_ld
 enable_gprofng
 enable_compressed_debug_sections
+enable_zstd_compressed_debug_sections
 enable_year2038
 enable_libquadmath
 enable_libquadmath_support
@@ -1523,6 +1524,9 @@  Optional Features:
   --enable-compressed-debug-sections={all,gas,gold,ld,none}
                           Enable compressed debug sections for gas, gold or ld
                           by default
+  --enable-zstd-compressed-debug-sections
+                          Use zstd for debug info sections compression by
+                          default.
   --enable-year2038       enable support for timestamps past the year 2038
   --disable-libquadmath   do not build libquadmath directory
   --disable-libquadmath-support
@@ -3119,6 +3123,19 @@  else
 fi
 
 
+
+# Check whether --enable-zstd_compressed_debug_sections was given.
+if test "${enable_zstd_compressed_debug_sections+set}" = set; then :
+  enableval=$enable_zstd_compressed_debug_sections;
+  if test x"$enable_compressed_debug_sections" = xyes; then
+    as_fn_error $? "no program with compressed debug sections specified" "$LINENO" 5
+  fi
+
+else
+  zstd_compressed_debug_sections=
+fi
+
+
 # Configure extra directories which are host specific
 
 case "${host}" in
diff --git a/configure.ac b/configure.ac
index 0152c69292e..1728991ab8a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -422,6 +422,12 @@  AC_ARG_ENABLE(compressed_debug_sections,
   fi
 ], [enable_compressed_debug_sections=])
 
+
+AC_ARG_ENABLE(zstd_compressed_debug_sections,
+[AS_HELP_STRING([--enable-zstd-compressed-debug-sections],
+		[Use zstd for debug info sections compression by default.])],
+[], [zstd_compressed_debug_sections=])
+
 # Configure extra directories which are host specific
 
 case "${host}" in
diff --git a/gas/NEWS b/gas/NEWS
index 9a8b726b942..8f509f3aebf 100644
--- a/gas/NEWS
+++ b/gas/NEWS
@@ -2,6 +2,9 @@ 
 
 * gas now supports --compress-debug-sections=zstd to compress
   debug sections with zstd.
+* Add a configure option --enable-zstd-compressed-debug-sections
+  in order to use zstd as a default compression algorithm when
+  --compress-debug-sections is used (or enabled by default).
 
 Changes in 2.39:
 
diff --git a/gas/as.c b/gas/as.c
index 9ce3d622f95..8c9c1d7abfd 100644
--- a/gas/as.c
+++ b/gas/as.c
@@ -226,8 +226,12 @@  print_version_id (void)
 
 #ifdef DEFAULT_FLAG_COMPRESS_DEBUG
 enum compressed_debug_section_type flag_compress_debug
+#if FLAG_ZSTD_COMPRESS_DEBUG
+  = COMPRESS_DEBUG_ZSTD;
+#else
   = COMPRESS_DEBUG_GABI_ZLIB;
 #endif
+#endif
 
 static void
 show_usage (FILE * stream)
@@ -250,21 +254,25 @@  Options:\n\
 
   fprintf (stream, _("\
   --alternate             initially turn on alternate macro syntax\n"));
-#ifdef DEFAULT_FLAG_COMPRESS_DEBUG
   fprintf (stream, _("\
   --compress-debug-sections[={none|zlib|zlib-gnu|zlib-gabi|zstd}]\n\
-                          compress DWARF debug sections using zlib [default]\n"));
+                          compress DWARF debug sections\n"));
+#ifdef DEFAULT_FLAG_COMPRESS_DEBUG
+#if FLAG_ZSTD_COMPRESS_DEBUG
   fprintf (stream, _("\
-  --nocompress-debug-sections\n\
-                          don't compress DWARF debug sections\n"));
+                            Default: zstd\n"));
 #else
   fprintf (stream, _("\
-  --compress-debug-sections[={none|zlib|zlib-gnu|zlib-gabi|zstd}]\n\
-                          compress DWARF debug sections using zlib\n"));
+                            Default: zlib-gabi\n"));
+#endif
+#else
   fprintf (stream, _("\
-  --nocompress-debug-sections\n\
-                          don't compress DWARF debug sections [default]\n"));
+                            Default: none\n"));
 #endif
+
+  fprintf (stream, _("\
+  --nocompress-debug-sections\n\
+                          don't compress DWARF debug sections\n"));
   fprintf (stream, _("\
   -D                      produce assembler debugging messages\n"));
   fprintf (stream, _("\
diff --git a/gas/config.in b/gas/config.in
index 0d1668a3eac..35b8c0cc26a 100644
--- a/gas/config.in
+++ b/gas/config.in
@@ -71,6 +71,9 @@ 
    language is requested. */
 #undef ENABLE_NLS
 
+/* Define to 1 if you want compressed debug sections with zstd by default. */
+#undef FLAG_ZSTD_COMPRESS_DEBUG
+
 /* Define to 1 if you have the declaration of `asprintf', and to 0 if you
    don't. */
 #undef HAVE_DECL_ASPRINTF
diff --git a/gas/configure b/gas/configure
index 02cded59b6a..10a095f89c7 100755
--- a/gas/configure
+++ b/gas/configure
@@ -810,6 +810,7 @@  enable_largefile
 enable_targets
 enable_checking
 enable_compressed_debug_sections
+enable_zstd_compressed_debug_sections
 enable_x86_relax_relocations
 enable_elf_stt_common
 enable_generate_build_notes
@@ -1476,6 +1477,9 @@  Optional Features:
   --enable-checking       enable run-time checks
   --enable-compressed-debug-sections={all,gas,none}
                           compress debug sections by default
+  --enable-zstd-compressed-debug-sections
+                          Use zstd for debug info sections compression by
+                          default
   --enable-x86-relax-relocations
                           generate x86 relax relocations by default
   --enable-elf-stt-common generate ELF common symbols with STT_COMMON type by
@@ -10722,7 +10726,7 @@  else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 10725 "configure"
+#line 10729 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -10828,7 +10832,7 @@  else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 10831 "configure"
+#line 10835 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11502,6 +11506,12 @@  if test "${enable_compressed_debug_sections+set}" = set; then :
 esac
 fi
 
+# Use zstd compression algorithm by default.
+# Check whether --enable-zstd_compressed_debug_sections was given.
+if test "${enable_zstd_compressed_debug_sections+set}" = set; then :
+  enableval=$enable_zstd_compressed_debug_sections; ac_zstd_compressed_debug_sections=$enableval
+fi
+
 # PR gas/19520
 # Decide if x86 assembler should generate relax relocations.
 ac_default_x86_relax_relocations=unset
@@ -12671,6 +12681,12 @@  $as_echo "#define DEFAULT_FLAG_COMPRESS_DEBUG 1" >>confdefs.h
 
 fi
 
+if test x$ac_zstd_compressed_debug_sections = xyes ; then
+
+$as_echo "#define FLAG_ZSTD_COMPRESS_DEBUG 1" >>confdefs.h
+
+fi
+
 # Turn on all targets if possible
 if test ${all_targets} = "yes"; then
   case ${target_cpu_type} in
diff --git a/gas/configure.ac b/gas/configure.ac
index e6f3298cb3e..b46acfe9033 100644
--- a/gas/configure.ac
+++ b/gas/configure.ac
@@ -76,6 +76,12 @@  AC_ARG_ENABLE(compressed_debug_sections,
   *)   ac_default_compressed_debug_sections=unset ;;
 esac])dnl
 
+# Use zstd compression algorithm by default.
+AC_ARG_ENABLE(zstd_compressed_debug_sections,
+	      AS_HELP_STRING([--enable-zstd-compressed-debug-sections],
+	      [Use zstd for debug info sections compression by default]),
+[ac_zstd_compressed_debug_sections=$enableval])dnl
+
 # PR gas/19520
 # Decide if x86 assembler should generate relax relocations.
 ac_default_x86_relax_relocations=unset
@@ -755,6 +761,10 @@  if test x$ac_default_compressed_debug_sections = xyes ; then
   AC_DEFINE(DEFAULT_FLAG_COMPRESS_DEBUG, 1, [Define if you want compressed debug sections by default.])
 fi
 
+if test x$ac_zstd_compressed_debug_sections = xyes ; then
+  AC_DEFINE(FLAG_ZSTD_COMPRESS_DEBUG, 1, [Define to 1 if you want compressed debug sections with zstd by default.])
+fi
+
 # Turn on all targets if possible
 if test ${all_targets} = "yes"; then
   case ${target_cpu_type} in
diff --git a/ld/NEWS b/ld/NEWS
index dfe2690d9f2..82619b3625e 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -2,6 +2,9 @@ 
 
 * ld now supports zstd compressed debug sections.  The new option
   --compress-debug-sections=zstd compresses debug sections with zstd.
+* Add a configure option --enable-zstd-compressed-debug-sections
+  in order to use zstd as a default compression algorithm when
+  --compress-debug-sections is used (or enabled by default).
 
 Changes in 2.39:
 
diff --git a/ld/config.in b/ld/config.in
index 3916740eee4..93ec9cee232 100644
--- a/ld/config.in
+++ b/ld/config.in
@@ -58,6 +58,9 @@ 
 /* Additional extension a shared object might have. */
 #undef EXTRA_SHLIB_EXTENSION
 
+/* Define to 1 if you want compressed debug sections with zstd by default. */
+#undef FLAG_ZSTD_COMPRESS_DEBUG
+
 /* Define to choose default GOT handling scheme */
 #undef GOT_HANDLING_DEFAULT
 
diff --git a/ld/configure b/ld/configure
index 9dd3ed5f1e7..fa21b5de04e 100755
--- a/ld/configure
+++ b/ld/configure
@@ -841,6 +841,7 @@  with_sysroot
 enable_gold
 enable_got
 enable_compressed_debug_sections
+enable_zstd_compressed_debug_sections
 enable_new_dtags
 enable_relro
 enable_textrel_check
@@ -1524,6 +1525,9 @@  Optional Features:
                           multigot)
   --enable-compressed-debug-sections={all,ld,none}
                           compress debug sections by default]
+  --enable-zstd-compressed-debug-sections
+                          Use zstd for debug info sections compression by
+                          default
   --enable-new-dtags      set DT_RUNPATH instead of DT_RPATH by default]
   --enable-relro          enable -z relro in ELF linker by default
   --enable-textrel-check=[yes|no|warning|error]
@@ -11620,7 +11624,7 @@  else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11623 "configure"
+#line 11627 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11726,7 +11730,7 @@  else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11729 "configure"
+#line 11733 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -15544,6 +15548,12 @@  if test "${enable_compressed_debug_sections+set}" = set; then :
 esac
 fi
 
+# Use zstd compression algorithm by default.
+# Check whether --enable-zstd_compressed_debug_sections was given.
+if test "${enable_zstd_compressed_debug_sections+set}" = set; then :
+  enableval=$enable_zstd_compressed_debug_sections; ac_zstd_compressed_debug_sections=$enableval
+fi
+
 # Decide setting DT_RUNPATH instead of DT_RPATH by default
 ac_default_new_dtags=unset
 # Provide a configure time option to override our default.
@@ -17340,6 +17350,12 @@  $as_echo "#define DEFAULT_FLAG_COMPRESS_DEBUG 1" >>confdefs.h
 
 fi
 
+if test x$ac_zstd_compressed_debug_sections = xyes ; then
+
+$as_echo "#define FLAG_ZSTD_COMPRESS_DEBUG 1" >>confdefs.h
+
+fi
+
 if test "${ac_default_new_dtags}" = unset; then
   ac_default_new_dtags=0
 fi
diff --git a/ld/configure.ac b/ld/configure.ac
index f1b2f9897f8..729f07343aa 100644
--- a/ld/configure.ac
+++ b/ld/configure.ac
@@ -163,6 +163,12 @@  AC_ARG_ENABLE(compressed_debug_sections,
   ,no, | ,none,)  ac_default_compressed_debug_sections=no ;;
 esac])dnl
 
+# Use zstd compression algorithm by default.
+AC_ARG_ENABLE(zstd_compressed_debug_sections,
+	      AS_HELP_STRING([--enable-zstd-compressed-debug-sections],
+	      [Use zstd for debug info sections compression by default]),
+[ac_zstd_compressed_debug_sections=$enableval])dnl
+
 # Decide setting DT_RUNPATH instead of DT_RPATH by default
 ac_default_new_dtags=unset
 # Provide a configure time option to override our default.
@@ -510,6 +516,10 @@  if test x$ac_default_compressed_debug_sections = xyes ; then
   AC_DEFINE(DEFAULT_FLAG_COMPRESS_DEBUG, 1, [Define if you want compressed debug sections by default.])
 fi
 
+if test x$ac_zstd_compressed_debug_sections = xyes ; then
+  AC_DEFINE(FLAG_ZSTD_COMPRESS_DEBUG, 1, [Define to 1 if you want compressed debug sections with zstd by default.])
+fi
+
 if test "${ac_default_new_dtags}" = unset; then
   ac_default_new_dtags=0
 fi
diff --git a/ld/ldmain.c b/ld/ldmain.c
index d63002c994a..d0ad5f3d797 100644
--- a/ld/ldmain.c
+++ b/ld/ldmain.c
@@ -352,8 +352,12 @@  main (int argc, char **argv)
   link_info.spare_dynamic_tags = 5;
   link_info.path_separator = ':';
 #ifdef DEFAULT_FLAG_COMPRESS_DEBUG
+#if FLAG_ZSTD_COMPRESS_DEBUG
+  link_info.compress_debug = COMPRESS_DEBUG_ZSTD;
+#else
   link_info.compress_debug = COMPRESS_DEBUG_GABI_ZLIB;
 #endif
+#endif
 #ifdef DEFAULT_NEW_DTAGS
   link_info.new_dtags = DEFAULT_NEW_DTAGS;
 #endif
diff --git a/ld/lexsup.c b/ld/lexsup.c
index 299371fb775..c46f1f822a8 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -2149,8 +2149,13 @@  elf_static_list_options (FILE *file)
   --compress-debug-sections=[none|zlib|zlib-gnu|zlib-gabi|zstd]\n\
 			      Compress DWARF debug sections\n"));
 #ifdef DEFAULT_FLAG_COMPRESS_DEBUG
+#if FLAG_ZSTD_COMPRESS_DEBUG
+  fprintf (file, _("\
+                                Default: zstd\n"));
+#else
   fprintf (file, _("\
                                 Default: zlib-gabi\n"));
+#endif
 #else
   fprintf (file, _("\
                                 Default: none\n"));