Re: [PATCH] Remove legacy -gz=zlib-gnu

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Sep 20, 2022 at 2:55 PM Martin Liška <mliska@suse.cz> wrote:
>
> On 7/1/22 09:20, Fangrui Song via Gcc-patches wrote:
> > On 2022-07-01, Andrew Pinski wrote:
> >> On Thu, Jun 30, 2022 at 11:58 PM Fangrui Song via Gcc-patches
> >> <gcc-patches@gcc.gnu.org> wrote:
> >>>
> >>> From: Fangrui Song <i@maskray.me>
> >>>
> >>> SHF_COMPRESSED style zlib has been supported since binutils 2.26
> >>> and the legacy zlib-gnu option hasn't gain adoption.
> >>> According to Debian Code Search (`gz=zlib-gnu`), no project uses
> >>> -gz=zlib-gnu (valgrind has a configure to use -gz=zlib).
> >>> Remove support for the legacy zlib-gnu and simplify configure.ac by
> >>> removing zlib-gnu ld/as check.
> >>
> >> A couple of things, you are missing a changelog.
> >
> > Sorry.
> >
> >> Second, why remove something which is still working?
>
> Hi.
>
> I do support the option removal, while I would replace the removal with a warning
> saying no compression will be used.
>
> >
> > It's unused and its existence causes confusion: the paradox of choice.
> > People may assume the support may be good but newer DWARF consumers may
> > not support the legacy format.
>
> Agree, the compression format is legacy. I verified all openSUSE packages (15k)
> and there's no project actively using it.
>
> >
> > The other motivation is to clean up it a bit.  I foresee that someone
> > will add --compress-debug-sections=zstd to binutils and configure.ac and
> > gcc/gcc.cc would become more messy.
>
> The argument makes sense, it will be even bigger mess.
>
> @Richi: Is it something we can deprecate for GCC 13?

What's the practical difference between zlib and zlib-gnu?  Can we just
map zlib-gnu to zlib?  If it's all configure time what's the point in
"deprecating" it?

Richard.

>
> Martin
>
> >
> >> Third, why not just make gz=zlib-gnu as an alias to gz=zlib instead so
> >> if someone used it before it will still work. we try not to remove
> >> options; have them emit a warning and be ignored (or moved over to the
> >> closed option).
> >
> > Changing the semantics of -gz=zlib-gnu would be even more confusing.
> >
> >> Thanks,
> >> Andrew
> >>
> >>> ---
> >>>  gcc/common.opt      |  3 ---
> >>>  gcc/configure       | 33 ++++++---------------------------
> >>>  gcc/configure.ac    | 29 ++++-------------------------
> >>>  gcc/doc/invoke.texi | 11 +++++------
> >>>  gcc/gcc.cc          | 22 ++--------------------
> >>>  5 files changed, 17 insertions(+), 81 deletions(-)
> >>>
> >>> diff --git a/gcc/common.opt b/gcc/common.opt
> >>> index e7a51e882ba..8754d93d545 100644
> >>> --- a/gcc/common.opt
> >>> +++ b/gcc/common.opt
> >>> @@ -3424,9 +3424,6 @@ Enum(compressed_debug_sections) String(none) Value(0)
> >>>  EnumValue
> >>>  Enum(compressed_debug_sections) String(zlib) Value(1)
> >>>
> >>> -EnumValue
> >>> -Enum(compressed_debug_sections) String(zlib-gnu) Value(2)
> >>> -
> >>>  gz
> >>>  Common Driver
> >>>  Generate compressed debug sections.
> >>> diff --git a/gcc/configure b/gcc/configure
> >>> index 62872d132ea..ca87e875e9d 100755
> >>> --- a/gcc/configure
> >>> +++ b/gcc/configure
> >>> @@ -19674,7 +19674,7 @@ else
> >>>    lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
> >>>    lt_status=$lt_dlunknown
> >>>    cat > conftest.$ac_ext <<_LT_EOF
> >>> -#line 19679 "configure"
> >>> +#line 19677 "configure"
> >>>  #include "confdefs.h"
> >>>
> >>>  #if HAVE_DLFCN_H
> >>> @@ -19780,7 +19780,7 @@ else
> >>>    lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
> >>>    lt_status=$lt_dlunknown
> >>>    cat > conftest.$ac_ext <<_LT_EOF
> >>> -#line 19785 "configure"
> >>> +#line 19783 "configure"
> >>>  #include "confdefs.h"
> >>>
> >>>  #if HAVE_DLFCN_H
> >>> @@ -29711,20 +29711,13 @@ else
> >>>     if $gcc_cv_as --compress-debug-sections -o conftest.o conftest.s 2>&1 | grep -i warning > /dev/null
> >>>     then
> >>>       gcc_cv_as_compress_debug=0
> >>> -   # Since binutils 2.26, gas supports --compress-debug-sections=type,
> >>> +   # Since binutils 2.26, gas supports --compress-debug-sections=zlib,
> >>>     # defaulting to the ELF gABI format.
> >>> -   elif $gcc_cv_as --compress-debug-sections=zlib-gnu -o conftest.o conftest.s > /dev/null 2>&1
> >>> +   elif $gcc_cv_as --compress-debug-sections=zlib -o conftest.o conftest.s > /dev/null 2>&1
> >>>     then
> >>>       gcc_cv_as_compress_debug=2
> >>>       gcc_cv_as_compress_debug_option="--compress-debug-sections"
> >>>       gcc_cv_as_no_compress_debug_option="--nocompress-debug-sections"
> >>> -   # Before binutils 2.26, gas only supported --compress-debug-options and
> >>> -   # emitted the traditional GNU format.
> >>> -   elif $gcc_cv_as --compress-debug-sections -o conftest.o conftest.s > /dev/null 2>&1
> >>> -   then
> >>> -     gcc_cv_as_compress_debug=1
> >>> -     gcc_cv_as_compress_debug_option="--compress-debug-sections"
> >>> -     gcc_cv_as_no_compress_debug_option="--nocompress-debug-sections"
> >>>     else
> >>>       gcc_cv_as_compress_debug=0
> >>>     fi
> >>> @@ -30238,42 +30231,28 @@ $as_echo "$gcc_cv_ld_eh_gc_sections_bug" >&6; }
> >>>
> >>>  { $as_echo "$as_me:${as_lineno-$LINENO}: checking linker for compressed debug sections" >&5
> >>>  $as_echo_n "checking linker for compressed debug sections... " >&6; }
> >>> -# gold/gld support compressed debug sections since binutils 2.19/2.21
> >>> -# In binutils 2.26, gld gained support for the ELF gABI format.
> >>> +# GNU ld/gold support --compressed-debug-sections=zlib since binutils 2.26.
> >>>  if test $in_tree_ld = yes ; then
> >>>    gcc_cv_ld_compress_debug=0
> >>>    if test $ld_is_mold = yes; then
> >>>      gcc_cv_ld_compress_debug=3
> >>>      gcc_cv_ld_compress_debug_option="--compress-debug-sections"
> >>> -  elif test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 19 -o "$gcc_cv_gld_major_version" -gt 2 \
> >>> -     && test $in_tree_ld_is_elf = yes && test $ld_is_gold = yes; then
> >>> -    gcc_cv_ld_compress_debug=2
> >>> -    gcc_cv_ld_compress_debug_option="--compress-debug-sections"
> >>>    elif test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 26 -o "$gcc_cv_gld_major_version" -gt 2 \
> >>>       && test $in_tree_ld_is_elf = yes && test $ld_is_gold = no; then
> >>>      gcc_cv_ld_compress_debug=3
> >>>      gcc_cv_ld_compress_debug_option="--compress-debug-sections"
> >>> -  elif test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2 \
> >>> -     && test $in_tree_ld_is_elf = yes; then
> >>> -    gcc_cv_ld_compress_debug=1
> >>>    fi
> >>>  elif echo "$ld_ver" | grep GNU > /dev/null; then
> >>>    if test $ld_is_mold = yes; then
> >>>      gcc_cv_ld_compress_debug=3
> >>>      gcc_cv_ld_compress_debug_option="--compress-debug-sections"
> >>>    elif test "$ld_vers_major" -lt 2 \
> >>> -     || test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -lt 21; then
> >>> +     || test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -lt 26; then
> >>>      gcc_cv_ld_compress_debug=0
> >>> -  elif test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -lt 26; then
> >>> -    gcc_cv_ld_compress_debug=1
> >>>    else
> >>>      gcc_cv_ld_compress_debug=3
> >>>      gcc_cv_ld_compress_debug_option="--compress-debug-sections"
> >>>    fi
> >>> -  if test $ld_is_gold = yes; then
> >>> -    gcc_cv_ld_compress_debug=2
> >>> -    gcc_cv_ld_compress_debug_option="--compress-debug-sections"
> >>> -  fi
> >>>  else
> >>>    case "${target}" in
> >>>      *-*-solaris2*)
> >>> diff --git a/gcc/configure.ac b/gcc/configure.ac
> >>> index 446747311a6..f40f75271c2 100644
> >>> --- a/gcc/configure.ac
> >>> +++ b/gcc/configure.ac
> >>> @@ -5733,20 +5733,13 @@ gcc_GAS_CHECK_FEATURE([compressed debug sections],
> >>>     if $gcc_cv_as --compress-debug-sections -o conftest.o conftest.s 2>&1 | grep -i warning > /dev/null
> >>>     then
> >>>       gcc_cv_as_compress_debug=0
> >>> -   # Since binutils 2.26, gas supports --compress-debug-sections=type,
> >>> +   # Since binutils 2.26, gas supports --compress-debug-sections=zlib,
> >>>     # defaulting to the ELF gABI format.
> >>> -   elif $gcc_cv_as --compress-debug-sections=zlib-gnu -o conftest.o conftest.s > /dev/null 2>&1
> >>> +   elif $gcc_cv_as --compress-debug-sections=zlib -o conftest.o conftest.s > /dev/null 2>&1
> >>>     then
> >>>       gcc_cv_as_compress_debug=2
> >>>       gcc_cv_as_compress_debug_option="--compress-debug-sections"
> >>>       gcc_cv_as_no_compress_debug_option="--nocompress-debug-sections"
> >>> -   # Before binutils 2.26, gas only supported --compress-debug-options and
> >>> -   # emitted the traditional GNU format.
> >>> -   elif $gcc_cv_as --compress-debug-sections -o conftest.o conftest.s > /dev/null 2>&1
> >>> -   then
> >>> -     gcc_cv_as_compress_debug=1
> >>> -     gcc_cv_as_compress_debug_option="--compress-debug-sections"
> >>> -     gcc_cv_as_no_compress_debug_option="--nocompress-debug-sections"
> >>>     else
> >>>       gcc_cv_as_compress_debug=0
> >>>     fi])
> >>> @@ -6131,42 +6124,28 @@ fi
> >>>  AC_MSG_RESULT($gcc_cv_ld_eh_gc_sections_bug)
> >>>
> >>>  AC_MSG_CHECKING(linker for compressed debug sections)
> >>> -# gold/gld support compressed debug sections since binutils 2.19/2.21
> >>> -# In binutils 2.26, gld gained support for the ELF gABI format.
> >>> +# GNU ld/gold support --compressed-debug-sections=zlib since binutils 2.26.
> >>>  if test $in_tree_ld = yes ; then
> >>>    gcc_cv_ld_compress_debug=0
> >>>    if test $ld_is_mold = yes; then
> >>>      gcc_cv_ld_compress_debug=3
> >>>      gcc_cv_ld_compress_debug_option="--compress-debug-sections"
> >>> -  elif test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 19 -o "$gcc_cv_gld_major_version" -gt 2 \
> >>> -     && test $in_tree_ld_is_elf = yes && test $ld_is_gold = yes; then
> >>> -    gcc_cv_ld_compress_debug=2
> >>> -    gcc_cv_ld_compress_debug_option="--compress-debug-sections"
> >>>    elif test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 26 -o "$gcc_cv_gld_major_version" -gt 2 \
> >>>       && test $in_tree_ld_is_elf = yes && test $ld_is_gold = no; then
> >>>      gcc_cv_ld_compress_debug=3
> >>>      gcc_cv_ld_compress_debug_option="--compress-debug-sections"
> >>> -  elif test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2 \
> >>> -     && test $in_tree_ld_is_elf = yes; then
> >>> -    gcc_cv_ld_compress_debug=1
> >>>    fi
> >>>  elif echo "$ld_ver" | grep GNU > /dev/null; then
> >>>    if test $ld_is_mold = yes; then
> >>>      gcc_cv_ld_compress_debug=3
> >>>      gcc_cv_ld_compress_debug_option="--compress-debug-sections"
> >>>    elif test "$ld_vers_major" -lt 2 \
> >>> -     || test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -lt 21; then
> >>> +     || test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -lt 26; then
> >>>      gcc_cv_ld_compress_debug=0
> >>> -  elif test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -lt 26; then
> >>> -    gcc_cv_ld_compress_debug=1
> >>>    else
> >>>      gcc_cv_ld_compress_debug=3
> >>>      gcc_cv_ld_compress_debug_option="--compress-debug-sections"
> >>>    fi
> >>> -  if test $ld_is_gold = yes; then
> >>> -    gcc_cv_ld_compress_debug=2
> >>> -    gcc_cv_ld_compress_debug_option="--compress-debug-sections"
> >>> -  fi
> >>>  else
> >>>  changequote(,)dnl
> >>>    case "${target}" in
> >>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> >>> index 757775ea576..467659cb094 100644
> >>> --- a/gcc/doc/invoke.texi
> >>> +++ b/gcc/doc/invoke.texi
> >>> @@ -10775,12 +10775,11 @@ location views are enabled.
> >>>  Produce compressed debug sections in DWARF format, if that is supported.
> >>>  If @var{type} is not given, the default type depends on the capabilities
> >>>  of the assembler and linker used.  @var{type} may be one of
> >>> -@samp{none} (don't compress debug sections), @samp{zlib} (use zlib
> >>> -compression in ELF gABI format), or @samp{zlib-gnu} (use zlib
> >>> -compression in traditional GNU format).  If the linker doesn't support
> >>> -writing compressed debug sections, the option is rejected.  Otherwise,
> >>> -if the assembler does not support them, @option{-gz} is silently ignored
> >>> -when producing object files.
> >>> +@samp{none} (don't compress debug sections), or @samp{zlib} (use zlib
> >>> +compression in ELF gABI format).  If the linker doesn't support writing
> >>> +compressed debug sections, the option is rejected.  Otherwise, if the
> >>> +assembler does not support them, @option{-gz} is silently ignored when
> >>> +producing object files.
> >>>
> >>>  @item -femit-struct-debug-baseonly
> >>>  @opindex femit-struct-debug-baseonly
> >>> diff --git a/gcc/gcc.cc b/gcc/gcc.cc
> >>> index 5cbb38560b2..660cd6f4ef4 100644
> >>> --- a/gcc/gcc.cc
> >>> +++ b/gcc/gcc.cc
> >>> @@ -827,22 +827,11 @@ proper position among the other output files.  */
> >>>  /* No linker support.  */
> >>>  #define LINK_COMPRESS_DEBUG_SPEC \
> >>>         " %{gz*:%e-gz is not supported in this configuration} "
> >>> -#elif HAVE_LD_COMPRESS_DEBUG == 1
> >>> -/* GNU style on input, GNU ld options.  Reject, not useful.  */
> >>> -#define LINK_COMPRESS_DEBUG_SPEC \
> >>> -       " %{gz*:%e-gz is not supported in this configuration} "
> >>> -#elif HAVE_LD_COMPRESS_DEBUG == 2
> >>> -/* GNU style, GNU gold options.  */
> >>> -#define LINK_COMPRESS_DEBUG_SPEC \
> >>> -       " %{gz|gz=zlib-gnu:" LD_COMPRESS_DEBUG_OPTION "=zlib}" \
> >>> -       " %{gz=none:"        LD_COMPRESS_DEBUG_OPTION "=none}" \
> >>> -       " %{gz=zlib:%e-gz=zlib is not supported in this configuration} "
> >>>  #elif HAVE_LD_COMPRESS_DEBUG == 3
> >>>  /* ELF gABI style.  */
> >>>  #define LINK_COMPRESS_DEBUG_SPEC \
> >>>         " %{gz|gz=zlib:"  LD_COMPRESS_DEBUG_OPTION "=zlib}" \
> >>> -       " %{gz=none:"     LD_COMPRESS_DEBUG_OPTION "=none}" \
> >>> -       " %{gz=zlib-gnu:" LD_COMPRESS_DEBUG_OPTION "=zlib-gnu} "
> >>> +       " %{gz=none:"     LD_COMPRESS_DEBUG_OPTION "=none}"
> >>>  #else
> >>>  #error Unknown value for HAVE_LD_COMPRESS_DEBUG.
> >>>  #endif
> >>> @@ -891,18 +880,11 @@ proper position among the other output files.  */
> >>>  /* No assembler support.  Ignore silently.  */
> >>>  #define ASM_COMPRESS_DEBUG_SPEC \
> >>>         " %{gz*:} "
> >>> -#elif HAVE_AS_COMPRESS_DEBUG == 1
> >>> -/* GNU style, GNU as options.  */
> >>> -#define ASM_COMPRESS_DEBUG_SPEC \
> >>> -       " %{gz|gz=zlib-gnu:" AS_COMPRESS_DEBUG_OPTION "}" \
> >>> -       " %{gz=none:"        AS_NO_COMPRESS_DEBUG_OPTION "}" \
> >>> -       " %{gz=zlib:%e-gz=zlib is not supported in this configuration} "
> >>>  #elif HAVE_AS_COMPRESS_DEBUG == 2
> >>>  /* ELF gABI style.  */
> >>>  #define ASM_COMPRESS_DEBUG_SPEC \
> >>>         " %{gz|gz=zlib:"  AS_COMPRESS_DEBUG_OPTION "=zlib}" \
> >>> -       " %{gz=none:"     AS_COMPRESS_DEBUG_OPTION "=none}" \
> >>> -       " %{gz=zlib-gnu:" AS_COMPRESS_DEBUG_OPTION "=zlib-gnu} "
> >>> +       " %{gz=none:"     AS_COMPRESS_DEBUG_OPTION "=none}"
> >>>  #else
> >>>  #error Unknown value for HAVE_AS_COMPRESS_DEBUG.
> >>>  #endif
> >>> --
> >>> 2.37.0.rc0.161.g10f37bed90-goog
> >>>
>