public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] readelf: support zstd compressed debug sections [PR 29640]
@ 2022-10-01  6:20 Fangrui Song
  2022-10-06  2:20 ` Fangrui Song
  2022-10-13 11:38 ` Martin Liška
  0 siblings, 2 replies; 14+ messages in thread
From: Fangrui Song @ 2022-10-01  6:20 UTC (permalink / raw)
  To: binutils; +Cc: Martin Liska, Fangrui Song

---
 binutils/Makefile.am |   6 +--
 binutils/Makefile.in |   6 +--
 binutils/readelf.c   | 112 +++++++++++++++++++++++++++----------------
 3 files changed, 78 insertions(+), 46 deletions(-)

diff --git a/binutils/Makefile.am b/binutils/Makefile.am
index 751fbacce12..b249af721ae 100644
--- a/binutils/Makefile.am
+++ b/binutils/Makefile.am
@@ -54,8 +54,8 @@ DEBUGINFOD_LIBS = @DEBUGINFOD_LIBS@
 WARN_CFLAGS = @WARN_CFLAGS@
 WARN_CFLAGS_FOR_BUILD = @WARN_CFLAGS_FOR_BUILD@
 NO_WERROR = @NO_WERROR@
-AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC)
-AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC)
+AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC) $(ZSTD_CFLAGS)
+AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC) $(ZSTD_CFLAGS)
 LIBICONV = @LIBICONV@
 
 # these two are almost the same program
@@ -256,7 +256,7 @@ objcopy_SOURCES = objcopy.c not-strip.c rename.c $(WRITE_DEBUG_SRCS) $(BULIBS)
 strings_SOURCES = strings.c $(BULIBS)
 
 readelf_SOURCES = readelf.c version.c unwind-ia64.c dwarf.c demanguse.c $(ELFLIBS)
-readelf_LDADD   = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
+readelf_LDADD   = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(ZSTD_LIBS) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
 
 elfedit_SOURCES = elfedit.c version.c $(ELFLIBS)
 elfedit_LDADD = $(LIBINTL) $(LIBIBERTY)
diff --git a/binutils/Makefile.in b/binutils/Makefile.in
index 6de4e239408..7d9039e0f2f 100644
--- a/binutils/Makefile.in
+++ b/binutils/Makefile.in
@@ -651,8 +651,8 @@ am__skipyacc =
 # case both are empty.
 ZLIB = @zlibdir@ -lz
 ZLIBINC = @zlibinc@
-AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC)
-AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC)
+AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC) $(ZSTD_CFLAGS)
+AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC) $(ZSTD_CFLAGS)
 
 # these two are almost the same program
 AR_PROG = ar
@@ -790,7 +790,7 @@ size_SOURCES = size.c $(BULIBS)
 objcopy_SOURCES = objcopy.c not-strip.c rename.c $(WRITE_DEBUG_SRCS) $(BULIBS)
 strings_SOURCES = strings.c $(BULIBS)
 readelf_SOURCES = readelf.c version.c unwind-ia64.c dwarf.c demanguse.c $(ELFLIBS)
-readelf_LDADD = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
+readelf_LDADD = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(ZSTD_LIBS) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
 elfedit_SOURCES = elfedit.c version.c $(ELFLIBS)
 elfedit_LDADD = $(LIBINTL) $(LIBIBERTY)
 strip_new_SOURCES = objcopy.c is-strip.c rename.c $(WRITE_DEBUG_SRCS) $(BULIBS)
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 351571c8abb..04cda213f33 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -44,6 +44,9 @@
 #include <assert.h>
 #include <time.h>
 #include <zlib.h>
+#ifdef HAVE_ZSTD
+#include <zstd.h>
+#endif
 #include <wchar.h>
 
 #if defined HAVE_MSGPACK
@@ -15170,48 +15173,56 @@ get_section_contents (Elf_Internal_Shdr * section, Filedata * filedata)
                              _("section contents"));
 }
 
-/* Uncompresses a section that was compressed using zlib, in place.  */
+/* Uncompresses a section that was compressed using zlib/zstd, in place.  */
 
 static bool
-uncompress_section_contents (unsigned char **buffer,
-			     uint64_t uncompressed_size,
-			     uint64_t *size)
+uncompress_section_contents (bool is_zstd, unsigned char **buffer,
+			     uint64_t uncompressed_size, uint64_t *size)
 {
   uint64_t compressed_size = *size;
   unsigned char *compressed_buffer = *buffer;
-  unsigned char *uncompressed_buffer;
+  unsigned char *uncompressed_buffer = xmalloc (uncompressed_size);
   z_stream strm;
   int rc;
 
-  /* It is possible the section consists of several compressed
-     buffers concatenated together, so we uncompress in a loop.  */
-  /* PR 18313: The state field in the z_stream structure is supposed
-     to be invisible to the user (ie us), but some compilers will
-     still complain about it being used without initialisation.  So
-     we first zero the entire z_stream structure and then set the fields
-     that we need.  */
-  memset (& strm, 0, sizeof strm);
-  strm.avail_in = compressed_size;
-  strm.next_in = (Bytef *) compressed_buffer;
-  strm.avail_out = uncompressed_size;
-  uncompressed_buffer = (unsigned char *) xmalloc (uncompressed_size);
+  if (is_zstd)
+    {
+#ifdef HAVE_ZSTD
+      size_t ret = ZSTD_decompress (uncompressed_buffer, uncompressed_size,
+				    compressed_buffer, compressed_size);
+      if (ZSTD_isError (ret))
+	goto fail;
+#endif
+    }
+  else
+    {
+      /* It is possible the section consists of several compressed
+	 buffers concatenated together, so we uncompress in a loop.  */
+      /* PR 18313: The state field in the z_stream structure is supposed
+	 to be invisible to the user (ie us), but some compilers will
+	 still complain about it being used without initialisation.  So
+	 we first zero the entire z_stream structure and then set the fields
+	 that we need.  */
+      memset (&strm, 0, sizeof strm);
+      strm.avail_in = compressed_size;
+      strm.next_in = (Bytef *)compressed_buffer;
+      strm.avail_out = uncompressed_size;
 
-  rc = inflateInit (& strm);
-  while (strm.avail_in > 0)
-    {
-      if (rc != Z_OK)
-        break;
-      strm.next_out = ((Bytef *) uncompressed_buffer
-                       + (uncompressed_size - strm.avail_out));
-      rc = inflate (&strm, Z_FINISH);
-      if (rc != Z_STREAM_END)
-        break;
-      rc = inflateReset (& strm);
+      rc = inflateInit (&strm);
+      while (strm.avail_in > 0)
+	{
+	  if (rc != Z_OK)
+	    break;
+	  strm.next_out = ((Bytef *)uncompressed_buffer
+			   + (uncompressed_size - strm.avail_out));
+	  rc = inflate (&strm, Z_FINISH);
+	  if (rc != Z_STREAM_END)
+	    break;
+	  rc = inflateReset (&strm);
+	}
+      if (inflateEnd (&strm) != Z_OK || rc != Z_OK || strm.avail_out != 0)
+	goto fail;
     }
-  if (inflateEnd (& strm) != Z_OK
-      || rc != Z_OK
-      || strm.avail_out != 0)
-    goto fail;
 
   *buffer = uncompressed_buffer;
   *size = uncompressed_size;
@@ -15254,6 +15265,7 @@ dump_section_as_strings (Elf_Internal_Shdr * section, Filedata * filedata)
     {
       uint64_t new_size = num_bytes;
       uint64_t uncompressed_size = 0;
+      bool is_zstd = false;
 
       if ((section->sh_flags & SHF_COMPRESSED) != 0)
 	{
@@ -15266,7 +15278,13 @@ dump_section_as_strings (Elf_Internal_Shdr * section, Filedata * filedata)
 	       by get_compression_header.  */
 	    goto error_out;
 
-	  if (chdr.ch_type != ELFCOMPRESS_ZLIB)
+	  if (chdr.ch_type == ELFCOMPRESS_ZLIB)
+	    ;
+#ifdef HAVE_ZSTD
+	  else if (chdr.ch_type == ELFCOMPRESS_ZSTD)
+	    is_zstd = true;
+#endif
+	  else
 	    {
 	      warn (_("section '%s' has unsupported compress type: %d\n"),
 		    printable_section_name (filedata, section), chdr.ch_type);
@@ -15295,8 +15313,8 @@ dump_section_as_strings (Elf_Internal_Shdr * section, Filedata * filedata)
 
       if (uncompressed_size)
 	{
-	  if (uncompress_section_contents (& start,
-					   uncompressed_size, & new_size))
+	  if (uncompress_section_contents (is_zstd, &start, uncompressed_size,
+					   &new_size))
 	    num_bytes = new_size;
 	  else
 	    {
@@ -15470,6 +15488,7 @@ dump_section_as_bytes (Elf_Internal_Shdr *section,
     {
       uint64_t new_size = section_size;
       uint64_t uncompressed_size = 0;
+      bool is_zstd = false;
 
       if ((section->sh_flags & SHF_COMPRESSED) != 0)
 	{
@@ -15482,7 +15501,13 @@ dump_section_as_bytes (Elf_Internal_Shdr *section,
 	       by get_compression_header.  */
 	    goto error_out;
 
-	  if (chdr.ch_type != ELFCOMPRESS_ZLIB)
+	  if (chdr.ch_type == ELFCOMPRESS_ZLIB)
+	    ;
+#ifdef HAVE_ZSTD
+	  else if (chdr.ch_type == ELFCOMPRESS_ZSTD)
+	    is_zstd = true;
+#endif
+	  else
 	    {
 	      warn (_("section '%s' has unsupported compress type: %d\n"),
 		    printable_section_name (filedata, section), chdr.ch_type);
@@ -15511,8 +15536,8 @@ dump_section_as_bytes (Elf_Internal_Shdr *section,
 
       if (uncompressed_size)
 	{
-	  if (uncompress_section_contents (& start, uncompressed_size,
-					   & new_size))
+	  if (uncompress_section_contents (is_zstd, &start, uncompressed_size,
+					   &new_size))
 	    {
 	      section_size = new_size;
 	    }
@@ -15848,6 +15873,7 @@ load_specific_debug_section (enum dwarf_section_display_enum  debug,
       unsigned char *start = section->start;
       uint64_t size = sec->sh_size;
       uint64_t uncompressed_size = 0;
+      bool is_zstd = false;
 
       if ((sec->sh_flags & SHF_COMPRESSED) != 0)
 	{
@@ -15869,7 +15895,13 @@ load_specific_debug_section (enum dwarf_section_display_enum  debug,
 	       by get_compression_header.  */
 	    return false;
 
-	  if (chdr.ch_type != ELFCOMPRESS_ZLIB)
+	  if (chdr.ch_type == ELFCOMPRESS_ZLIB)
+	    ;
+#ifdef HAVE_ZSTD
+	  else if (chdr.ch_type == ELFCOMPRESS_ZSTD)
+	    is_zstd = true;
+#endif
+	  else
 	    {
 	      warn (_("section '%s' has unsupported compress type: %d\n"),
 		    section->name, chdr.ch_type);
@@ -15898,7 +15930,7 @@ load_specific_debug_section (enum dwarf_section_display_enum  debug,
 
       if (uncompressed_size)
 	{
-	  if (uncompress_section_contents (&start, uncompressed_size,
+	  if (uncompress_section_contents (is_zstd, &start, uncompressed_size,
 					   &size))
 	    {
 	      /* Free the compressed buffer, update the section buffer
-- 
2.38.0.rc1.362.ged0d419d3c-goog


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] readelf: support zstd compressed debug sections [PR 29640]
  2022-10-01  6:20 [PATCH] readelf: support zstd compressed debug sections [PR 29640] Fangrui Song
@ 2022-10-06  2:20 ` Fangrui Song
  2022-10-20  8:04   ` Martin Liška
  2022-10-20  8:05   ` Martin Liška
  2022-10-13 11:38 ` Martin Liška
  1 sibling, 2 replies; 14+ messages in thread
From: Fangrui Song @ 2022-10-06  2:20 UTC (permalink / raw)
  To: binutils

On 2022-09-30, Fangrui Song wrote:
>---
> binutils/Makefile.am |   6 +--
> binutils/Makefile.in |   6 +--
> binutils/readelf.c   | 112 +++++++++++++++++++++++++++----------------
> 3 files changed, 78 insertions(+), 46 deletions(-)

ping:)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] readelf: support zstd compressed debug sections [PR 29640]
  2022-10-01  6:20 [PATCH] readelf: support zstd compressed debug sections [PR 29640] Fangrui Song
  2022-10-06  2:20 ` Fangrui Song
@ 2022-10-13 11:38 ` Martin Liška
  2022-10-14  3:35   ` Fangrui Song
  1 sibling, 1 reply; 14+ messages in thread
From: Martin Liška @ 2022-10-13 11:38 UTC (permalink / raw)
  To: Fangrui Song, binutils; +Cc: Alan Modra

On 10/1/22 08:20, Fangrui Song wrote:
> ---
>  binutils/Makefile.am |   6 +--
>  binutils/Makefile.in |   6 +--
>  binutils/readelf.c   | 112 +++++++++++++++++++++++++++----------------
>  3 files changed, 78 insertions(+), 46 deletions(-)
> 
> diff --git a/binutils/Makefile.am b/binutils/Makefile.am
> index 751fbacce12..b249af721ae 100644
> --- a/binutils/Makefile.am
> +++ b/binutils/Makefile.am
> @@ -54,8 +54,8 @@ DEBUGINFOD_LIBS = @DEBUGINFOD_LIBS@
>  WARN_CFLAGS = @WARN_CFLAGS@
>  WARN_CFLAGS_FOR_BUILD = @WARN_CFLAGS_FOR_BUILD@
>  NO_WERROR = @NO_WERROR@
> -AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC)
> -AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC)
> +AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC) $(ZSTD_CFLAGS)
> +AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC) $(ZSTD_CFLAGS)
>  LIBICONV = @LIBICONV@
>  
>  # these two are almost the same program
> @@ -256,7 +256,7 @@ objcopy_SOURCES = objcopy.c not-strip.c rename.c $(WRITE_DEBUG_SRCS) $(BULIBS)
>  strings_SOURCES = strings.c $(BULIBS)
>  
>  readelf_SOURCES = readelf.c version.c unwind-ia64.c dwarf.c demanguse.c $(ELFLIBS)
> -readelf_LDADD   = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
> +readelf_LDADD   = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(ZSTD_LIBS) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
>  
>  elfedit_SOURCES = elfedit.c version.c $(ELFLIBS)
>  elfedit_LDADD = $(LIBINTL) $(LIBIBERTY)
> diff --git a/binutils/Makefile.in b/binutils/Makefile.in
> index 6de4e239408..7d9039e0f2f 100644
> --- a/binutils/Makefile.in
> +++ b/binutils/Makefile.in
> @@ -651,8 +651,8 @@ am__skipyacc =
>  # case both are empty.
>  ZLIB = @zlibdir@ -lz
>  ZLIBINC = @zlibinc@
> -AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC)
> -AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC)
> +AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC) $(ZSTD_CFLAGS)
> +AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC) $(ZSTD_CFLAGS)
>  
>  # these two are almost the same program
>  AR_PROG = ar
> @@ -790,7 +790,7 @@ size_SOURCES = size.c $(BULIBS)
>  objcopy_SOURCES = objcopy.c not-strip.c rename.c $(WRITE_DEBUG_SRCS) $(BULIBS)
>  strings_SOURCES = strings.c $(BULIBS)
>  readelf_SOURCES = readelf.c version.c unwind-ia64.c dwarf.c demanguse.c $(ELFLIBS)
> -readelf_LDADD = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
> +readelf_LDADD = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(ZSTD_LIBS) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
>  elfedit_SOURCES = elfedit.c version.c $(ELFLIBS)
>  elfedit_LDADD = $(LIBINTL) $(LIBIBERTY)
>  strip_new_SOURCES = objcopy.c is-strip.c rename.c $(WRITE_DEBUG_SRCS) $(BULIBS)
> diff --git a/binutils/readelf.c b/binutils/readelf.c
> index 351571c8abb..04cda213f33 100644
> --- a/binutils/readelf.c
> +++ b/binutils/readelf.c
> @@ -44,6 +44,9 @@
>  #include <assert.h>
>  #include <time.h>
>  #include <zlib.h>
> +#ifdef HAVE_ZSTD
> +#include <zstd.h>
> +#endif
>  #include <wchar.h>
>  
>  #if defined HAVE_MSGPACK
> @@ -15170,48 +15173,56 @@ get_section_contents (Elf_Internal_Shdr * section, Filedata * filedata)
>                               _("section contents"));
>  }
>  
> -/* Uncompresses a section that was compressed using zlib, in place.  */
> +/* Uncompresses a section that was compressed using zlib/zstd, in place.  */
>  
>  static bool
> -uncompress_section_contents (unsigned char **buffer,
> -			     uint64_t uncompressed_size,
> -			     uint64_t *size)
> +uncompress_section_contents (bool is_zstd, unsigned char **buffer,
> +			     uint64_t uncompressed_size, uint64_t *size)
>  {
>    uint64_t compressed_size = *size;
>    unsigned char *compressed_buffer = *buffer;
> -  unsigned char *uncompressed_buffer;
> +  unsigned char *uncompressed_buffer = xmalloc (uncompressed_size);
>    z_stream strm;
>    int rc;
>  
> -  /* It is possible the section consists of several compressed
> -     buffers concatenated together, so we uncompress in a loop.  */
> -  /* PR 18313: The state field in the z_stream structure is supposed
> -     to be invisible to the user (ie us), but some compilers will
> -     still complain about it being used without initialisation.  So
> -     we first zero the entire z_stream structure and then set the fields
> -     that we need.  */
> -  memset (& strm, 0, sizeof strm);
> -  strm.avail_in = compressed_size;
> -  strm.next_in = (Bytef *) compressed_buffer;
> -  strm.avail_out = uncompressed_size;
> -  uncompressed_buffer = (unsigned char *) xmalloc (uncompressed_size);
> +  if (is_zstd)
> +    {
> +#ifdef HAVE_ZSTD
> +      size_t ret = ZSTD_decompress (uncompressed_buffer, uncompressed_size,
> +				    compressed_buffer, compressed_size);
> +      if (ZSTD_isError (ret))
> +	goto fail;
> +#endif
> +    }
> +  else
> +    {
> +      /* It is possible the section consists of several compressed
> +	 buffers concatenated together, so we uncompress in a loop.  */
> +      /* PR 18313: The state field in the z_stream structure is supposed
> +	 to be invisible to the user (ie us), but some compilers will
> +	 still complain about it being used without initialisation.  So
> +	 we first zero the entire z_stream structure and then set the fields
> +	 that we need.  */
> +      memset (&strm, 0, sizeof strm);
> +      strm.avail_in = compressed_size;
> +      strm.next_in = (Bytef *)compressed_buffer;
> +      strm.avail_out = uncompressed_size;
>  
> -  rc = inflateInit (& strm);
> -  while (strm.avail_in > 0)
> -    {
> -      if (rc != Z_OK)
> -        break;
> -      strm.next_out = ((Bytef *) uncompressed_buffer
> -                       + (uncompressed_size - strm.avail_out));
> -      rc = inflate (&strm, Z_FINISH);
> -      if (rc != Z_STREAM_END)
> -        break;
> -      rc = inflateReset (& strm);
> +      rc = inflateInit (&strm);
> +      while (strm.avail_in > 0)
> +	{
> +	  if (rc != Z_OK)
> +	    break;
> +	  strm.next_out = ((Bytef *)uncompressed_buffer
> +			   + (uncompressed_size - strm.avail_out));
> +	  rc = inflate (&strm, Z_FINISH);
> +	  if (rc != Z_STREAM_END)
> +	    break;
> +	  rc = inflateReset (&strm);
> +	}
> +      if (inflateEnd (&strm) != Z_OK || rc != Z_OK || strm.avail_out != 0)
> +	goto fail;
>      }
> -  if (inflateEnd (& strm) != Z_OK
> -      || rc != Z_OK
> -      || strm.avail_out != 0)
> -    goto fail;
>  
>    *buffer = uncompressed_buffer;
>    *size = uncompressed_size;
> @@ -15254,6 +15265,7 @@ dump_section_as_strings (Elf_Internal_Shdr * section, Filedata * filedata)
>      {
>        uint64_t new_size = num_bytes;
>        uint64_t uncompressed_size = 0;
> +      bool is_zstd = false;
>  
>        if ((section->sh_flags & SHF_COMPRESSED) != 0)
>  	{
> @@ -15266,7 +15278,13 @@ dump_section_as_strings (Elf_Internal_Shdr * section, Filedata * filedata)
>  	       by get_compression_header.  */
>  	    goto error_out;
>  
> -	  if (chdr.ch_type != ELFCOMPRESS_ZLIB)
> +	  if (chdr.ch_type == ELFCOMPRESS_ZLIB)
> +	    ;
> +#ifdef HAVE_ZSTD
> +	  else if (chdr.ch_type == ELFCOMPRESS_ZSTD)
> +	    is_zstd = true;
> +#endif
> +	  else
>  	    {
>  	      warn (_("section '%s' has unsupported compress type: %d\n"),
>  		    printable_section_name (filedata, section), chdr.ch_type);
> @@ -15295,8 +15313,8 @@ dump_section_as_strings (Elf_Internal_Shdr * section, Filedata * filedata)
>  
>        if (uncompressed_size)
>  	{
> -	  if (uncompress_section_contents (& start,
> -					   uncompressed_size, & new_size))
> +	  if (uncompress_section_contents (is_zstd, &start, uncompressed_size,
> +					   &new_size))
>  	    num_bytes = new_size;
>  	  else
>  	    {
> @@ -15470,6 +15488,7 @@ dump_section_as_bytes (Elf_Internal_Shdr *section,
>      {
>        uint64_t new_size = section_size;
>        uint64_t uncompressed_size = 0;
> +      bool is_zstd = false;
>  
>        if ((section->sh_flags & SHF_COMPRESSED) != 0)
>  	{
> @@ -15482,7 +15501,13 @@ dump_section_as_bytes (Elf_Internal_Shdr *section,
>  	       by get_compression_header.  */
>  	    goto error_out;
>  
> -	  if (chdr.ch_type != ELFCOMPRESS_ZLIB)
> +	  if (chdr.ch_type == ELFCOMPRESS_ZLIB)
> +	    ;
> +#ifdef HAVE_ZSTD
> +	  else if (chdr.ch_type == ELFCOMPRESS_ZSTD)
> +	    is_zstd = true;
> +#endif
> +	  else
>  	    {
>  	      warn (_("section '%s' has unsupported compress type: %d\n"),
>  		    printable_section_name (filedata, section), chdr.ch_type);
> @@ -15511,8 +15536,8 @@ dump_section_as_bytes (Elf_Internal_Shdr *section,
>  
>        if (uncompressed_size)
>  	{
> -	  if (uncompress_section_contents (& start, uncompressed_size,
> -					   & new_size))
> +	  if (uncompress_section_contents (is_zstd, &start, uncompressed_size,
> +					   &new_size))
>  	    {
>  	      section_size = new_size;
>  	    }
> @@ -15848,6 +15873,7 @@ load_specific_debug_section (enum dwarf_section_display_enum  debug,
>        unsigned char *start = section->start;
>        uint64_t size = sec->sh_size;
>        uint64_t uncompressed_size = 0;
> +      bool is_zstd = false;
>  
>        if ((sec->sh_flags & SHF_COMPRESSED) != 0)
>  	{
> @@ -15869,7 +15895,13 @@ load_specific_debug_section (enum dwarf_section_display_enum  debug,
>  	       by get_compression_header.  */
>  	    return false;
>  
> -	  if (chdr.ch_type != ELFCOMPRESS_ZLIB)
> +	  if (chdr.ch_type == ELFCOMPRESS_ZLIB)
> +	    ;
> +#ifdef HAVE_ZSTD
> +	  else if (chdr.ch_type == ELFCOMPRESS_ZSTD)
> +	    is_zstd = true;
> +#endif
> +	  else
>  	    {
>  	      warn (_("section '%s' has unsupported compress type: %d\n"),
>  		    section->name, chdr.ch_type);
> @@ -15898,7 +15930,7 @@ load_specific_debug_section (enum dwarf_section_display_enum  debug,
>  
>        if (uncompressed_size)
>  	{
> -	  if (uncompress_section_contents (&start, uncompressed_size,
> +	  if (uncompress_section_contents (is_zstd, &start, uncompressed_size,
>  					   &size))
>  	    {
>  	      /* Free the compressed buffer, update the section buffer

Hi.

I noticed the following issue (dunno if caused by objcopy or the patches readelf) with this patch:

$ echo 'int main() {return 0;}' | gcc -O2 -g -c -o a.o -x c -
$ /home/marxin/Programming/binutils/objdir/ld/../binutils/objcopy -O elf32-x86-64 a.o a-x32.o --compress-debug-sections=zstd
$ /home/marxin/Programming/binutils/objdir/binutils/readelf -wi a-x32.o >/dev/null
readelf: Error: Unable to decompress section .debug_info
readelf: Error: No comp units in .debug_info section ?

while zlib is fine:

$ /home/marxin/Programming/binutils/objdir/ld/../binutils/objcopy -O elf32-x86-64 a.o a-x32.o --compress-debug-sections=zlib
$ /home/marxin/Programming/binutils/objdir/binutils/readelf -wi a-x32.o >/dev/null

Can you please take a look?

Cheers,
Martin

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] readelf: support zstd compressed debug sections [PR 29640]
  2022-10-13 11:38 ` Martin Liška
@ 2022-10-14  3:35   ` Fangrui Song
  2022-10-14  7:47     ` Martin Liška
  0 siblings, 1 reply; 14+ messages in thread
From: Fangrui Song @ 2022-10-14  3:35 UTC (permalink / raw)
  To: Martin Liška; +Cc: binutils, Alan Modra

On 2022-10-13, Martin Liška wrote:
>On 10/1/22 08:20, Fangrui Song wrote:
>> ---
>>  binutils/Makefile.am |   6 +--
>>  binutils/Makefile.in |   6 +--
>>  binutils/readelf.c   | 112 +++++++++++++++++++++++++++----------------
>>  3 files changed, 78 insertions(+), 46 deletions(-)
>>
>> diff --git a/binutils/Makefile.am b/binutils/Makefile.am
>> index 751fbacce12..b249af721ae 100644
>> --- a/binutils/Makefile.am
>> +++ b/binutils/Makefile.am
>> @@ -54,8 +54,8 @@ DEBUGINFOD_LIBS = @DEBUGINFOD_LIBS@
>>  WARN_CFLAGS = @WARN_CFLAGS@
>>  WARN_CFLAGS_FOR_BUILD = @WARN_CFLAGS_FOR_BUILD@
>>  NO_WERROR = @NO_WERROR@
>> -AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC)
>> -AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC)
>> +AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC) $(ZSTD_CFLAGS)
>> +AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC) $(ZSTD_CFLAGS)
>>  LIBICONV = @LIBICONV@
>>
>>  # these two are almost the same program
>> @@ -256,7 +256,7 @@ objcopy_SOURCES = objcopy.c not-strip.c rename.c $(WRITE_DEBUG_SRCS) $(BULIBS)
>>  strings_SOURCES = strings.c $(BULIBS)
>>
>>  readelf_SOURCES = readelf.c version.c unwind-ia64.c dwarf.c demanguse.c $(ELFLIBS)
>> -readelf_LDADD   = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
>> +readelf_LDADD   = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(ZSTD_LIBS) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
>>
>>  elfedit_SOURCES = elfedit.c version.c $(ELFLIBS)
>>  elfedit_LDADD = $(LIBINTL) $(LIBIBERTY)
>> diff --git a/binutils/Makefile.in b/binutils/Makefile.in
>> index 6de4e239408..7d9039e0f2f 100644
>> --- a/binutils/Makefile.in
>> +++ b/binutils/Makefile.in
>> @@ -651,8 +651,8 @@ am__skipyacc =
>>  # case both are empty.
>>  ZLIB = @zlibdir@ -lz
>>  ZLIBINC = @zlibinc@
>> -AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC)
>> -AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC)
>> +AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC) $(ZSTD_CFLAGS)
>> +AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC) $(ZSTD_CFLAGS)
>>
>>  # these two are almost the same program
>>  AR_PROG = ar
>> @@ -790,7 +790,7 @@ size_SOURCES = size.c $(BULIBS)
>>  objcopy_SOURCES = objcopy.c not-strip.c rename.c $(WRITE_DEBUG_SRCS) $(BULIBS)
>>  strings_SOURCES = strings.c $(BULIBS)
>>  readelf_SOURCES = readelf.c version.c unwind-ia64.c dwarf.c demanguse.c $(ELFLIBS)
>> -readelf_LDADD = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
>> +readelf_LDADD = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(ZSTD_LIBS) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
>>  elfedit_SOURCES = elfedit.c version.c $(ELFLIBS)
>>  elfedit_LDADD = $(LIBINTL) $(LIBIBERTY)
>>  strip_new_SOURCES = objcopy.c is-strip.c rename.c $(WRITE_DEBUG_SRCS) $(BULIBS)
>> diff --git a/binutils/readelf.c b/binutils/readelf.c
>> index 351571c8abb..04cda213f33 100644
>> --- a/binutils/readelf.c
>> +++ b/binutils/readelf.c
>> @@ -44,6 +44,9 @@
>>  #include <assert.h>
>>  #include <time.h>
>>  #include <zlib.h>
>> +#ifdef HAVE_ZSTD
>> +#include <zstd.h>
>> +#endif
>>  #include <wchar.h>
>>
>>  #if defined HAVE_MSGPACK
>> @@ -15170,48 +15173,56 @@ get_section_contents (Elf_Internal_Shdr * section, Filedata * filedata)
>>                               _("section contents"));
>>  }
>>
>> -/* Uncompresses a section that was compressed using zlib, in place.  */
>> +/* Uncompresses a section that was compressed using zlib/zstd, in place.  */
>>
>>  static bool
>> -uncompress_section_contents (unsigned char **buffer,
>> -			     uint64_t uncompressed_size,
>> -			     uint64_t *size)
>> +uncompress_section_contents (bool is_zstd, unsigned char **buffer,
>> +			     uint64_t uncompressed_size, uint64_t *size)
>>  {
>>    uint64_t compressed_size = *size;
>>    unsigned char *compressed_buffer = *buffer;
>> -  unsigned char *uncompressed_buffer;
>> +  unsigned char *uncompressed_buffer = xmalloc (uncompressed_size);
>>    z_stream strm;
>>    int rc;
>>
>> -  /* It is possible the section consists of several compressed
>> -     buffers concatenated together, so we uncompress in a loop.  */
>> -  /* PR 18313: The state field in the z_stream structure is supposed
>> -     to be invisible to the user (ie us), but some compilers will
>> -     still complain about it being used without initialisation.  So
>> -     we first zero the entire z_stream structure and then set the fields
>> -     that we need.  */
>> -  memset (& strm, 0, sizeof strm);
>> -  strm.avail_in = compressed_size;
>> -  strm.next_in = (Bytef *) compressed_buffer;
>> -  strm.avail_out = uncompressed_size;
>> -  uncompressed_buffer = (unsigned char *) xmalloc (uncompressed_size);
>> +  if (is_zstd)
>> +    {
>> +#ifdef HAVE_ZSTD
>> +      size_t ret = ZSTD_decompress (uncompressed_buffer, uncompressed_size,
>> +				    compressed_buffer, compressed_size);
>> +      if (ZSTD_isError (ret))
>> +	goto fail;
>> +#endif
>> +    }
>> +  else
>> +    {
>> +      /* It is possible the section consists of several compressed
>> +	 buffers concatenated together, so we uncompress in a loop.  */
>> +      /* PR 18313: The state field in the z_stream structure is supposed
>> +	 to be invisible to the user (ie us), but some compilers will
>> +	 still complain about it being used without initialisation.  So
>> +	 we first zero the entire z_stream structure and then set the fields
>> +	 that we need.  */
>> +      memset (&strm, 0, sizeof strm);
>> +      strm.avail_in = compressed_size;
>> +      strm.next_in = (Bytef *)compressed_buffer;
>> +      strm.avail_out = uncompressed_size;
>>
>> -  rc = inflateInit (& strm);
>> -  while (strm.avail_in > 0)
>> -    {
>> -      if (rc != Z_OK)
>> -        break;
>> -      strm.next_out = ((Bytef *) uncompressed_buffer
>> -                       + (uncompressed_size - strm.avail_out));
>> -      rc = inflate (&strm, Z_FINISH);
>> -      if (rc != Z_STREAM_END)
>> -        break;
>> -      rc = inflateReset (& strm);
>> +      rc = inflateInit (&strm);
>> +      while (strm.avail_in > 0)
>> +	{
>> +	  if (rc != Z_OK)
>> +	    break;
>> +	  strm.next_out = ((Bytef *)uncompressed_buffer
>> +			   + (uncompressed_size - strm.avail_out));
>> +	  rc = inflate (&strm, Z_FINISH);
>> +	  if (rc != Z_STREAM_END)
>> +	    break;
>> +	  rc = inflateReset (&strm);
>> +	}
>> +      if (inflateEnd (&strm) != Z_OK || rc != Z_OK || strm.avail_out != 0)
>> +	goto fail;
>>      }
>> -  if (inflateEnd (& strm) != Z_OK
>> -      || rc != Z_OK
>> -      || strm.avail_out != 0)
>> -    goto fail;
>>
>>    *buffer = uncompressed_buffer;
>>    *size = uncompressed_size;
>> @@ -15254,6 +15265,7 @@ dump_section_as_strings (Elf_Internal_Shdr * section, Filedata * filedata)
>>      {
>>        uint64_t new_size = num_bytes;
>>        uint64_t uncompressed_size = 0;
>> +      bool is_zstd = false;
>>
>>        if ((section->sh_flags & SHF_COMPRESSED) != 0)
>>  	{
>> @@ -15266,7 +15278,13 @@ dump_section_as_strings (Elf_Internal_Shdr * section, Filedata * filedata)
>>  	       by get_compression_header.  */
>>  	    goto error_out;
>>
>> -	  if (chdr.ch_type != ELFCOMPRESS_ZLIB)
>> +	  if (chdr.ch_type == ELFCOMPRESS_ZLIB)
>> +	    ;
>> +#ifdef HAVE_ZSTD
>> +	  else if (chdr.ch_type == ELFCOMPRESS_ZSTD)
>> +	    is_zstd = true;
>> +#endif
>> +	  else
>>  	    {
>>  	      warn (_("section '%s' has unsupported compress type: %d\n"),
>>  		    printable_section_name (filedata, section), chdr.ch_type);
>> @@ -15295,8 +15313,8 @@ dump_section_as_strings (Elf_Internal_Shdr * section, Filedata * filedata)
>>
>>        if (uncompressed_size)
>>  	{
>> -	  if (uncompress_section_contents (& start,
>> -					   uncompressed_size, & new_size))
>> +	  if (uncompress_section_contents (is_zstd, &start, uncompressed_size,
>> +					   &new_size))
>>  	    num_bytes = new_size;
>>  	  else
>>  	    {
>> @@ -15470,6 +15488,7 @@ dump_section_as_bytes (Elf_Internal_Shdr *section,
>>      {
>>        uint64_t new_size = section_size;
>>        uint64_t uncompressed_size = 0;
>> +      bool is_zstd = false;
>>
>>        if ((section->sh_flags & SHF_COMPRESSED) != 0)
>>  	{
>> @@ -15482,7 +15501,13 @@ dump_section_as_bytes (Elf_Internal_Shdr *section,
>>  	       by get_compression_header.  */
>>  	    goto error_out;
>>
>> -	  if (chdr.ch_type != ELFCOMPRESS_ZLIB)
>> +	  if (chdr.ch_type == ELFCOMPRESS_ZLIB)
>> +	    ;
>> +#ifdef HAVE_ZSTD
>> +	  else if (chdr.ch_type == ELFCOMPRESS_ZSTD)
>> +	    is_zstd = true;
>> +#endif
>> +	  else
>>  	    {
>>  	      warn (_("section '%s' has unsupported compress type: %d\n"),
>>  		    printable_section_name (filedata, section), chdr.ch_type);
>> @@ -15511,8 +15536,8 @@ dump_section_as_bytes (Elf_Internal_Shdr *section,
>>
>>        if (uncompressed_size)
>>  	{
>> -	  if (uncompress_section_contents (& start, uncompressed_size,
>> -					   & new_size))
>> +	  if (uncompress_section_contents (is_zstd, &start, uncompressed_size,
>> +					   &new_size))
>>  	    {
>>  	      section_size = new_size;
>>  	    }
>> @@ -15848,6 +15873,7 @@ load_specific_debug_section (enum dwarf_section_display_enum  debug,
>>        unsigned char *start = section->start;
>>        uint64_t size = sec->sh_size;
>>        uint64_t uncompressed_size = 0;
>> +      bool is_zstd = false;
>>
>>        if ((sec->sh_flags & SHF_COMPRESSED) != 0)
>>  	{
>> @@ -15869,7 +15895,13 @@ load_specific_debug_section (enum dwarf_section_display_enum  debug,
>>  	       by get_compression_header.  */
>>  	    return false;
>>
>> -	  if (chdr.ch_type != ELFCOMPRESS_ZLIB)
>> +	  if (chdr.ch_type == ELFCOMPRESS_ZLIB)
>> +	    ;
>> +#ifdef HAVE_ZSTD
>> +	  else if (chdr.ch_type == ELFCOMPRESS_ZSTD)
>> +	    is_zstd = true;
>> +#endif
>> +	  else
>>  	    {
>>  	      warn (_("section '%s' has unsupported compress type: %d\n"),
>>  		    section->name, chdr.ch_type);
>> @@ -15898,7 +15930,7 @@ load_specific_debug_section (enum dwarf_section_display_enum  debug,
>>
>>        if (uncompressed_size)
>>  	{
>> -	  if (uncompress_section_contents (&start, uncompressed_size,
>> +	  if (uncompress_section_contents (is_zstd, &start, uncompressed_size,
>>  					   &size))
>>  	    {
>>  	      /* Free the compressed buffer, update the section buffer
>
>Hi.
>
>I noticed the following issue (dunno if caused by objcopy or the patches readelf) with this patch:
>
>$ echo 'int main() {return 0;}' | gcc -O2 -g -c -o a.o -x c -
>$ /home/marxin/Programming/binutils/objdir/ld/../binutils/objcopy -O elf32-x86-64 a.o a-x32.o --compress-debug-sections=zstd
>$ /home/marxin/Programming/binutils/objdir/binutils/readelf -wi a-x32.o >/dev/null
>readelf: Error: Unable to decompress section .debug_info
>readelf: Error: No comp units in .debug_info section ?
>
>while zlib is fine:
>
>$ /home/marxin/Programming/binutils/objdir/ld/../binutils/objcopy -O elf32-x86-64 a.o a-x32.o --compress-debug-sections=zlib
>$ /home/marxin/Programming/binutils/objdir/binutils/readelf -wi a-x32.o >/dev/null
>
>Can you please take a look?

objcopy -O elf32-x86-64 a.o a-x32.o --compress-debug-sections=zstd uses
ch_type=1 (ELFCOMPRESS_ZLIB), but I don't know where goes wrong.

Converting EI_CLASS looks very hacky to me.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] readelf: support zstd compressed debug sections [PR 29640]
  2022-10-14  3:35   ` Fangrui Song
@ 2022-10-14  7:47     ` Martin Liška
  2022-10-14  8:26       ` Martin Liška
  0 siblings, 1 reply; 14+ messages in thread
From: Martin Liška @ 2022-10-14  7:47 UTC (permalink / raw)
  To: Fangrui Song; +Cc: binutils, Alan Modra

On 10/14/22 05:35, Fangrui Song wrote:
> On 2022-10-13, Martin Liška wrote:
>> On 10/1/22 08:20, Fangrui Song wrote:
>>> ---
>>>  binutils/Makefile.am |   6 +--
>>>  binutils/Makefile.in |   6 +--
>>>  binutils/readelf.c   | 112 +++++++++++++++++++++++++++----------------
>>>  3 files changed, 78 insertions(+), 46 deletions(-)
>>>
>>> diff --git a/binutils/Makefile.am b/binutils/Makefile.am
>>> index 751fbacce12..b249af721ae 100644
>>> --- a/binutils/Makefile.am
>>> +++ b/binutils/Makefile.am
>>> @@ -54,8 +54,8 @@ DEBUGINFOD_LIBS = @DEBUGINFOD_LIBS@
>>>  WARN_CFLAGS = @WARN_CFLAGS@
>>>  WARN_CFLAGS_FOR_BUILD = @WARN_CFLAGS_FOR_BUILD@
>>>  NO_WERROR = @NO_WERROR@
>>> -AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC)
>>> -AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC)
>>> +AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC) $(ZSTD_CFLAGS)
>>> +AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC) $(ZSTD_CFLAGS)
>>>  LIBICONV = @LIBICONV@
>>>
>>>  # these two are almost the same program
>>> @@ -256,7 +256,7 @@ objcopy_SOURCES = objcopy.c not-strip.c rename.c $(WRITE_DEBUG_SRCS) $(BULIBS)
>>>  strings_SOURCES = strings.c $(BULIBS)
>>>
>>>  readelf_SOURCES = readelf.c version.c unwind-ia64.c dwarf.c demanguse.c $(ELFLIBS)
>>> -readelf_LDADD   = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
>>> +readelf_LDADD   = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(ZSTD_LIBS) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
>>>
>>>  elfedit_SOURCES = elfedit.c version.c $(ELFLIBS)
>>>  elfedit_LDADD = $(LIBINTL) $(LIBIBERTY)
>>> diff --git a/binutils/Makefile.in b/binutils/Makefile.in
>>> index 6de4e239408..7d9039e0f2f 100644
>>> --- a/binutils/Makefile.in
>>> +++ b/binutils/Makefile.in
>>> @@ -651,8 +651,8 @@ am__skipyacc =
>>>  # case both are empty.
>>>  ZLIB = @zlibdir@ -lz
>>>  ZLIBINC = @zlibinc@
>>> -AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC)
>>> -AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC)
>>> +AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC) $(ZSTD_CFLAGS)
>>> +AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC) $(ZSTD_CFLAGS)
>>>
>>>  # these two are almost the same program
>>>  AR_PROG = ar
>>> @@ -790,7 +790,7 @@ size_SOURCES = size.c $(BULIBS)
>>>  objcopy_SOURCES = objcopy.c not-strip.c rename.c $(WRITE_DEBUG_SRCS) $(BULIBS)
>>>  strings_SOURCES = strings.c $(BULIBS)
>>>  readelf_SOURCES = readelf.c version.c unwind-ia64.c dwarf.c demanguse.c $(ELFLIBS)
>>> -readelf_LDADD = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
>>> +readelf_LDADD = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(ZSTD_LIBS) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
>>>  elfedit_SOURCES = elfedit.c version.c $(ELFLIBS)
>>>  elfedit_LDADD = $(LIBINTL) $(LIBIBERTY)
>>>  strip_new_SOURCES = objcopy.c is-strip.c rename.c $(WRITE_DEBUG_SRCS) $(BULIBS)
>>> diff --git a/binutils/readelf.c b/binutils/readelf.c
>>> index 351571c8abb..04cda213f33 100644
>>> --- a/binutils/readelf.c
>>> +++ b/binutils/readelf.c
>>> @@ -44,6 +44,9 @@
>>>  #include <assert.h>
>>>  #include <time.h>
>>>  #include <zlib.h>
>>> +#ifdef HAVE_ZSTD
>>> +#include <zstd.h>
>>> +#endif
>>>  #include <wchar.h>
>>>
>>>  #if defined HAVE_MSGPACK
>>> @@ -15170,48 +15173,56 @@ get_section_contents (Elf_Internal_Shdr * section, Filedata * filedata)
>>>                               _("section contents"));
>>>  }
>>>
>>> -/* Uncompresses a section that was compressed using zlib, in place.  */
>>> +/* Uncompresses a section that was compressed using zlib/zstd, in place.  */
>>>
>>>  static bool
>>> -uncompress_section_contents (unsigned char **buffer,
>>> -                 uint64_t uncompressed_size,
>>> -                 uint64_t *size)
>>> +uncompress_section_contents (bool is_zstd, unsigned char **buffer,
>>> +                 uint64_t uncompressed_size, uint64_t *size)
>>>  {
>>>    uint64_t compressed_size = *size;
>>>    unsigned char *compressed_buffer = *buffer;
>>> -  unsigned char *uncompressed_buffer;
>>> +  unsigned char *uncompressed_buffer = xmalloc (uncompressed_size);
>>>    z_stream strm;
>>>    int rc;
>>>
>>> -  /* It is possible the section consists of several compressed
>>> -     buffers concatenated together, so we uncompress in a loop.  */
>>> -  /* PR 18313: The state field in the z_stream structure is supposed
>>> -     to be invisible to the user (ie us), but some compilers will
>>> -     still complain about it being used without initialisation.  So
>>> -     we first zero the entire z_stream structure and then set the fields
>>> -     that we need.  */
>>> -  memset (& strm, 0, sizeof strm);
>>> -  strm.avail_in = compressed_size;
>>> -  strm.next_in = (Bytef *) compressed_buffer;
>>> -  strm.avail_out = uncompressed_size;
>>> -  uncompressed_buffer = (unsigned char *) xmalloc (uncompressed_size);
>>> +  if (is_zstd)
>>> +    {
>>> +#ifdef HAVE_ZSTD
>>> +      size_t ret = ZSTD_decompress (uncompressed_buffer, uncompressed_size,
>>> +                    compressed_buffer, compressed_size);
>>> +      if (ZSTD_isError (ret))
>>> +    goto fail;
>>> +#endif
>>> +    }
>>> +  else
>>> +    {
>>> +      /* It is possible the section consists of several compressed
>>> +     buffers concatenated together, so we uncompress in a loop.  */
>>> +      /* PR 18313: The state field in the z_stream structure is supposed
>>> +     to be invisible to the user (ie us), but some compilers will
>>> +     still complain about it being used without initialisation.  So
>>> +     we first zero the entire z_stream structure and then set the fields
>>> +     that we need.  */
>>> +      memset (&strm, 0, sizeof strm);
>>> +      strm.avail_in = compressed_size;
>>> +      strm.next_in = (Bytef *)compressed_buffer;
>>> +      strm.avail_out = uncompressed_size;
>>>
>>> -  rc = inflateInit (& strm);
>>> -  while (strm.avail_in > 0)
>>> -    {
>>> -      if (rc != Z_OK)
>>> -        break;
>>> -      strm.next_out = ((Bytef *) uncompressed_buffer
>>> -                       + (uncompressed_size - strm.avail_out));
>>> -      rc = inflate (&strm, Z_FINISH);
>>> -      if (rc != Z_STREAM_END)
>>> -        break;
>>> -      rc = inflateReset (& strm);
>>> +      rc = inflateInit (&strm);
>>> +      while (strm.avail_in > 0)
>>> +    {
>>> +      if (rc != Z_OK)
>>> +        break;
>>> +      strm.next_out = ((Bytef *)uncompressed_buffer
>>> +               + (uncompressed_size - strm.avail_out));
>>> +      rc = inflate (&strm, Z_FINISH);
>>> +      if (rc != Z_STREAM_END)
>>> +        break;
>>> +      rc = inflateReset (&strm);
>>> +    }
>>> +      if (inflateEnd (&strm) != Z_OK || rc != Z_OK || strm.avail_out != 0)
>>> +    goto fail;
>>>      }
>>> -  if (inflateEnd (& strm) != Z_OK
>>> -      || rc != Z_OK
>>> -      || strm.avail_out != 0)
>>> -    goto fail;
>>>
>>>    *buffer = uncompressed_buffer;
>>>    *size = uncompressed_size;
>>> @@ -15254,6 +15265,7 @@ dump_section_as_strings (Elf_Internal_Shdr * section, Filedata * filedata)
>>>      {
>>>        uint64_t new_size = num_bytes;
>>>        uint64_t uncompressed_size = 0;
>>> +      bool is_zstd = false;
>>>
>>>        if ((section->sh_flags & SHF_COMPRESSED) != 0)
>>>      {
>>> @@ -15266,7 +15278,13 @@ dump_section_as_strings (Elf_Internal_Shdr * section, Filedata * filedata)
>>>             by get_compression_header.  */
>>>          goto error_out;
>>>
>>> -      if (chdr.ch_type != ELFCOMPRESS_ZLIB)
>>> +      if (chdr.ch_type == ELFCOMPRESS_ZLIB)
>>> +        ;
>>> +#ifdef HAVE_ZSTD
>>> +      else if (chdr.ch_type == ELFCOMPRESS_ZSTD)
>>> +        is_zstd = true;
>>> +#endif
>>> +      else
>>>          {
>>>            warn (_("section '%s' has unsupported compress type: %d\n"),
>>>              printable_section_name (filedata, section), chdr.ch_type);
>>> @@ -15295,8 +15313,8 @@ dump_section_as_strings (Elf_Internal_Shdr * section, Filedata * filedata)
>>>
>>>        if (uncompressed_size)
>>>      {
>>> -      if (uncompress_section_contents (& start,
>>> -                       uncompressed_size, & new_size))
>>> +      if (uncompress_section_contents (is_zstd, &start, uncompressed_size,
>>> +                       &new_size))
>>>          num_bytes = new_size;
>>>        else
>>>          {
>>> @@ -15470,6 +15488,7 @@ dump_section_as_bytes (Elf_Internal_Shdr *section,
>>>      {
>>>        uint64_t new_size = section_size;
>>>        uint64_t uncompressed_size = 0;
>>> +      bool is_zstd = false;
>>>
>>>        if ((section->sh_flags & SHF_COMPRESSED) != 0)
>>>      {
>>> @@ -15482,7 +15501,13 @@ dump_section_as_bytes (Elf_Internal_Shdr *section,
>>>             by get_compression_header.  */
>>>          goto error_out;
>>>
>>> -      if (chdr.ch_type != ELFCOMPRESS_ZLIB)
>>> +      if (chdr.ch_type == ELFCOMPRESS_ZLIB)
>>> +        ;
>>> +#ifdef HAVE_ZSTD
>>> +      else if (chdr.ch_type == ELFCOMPRESS_ZSTD)
>>> +        is_zstd = true;
>>> +#endif
>>> +      else
>>>          {
>>>            warn (_("section '%s' has unsupported compress type: %d\n"),
>>>              printable_section_name (filedata, section), chdr.ch_type);
>>> @@ -15511,8 +15536,8 @@ dump_section_as_bytes (Elf_Internal_Shdr *section,
>>>
>>>        if (uncompressed_size)
>>>      {
>>> -      if (uncompress_section_contents (& start, uncompressed_size,
>>> -                       & new_size))
>>> +      if (uncompress_section_contents (is_zstd, &start, uncompressed_size,
>>> +                       &new_size))
>>>          {
>>>            section_size = new_size;
>>>          }
>>> @@ -15848,6 +15873,7 @@ load_specific_debug_section (enum dwarf_section_display_enum  debug,
>>>        unsigned char *start = section->start;
>>>        uint64_t size = sec->sh_size;
>>>        uint64_t uncompressed_size = 0;
>>> +      bool is_zstd = false;
>>>
>>>        if ((sec->sh_flags & SHF_COMPRESSED) != 0)
>>>      {
>>> @@ -15869,7 +15895,13 @@ load_specific_debug_section (enum dwarf_section_display_enum  debug,
>>>             by get_compression_header.  */
>>>          return false;
>>>
>>> -      if (chdr.ch_type != ELFCOMPRESS_ZLIB)
>>> +      if (chdr.ch_type == ELFCOMPRESS_ZLIB)
>>> +        ;
>>> +#ifdef HAVE_ZSTD
>>> +      else if (chdr.ch_type == ELFCOMPRESS_ZSTD)
>>> +        is_zstd = true;
>>> +#endif
>>> +      else
>>>          {
>>>            warn (_("section '%s' has unsupported compress type: %d\n"),
>>>              section->name, chdr.ch_type);
>>> @@ -15898,7 +15930,7 @@ load_specific_debug_section (enum dwarf_section_display_enum  debug,
>>>
>>>        if (uncompressed_size)
>>>      {
>>> -      if (uncompress_section_contents (&start, uncompressed_size,
>>> +      if (uncompress_section_contents (is_zstd, &start, uncompressed_size,
>>>                         &size))
>>>          {
>>>            /* Free the compressed buffer, update the section buffer
>>
>> Hi.
>>
>> I noticed the following issue (dunno if caused by objcopy or the patches readelf) with this patch:
>>
>> $ echo 'int main() {return 0;}' | gcc -O2 -g -c -o a.o -x c -
>> $ /home/marxin/Programming/binutils/objdir/ld/../binutils/objcopy -O elf32-x86-64 a.o a-x32.o --compress-debug-sections=zstd
>> $ /home/marxin/Programming/binutils/objdir/binutils/readelf -wi a-x32.o >/dev/null
>> readelf: Error: Unable to decompress section .debug_info
>> readelf: Error: No comp units in .debug_info section ?
>>
>> while zlib is fine:
>>
>> $ /home/marxin/Programming/binutils/objdir/ld/../binutils/objcopy -O elf32-x86-64 a.o a-x32.o --compress-debug-sections=zlib
>> $ /home/marxin/Programming/binutils/objdir/binutils/readelf -wi a-x32.o >/dev/null
>>
>> Can you please take a look?
> 
> objcopy -O elf32-x86-64 a.o a-x32.o --compress-debug-sections=zstd uses
> ch_type=1 (ELFCOMPRESS_ZLIB), but I don't know where goes wrong.
> 
> Converting EI_CLASS looks very hacky to me.

Ok, I've got it. It's unrelated to '-O elf32-x86-64', but to the fact we move from zlib-gabi compression of a.o
to zstd for the objcopy output file.

Simpler reproducer:

$ echo 'int main() {return 0;}' | gcc -O2 -g -c -o a.o -x c - -Wa,--compress-debug-sections=zlib
$ /home/marxin/Programming/binutils/objdir//binutils/objcopy a.o a2.o --compress-debug-sections=zstd
$ /home/marxin/Programming/binutils/objdir/binutils/readelf -wi a2.o
readelf: Error: Unable to decompress section .debug_info
readelf: Error: No comp units in .debug_info section ?

As seen the .debug_info section in a2.o claims to be compressed:

$ readelf -SW a2.o | grep C
  [ 5] .debug_info       PROGBITS        0000000000000000 000043 000050 00   C  0   0  1

But it's not as it's not beneficial and the following code in in bfd/compress.c is taken:

  /* If compression didn't make the section smaller, keep it uncompressed.  */
  if (compressed_size >= uncompressed_size)
    {
      memcpy (buffer, input_buffer, uncompressed_size);
      sec->compress_status = COMPRESS_SECTION_NONE;
    }

Apparently, we miss removal of the compression header :/ Moreover, the following is fine:
$ /home/marxin/Programming/binutils/objdir//binutils/objcopy a.o a2.o --compress-debug-sections=none
$ /home/marxin/Programming/binutils/objdir/binutils/readelf -wi a2.o >/dev/null

Lemme try preparing a fix.

Martin

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] readelf: support zstd compressed debug sections [PR 29640]
  2022-10-14  7:47     ` Martin Liška
@ 2022-10-14  8:26       ` Martin Liška
  2022-10-14 11:28         ` Alan Modra
  0 siblings, 1 reply; 14+ messages in thread
From: Martin Liška @ 2022-10-14  8:26 UTC (permalink / raw)
  To: Fangrui Song; +Cc: binutils, Alan Modra

On 10/14/22 09:47, Martin Liška wrote:
> On 10/14/22 05:35, Fangrui Song wrote:
>> On 2022-10-13, Martin Liška wrote:
>>> On 10/1/22 08:20, Fangrui Song wrote:
>>>> ---
>>>>  binutils/Makefile.am |   6 +--
>>>>  binutils/Makefile.in |   6 +--
>>>>  binutils/readelf.c   | 112 +++++++++++++++++++++++++++----------------
>>>>  3 files changed, 78 insertions(+), 46 deletions(-)
>>>>
>>>> diff --git a/binutils/Makefile.am b/binutils/Makefile.am
>>>> index 751fbacce12..b249af721ae 100644
>>>> --- a/binutils/Makefile.am
>>>> +++ b/binutils/Makefile.am
>>>> @@ -54,8 +54,8 @@ DEBUGINFOD_LIBS = @DEBUGINFOD_LIBS@
>>>>  WARN_CFLAGS = @WARN_CFLAGS@
>>>>  WARN_CFLAGS_FOR_BUILD = @WARN_CFLAGS_FOR_BUILD@
>>>>  NO_WERROR = @NO_WERROR@
>>>> -AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC)
>>>> -AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC)
>>>> +AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC) $(ZSTD_CFLAGS)
>>>> +AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC) $(ZSTD_CFLAGS)
>>>>  LIBICONV = @LIBICONV@
>>>>
>>>>  # these two are almost the same program
>>>> @@ -256,7 +256,7 @@ objcopy_SOURCES = objcopy.c not-strip.c rename.c $(WRITE_DEBUG_SRCS) $(BULIBS)
>>>>  strings_SOURCES = strings.c $(BULIBS)
>>>>
>>>>  readelf_SOURCES = readelf.c version.c unwind-ia64.c dwarf.c demanguse.c $(ELFLIBS)
>>>> -readelf_LDADD   = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
>>>> +readelf_LDADD   = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(ZSTD_LIBS) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
>>>>
>>>>  elfedit_SOURCES = elfedit.c version.c $(ELFLIBS)
>>>>  elfedit_LDADD = $(LIBINTL) $(LIBIBERTY)
>>>> diff --git a/binutils/Makefile.in b/binutils/Makefile.in
>>>> index 6de4e239408..7d9039e0f2f 100644
>>>> --- a/binutils/Makefile.in
>>>> +++ b/binutils/Makefile.in
>>>> @@ -651,8 +651,8 @@ am__skipyacc =
>>>>  # case both are empty.
>>>>  ZLIB = @zlibdir@ -lz
>>>>  ZLIBINC = @zlibinc@
>>>> -AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC)
>>>> -AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC)
>>>> +AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC) $(ZSTD_CFLAGS)
>>>> +AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC) $(ZSTD_CFLAGS)
>>>>
>>>>  # these two are almost the same program
>>>>  AR_PROG = ar
>>>> @@ -790,7 +790,7 @@ size_SOURCES = size.c $(BULIBS)
>>>>  objcopy_SOURCES = objcopy.c not-strip.c rename.c $(WRITE_DEBUG_SRCS) $(BULIBS)
>>>>  strings_SOURCES = strings.c $(BULIBS)
>>>>  readelf_SOURCES = readelf.c version.c unwind-ia64.c dwarf.c demanguse.c $(ELFLIBS)
>>>> -readelf_LDADD = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
>>>> +readelf_LDADD = $(LIBCTF_NOBFD) $(LIBINTL) $(LIBIBERTY) $(ZLIB) $(ZSTD_LIBS) $(DEBUGINFOD_LIBS) $(MSGPACK_LIBS)
>>>>  elfedit_SOURCES = elfedit.c version.c $(ELFLIBS)
>>>>  elfedit_LDADD = $(LIBINTL) $(LIBIBERTY)
>>>>  strip_new_SOURCES = objcopy.c is-strip.c rename.c $(WRITE_DEBUG_SRCS) $(BULIBS)
>>>> diff --git a/binutils/readelf.c b/binutils/readelf.c
>>>> index 351571c8abb..04cda213f33 100644
>>>> --- a/binutils/readelf.c
>>>> +++ b/binutils/readelf.c
>>>> @@ -44,6 +44,9 @@
>>>>  #include <assert.h>
>>>>  #include <time.h>
>>>>  #include <zlib.h>
>>>> +#ifdef HAVE_ZSTD
>>>> +#include <zstd.h>
>>>> +#endif
>>>>  #include <wchar.h>
>>>>
>>>>  #if defined HAVE_MSGPACK
>>>> @@ -15170,48 +15173,56 @@ get_section_contents (Elf_Internal_Shdr * section, Filedata * filedata)
>>>>                               _("section contents"));
>>>>  }
>>>>
>>>> -/* Uncompresses a section that was compressed using zlib, in place.  */
>>>> +/* Uncompresses a section that was compressed using zlib/zstd, in place.  */
>>>>
>>>>  static bool
>>>> -uncompress_section_contents (unsigned char **buffer,
>>>> -                 uint64_t uncompressed_size,
>>>> -                 uint64_t *size)
>>>> +uncompress_section_contents (bool is_zstd, unsigned char **buffer,
>>>> +                 uint64_t uncompressed_size, uint64_t *size)
>>>>  {
>>>>    uint64_t compressed_size = *size;
>>>>    unsigned char *compressed_buffer = *buffer;
>>>> -  unsigned char *uncompressed_buffer;
>>>> +  unsigned char *uncompressed_buffer = xmalloc (uncompressed_size);
>>>>    z_stream strm;
>>>>    int rc;
>>>>
>>>> -  /* It is possible the section consists of several compressed
>>>> -     buffers concatenated together, so we uncompress in a loop.  */
>>>> -  /* PR 18313: The state field in the z_stream structure is supposed
>>>> -     to be invisible to the user (ie us), but some compilers will
>>>> -     still complain about it being used without initialisation.  So
>>>> -     we first zero the entire z_stream structure and then set the fields
>>>> -     that we need.  */
>>>> -  memset (& strm, 0, sizeof strm);
>>>> -  strm.avail_in = compressed_size;
>>>> -  strm.next_in = (Bytef *) compressed_buffer;
>>>> -  strm.avail_out = uncompressed_size;
>>>> -  uncompressed_buffer = (unsigned char *) xmalloc (uncompressed_size);
>>>> +  if (is_zstd)
>>>> +    {
>>>> +#ifdef HAVE_ZSTD
>>>> +      size_t ret = ZSTD_decompress (uncompressed_buffer, uncompressed_size,
>>>> +                    compressed_buffer, compressed_size);
>>>> +      if (ZSTD_isError (ret))
>>>> +    goto fail;
>>>> +#endif
>>>> +    }
>>>> +  else
>>>> +    {
>>>> +      /* It is possible the section consists of several compressed
>>>> +     buffers concatenated together, so we uncompress in a loop.  */
>>>> +      /* PR 18313: The state field in the z_stream structure is supposed
>>>> +     to be invisible to the user (ie us), but some compilers will
>>>> +     still complain about it being used without initialisation.  So
>>>> +     we first zero the entire z_stream structure and then set the fields
>>>> +     that we need.  */
>>>> +      memset (&strm, 0, sizeof strm);
>>>> +      strm.avail_in = compressed_size;
>>>> +      strm.next_in = (Bytef *)compressed_buffer;
>>>> +      strm.avail_out = uncompressed_size;
>>>>
>>>> -  rc = inflateInit (& strm);
>>>> -  while (strm.avail_in > 0)
>>>> -    {
>>>> -      if (rc != Z_OK)
>>>> -        break;
>>>> -      strm.next_out = ((Bytef *) uncompressed_buffer
>>>> -                       + (uncompressed_size - strm.avail_out));
>>>> -      rc = inflate (&strm, Z_FINISH);
>>>> -      if (rc != Z_STREAM_END)
>>>> -        break;
>>>> -      rc = inflateReset (& strm);
>>>> +      rc = inflateInit (&strm);
>>>> +      while (strm.avail_in > 0)
>>>> +    {
>>>> +      if (rc != Z_OK)
>>>> +        break;
>>>> +      strm.next_out = ((Bytef *)uncompressed_buffer
>>>> +               + (uncompressed_size - strm.avail_out));
>>>> +      rc = inflate (&strm, Z_FINISH);
>>>> +      if (rc != Z_STREAM_END)
>>>> +        break;
>>>> +      rc = inflateReset (&strm);
>>>> +    }
>>>> +      if (inflateEnd (&strm) != Z_OK || rc != Z_OK || strm.avail_out != 0)
>>>> +    goto fail;
>>>>      }
>>>> -  if (inflateEnd (& strm) != Z_OK
>>>> -      || rc != Z_OK
>>>> -      || strm.avail_out != 0)
>>>> -    goto fail;
>>>>
>>>>    *buffer = uncompressed_buffer;
>>>>    *size = uncompressed_size;
>>>> @@ -15254,6 +15265,7 @@ dump_section_as_strings (Elf_Internal_Shdr * section, Filedata * filedata)
>>>>      {
>>>>        uint64_t new_size = num_bytes;
>>>>        uint64_t uncompressed_size = 0;
>>>> +      bool is_zstd = false;
>>>>
>>>>        if ((section->sh_flags & SHF_COMPRESSED) != 0)
>>>>      {
>>>> @@ -15266,7 +15278,13 @@ dump_section_as_strings (Elf_Internal_Shdr * section, Filedata * filedata)
>>>>             by get_compression_header.  */
>>>>          goto error_out;
>>>>
>>>> -      if (chdr.ch_type != ELFCOMPRESS_ZLIB)
>>>> +      if (chdr.ch_type == ELFCOMPRESS_ZLIB)
>>>> +        ;
>>>> +#ifdef HAVE_ZSTD
>>>> +      else if (chdr.ch_type == ELFCOMPRESS_ZSTD)
>>>> +        is_zstd = true;
>>>> +#endif
>>>> +      else
>>>>          {
>>>>            warn (_("section '%s' has unsupported compress type: %d\n"),
>>>>              printable_section_name (filedata, section), chdr.ch_type);
>>>> @@ -15295,8 +15313,8 @@ dump_section_as_strings (Elf_Internal_Shdr * section, Filedata * filedata)
>>>>
>>>>        if (uncompressed_size)
>>>>      {
>>>> -      if (uncompress_section_contents (& start,
>>>> -                       uncompressed_size, & new_size))
>>>> +      if (uncompress_section_contents (is_zstd, &start, uncompressed_size,
>>>> +                       &new_size))
>>>>          num_bytes = new_size;
>>>>        else
>>>>          {
>>>> @@ -15470,6 +15488,7 @@ dump_section_as_bytes (Elf_Internal_Shdr *section,
>>>>      {
>>>>        uint64_t new_size = section_size;
>>>>        uint64_t uncompressed_size = 0;
>>>> +      bool is_zstd = false;
>>>>
>>>>        if ((section->sh_flags & SHF_COMPRESSED) != 0)
>>>>      {
>>>> @@ -15482,7 +15501,13 @@ dump_section_as_bytes (Elf_Internal_Shdr *section,
>>>>             by get_compression_header.  */
>>>>          goto error_out;
>>>>
>>>> -      if (chdr.ch_type != ELFCOMPRESS_ZLIB)
>>>> +      if (chdr.ch_type == ELFCOMPRESS_ZLIB)
>>>> +        ;
>>>> +#ifdef HAVE_ZSTD
>>>> +      else if (chdr.ch_type == ELFCOMPRESS_ZSTD)
>>>> +        is_zstd = true;
>>>> +#endif
>>>> +      else
>>>>          {
>>>>            warn (_("section '%s' has unsupported compress type: %d\n"),
>>>>              printable_section_name (filedata, section), chdr.ch_type);
>>>> @@ -15511,8 +15536,8 @@ dump_section_as_bytes (Elf_Internal_Shdr *section,
>>>>
>>>>        if (uncompressed_size)
>>>>      {
>>>> -      if (uncompress_section_contents (& start, uncompressed_size,
>>>> -                       & new_size))
>>>> +      if (uncompress_section_contents (is_zstd, &start, uncompressed_size,
>>>> +                       &new_size))
>>>>          {
>>>>            section_size = new_size;
>>>>          }
>>>> @@ -15848,6 +15873,7 @@ load_specific_debug_section (enum dwarf_section_display_enum  debug,
>>>>        unsigned char *start = section->start;
>>>>        uint64_t size = sec->sh_size;
>>>>        uint64_t uncompressed_size = 0;
>>>> +      bool is_zstd = false;
>>>>
>>>>        if ((sec->sh_flags & SHF_COMPRESSED) != 0)
>>>>      {
>>>> @@ -15869,7 +15895,13 @@ load_specific_debug_section (enum dwarf_section_display_enum  debug,
>>>>             by get_compression_header.  */
>>>>          return false;
>>>>
>>>> -      if (chdr.ch_type != ELFCOMPRESS_ZLIB)
>>>> +      if (chdr.ch_type == ELFCOMPRESS_ZLIB)
>>>> +        ;
>>>> +#ifdef HAVE_ZSTD
>>>> +      else if (chdr.ch_type == ELFCOMPRESS_ZSTD)
>>>> +        is_zstd = true;
>>>> +#endif
>>>> +      else
>>>>          {
>>>>            warn (_("section '%s' has unsupported compress type: %d\n"),
>>>>              section->name, chdr.ch_type);
>>>> @@ -15898,7 +15930,7 @@ load_specific_debug_section (enum dwarf_section_display_enum  debug,
>>>>
>>>>        if (uncompressed_size)
>>>>      {
>>>> -      if (uncompress_section_contents (&start, uncompressed_size,
>>>> +      if (uncompress_section_contents (is_zstd, &start, uncompressed_size,
>>>>                         &size))
>>>>          {
>>>>            /* Free the compressed buffer, update the section buffer
>>>
>>> Hi.
>>>
>>> I noticed the following issue (dunno if caused by objcopy or the patches readelf) with this patch:
>>>
>>> $ echo 'int main() {return 0;}' | gcc -O2 -g -c -o a.o -x c -
>>> $ /home/marxin/Programming/binutils/objdir/ld/../binutils/objcopy -O elf32-x86-64 a.o a-x32.o --compress-debug-sections=zstd
>>> $ /home/marxin/Programming/binutils/objdir/binutils/readelf -wi a-x32.o >/dev/null
>>> readelf: Error: Unable to decompress section .debug_info
>>> readelf: Error: No comp units in .debug_info section ?
>>>
>>> while zlib is fine:
>>>
>>> $ /home/marxin/Programming/binutils/objdir/ld/../binutils/objcopy -O elf32-x86-64 a.o a-x32.o --compress-debug-sections=zlib
>>> $ /home/marxin/Programming/binutils/objdir/binutils/readelf -wi a-x32.o >/dev/null
>>>
>>> Can you please take a look?
>>
>> objcopy -O elf32-x86-64 a.o a-x32.o --compress-debug-sections=zstd uses
>> ch_type=1 (ELFCOMPRESS_ZLIB), but I don't know where goes wrong.
>>
>> Converting EI_CLASS looks very hacky to me.
> 
> Ok, I've got it. It's unrelated to '-O elf32-x86-64', but to the fact we move from zlib-gabi compression of a.o
> to zstd for the objcopy output file.
> 
> Simpler reproducer:
> 
> $ echo 'int main() {return 0;}' | gcc -O2 -g -c -o a.o -x c - -Wa,--compress-debug-sections=zlib
> $ /home/marxin/Programming/binutils/objdir//binutils/objcopy a.o a2.o --compress-debug-sections=zstd
> $ /home/marxin/Programming/binutils/objdir/binutils/readelf -wi a2.o
> readelf: Error: Unable to decompress section .debug_info
> readelf: Error: No comp units in .debug_info section ?
> 
> As seen the .debug_info section in a2.o claims to be compressed:
> 
> $ readelf -SW a2.o | grep C
>   [ 5] .debug_info       PROGBITS        0000000000000000 000043 000050 00   C  0   0  1
> 
> But it's not as it's not beneficial and the following code in in bfd/compress.c is taken:
> 
>   /* If compression didn't make the section smaller, keep it uncompressed.  */
>   if (compressed_size >= uncompressed_size)
>     {
>       memcpy (buffer, input_buffer, uncompressed_size);
>       sec->compress_status = COMPRESS_SECTION_NONE;
>     }
> 
> Apparently, we miss removal of the compression header :/ Moreover, the following is fine:
> $ /home/marxin/Programming/binutils/objdir//binutils/objcopy a.o a2.o --compress-debug-sections=none
> $ /home/marxin/Programming/binutils/objdir/binutils/readelf -wi a2.o >/dev/null
> 
> Lemme try preparing a fix.
> 
> Martin

There's a patch candidate that works for the simple example (all x86_64 ELF), but fails for:
objcopy -O elf32-x86-64 a.o a-x32.o --compress-debug-sections=zstd

likely due to bfd_update_compression_header where compression header is updated.

diff --git a/bfd/compress.c b/bfd/compress.c
index 364df14142b..262e2f5bed2 100644
--- a/bfd/compress.c
+++ b/bfd/compress.c
@@ -202,8 +202,10 @@ bfd_compress_section_contents (bfd *abfd, sec_ptr sec)
       compressed_size += new_header_size;
     }
 
-  /* If compression didn't make the section smaller, keep it uncompressed.  */
-  if (compressed_size >= uncompressed_size)
+  /* If compression didn't make the section smaller, keep it uncompressed.
+     Do it only if the original section is not compressed, otherwise we would
+     need to drop the section header.  */
+  if (!compressed && compressed_size >= uncompressed_size)
     {
       memcpy (buffer, input_buffer, uncompressed_size);
       sec->compress_status = COMPRESS_SECTION_NONE;

@Alan: Can you please help us?

Thanks,
Martin


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] readelf: support zstd compressed debug sections [PR 29640]
  2022-10-14  8:26       ` Martin Liška
@ 2022-10-14 11:28         ` Alan Modra
  2022-10-14 12:09           ` Alan Modra
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Modra @ 2022-10-14 11:28 UTC (permalink / raw)
  To: Martin Liška; +Cc: Fangrui Song, binutils

So we have a zlib-gabi .debug_info section that increases in size with
zstd, so much so that it's better to leave the section uncompressed.
Things go horribly wrong due to leaving compress_status as
COMPRESS_SECTION_NONE.  The section is read again off disk using the
uncompressed size.  So you get the zlib section again with some
garbage at the end.

Also, if the section is to be left uncompressed, the input
SHF_COMPRESSED flag needs to be reset otherwise it is copied to
output.

I'm not ready to commit this, just thought I'd post the results of a
bit of debugging.

diff --git a/bfd/compress.c b/bfd/compress.c
index 364df14142b..5eef38bd50a 100644
--- a/bfd/compress.c
+++ b/bfd/compress.c
@@ -206,15 +206,15 @@ bfd_compress_section_contents (bfd *abfd, sec_ptr sec)
   if (compressed_size >= uncompressed_size)
     {
       memcpy (buffer, input_buffer, uncompressed_size);
-      sec->compress_status = COMPRESS_SECTION_NONE;
+      elf_section_flags (sec) &= ~SHF_COMPRESSED;
     }
   else
     {
       sec->size = uncompressed_size;
       bfd_update_compression_header (abfd, buffer, sec);
       sec->size = compressed_size;
-      sec->compress_status = COMPRESS_SECTION_DONE;
     }
+  sec->compress_status = COMPRESS_SECTION_DONE;
   sec->contents = buffer;
   free (input_buffer);
   return uncompressed_size;

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] readelf: support zstd compressed debug sections [PR 29640]
  2022-10-14 11:28         ` Alan Modra
@ 2022-10-14 12:09           ` Alan Modra
  2022-10-16  4:42             ` Alan Modra
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Modra @ 2022-10-14 12:09 UTC (permalink / raw)
  To: Martin Liška; +Cc: Fangrui Song, binutils

On Fri, Oct 14, 2022 at 09:58:41PM +1030, Alan Modra wrote:
> So we have a zlib-gabi .debug_info section that increases in size with
> zstd, so much so that it's better to leave the section uncompressed.
> Things go horribly wrong due to leaving compress_status as
> COMPRESS_SECTION_NONE.  The section is read again off disk using the
> uncompressed size.  So you get the zlib section again with some
> garbage at the end.
> 
> Also, if the section is to be left uncompressed, the input
> SHF_COMPRESSED flag needs to be reset otherwise it is copied to
> output.
> 
> I'm not ready to commit this, just thought I'd post the results of a
> bit of debugging.

And if I'd run the testsuite before posting, I may have posted a
better patch..  Using COMPRESS_SECTION_DONE led to .debug -> .zdebug
renaming of sections, so it appears we want another compress_status
that says the final section contents are in sec->contents.

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 25e1806e689..4f4658021c6 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -974,11 +974,12 @@ typedef struct bfd_section
   unsigned int gc_mark : 1;
 
   /* Section compression status.  */
-  unsigned int compress_status : 2;
+  unsigned int compress_status : 3;
 #define COMPRESS_SECTION_NONE    0
 #define COMPRESS_SECTION_DONE    1
 #define DECOMPRESS_SECTION_ZLIB  2
 #define DECOMPRESS_SECTION_ZSTD  3
+#define DECOMPRESS_SECTION_DONE  4
 
   /* The following flags are used by the ELF linker. */
 
diff --git a/bfd/compress.c b/bfd/compress.c
index 364df14142b..94bc5412a89 100644
--- a/bfd/compress.c
+++ b/bfd/compress.c
@@ -151,7 +151,7 @@ bfd_compress_section_contents (bfd *abfd, sec_ptr sec)
 	  free (input_buffer);
 	  bfd_set_section_alignment (sec, uncompressed_alignment_pow);
 	  sec->contents = buffer;
-	  sec->compress_status = COMPRESS_SECTION_DONE;
+	  sec->compress_status = DECOMPRESS_SECTION_DONE;
 	  sec->size = uncompressed_size;
 	  input_buffer = buffer;
 	}
@@ -206,7 +206,8 @@ bfd_compress_section_contents (bfd *abfd, sec_ptr sec)
   if (compressed_size >= uncompressed_size)
     {
       memcpy (buffer, input_buffer, uncompressed_size);
-      sec->compress_status = COMPRESS_SECTION_NONE;
+      elf_section_flags (sec) &= ~SHF_COMPRESSED;
+      sec->compress_status = DECOMPRESS_SECTION_DONE;
     }
   else
     {
@@ -361,6 +362,7 @@ bfd_get_full_section_contents (bfd *abfd, sec_ptr sec, bfd_byte **ptr)
       return true;
 
     case COMPRESS_SECTION_DONE:
+    case DECOMPRESS_SECTION_DONE:
       if (sec->contents == NULL)
 	return false;
       if (p == NULL)
diff --git a/bfd/section.c b/bfd/section.c
index 614570e976e..52cf7e042cd 100644
--- a/bfd/section.c
+++ b/bfd/section.c
@@ -389,11 +389,12 @@ CODE_FRAGMENT
 .  unsigned int gc_mark : 1;
 .
 .  {* Section compression status.  *}
-.  unsigned int compress_status : 2;
+.  unsigned int compress_status : 3;
 .#define COMPRESS_SECTION_NONE    0
 .#define COMPRESS_SECTION_DONE    1
 .#define DECOMPRESS_SECTION_ZLIB  2
 .#define DECOMPRESS_SECTION_ZSTD  3
+.#define DECOMPRESS_SECTION_DONE  4
 .
 .  {* The following flags are used by the ELF linker. *}
 .


-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] readelf: support zstd compressed debug sections [PR 29640]
  2022-10-14 12:09           ` Alan Modra
@ 2022-10-16  4:42             ` Alan Modra
  2022-10-16 20:46               ` Fangrui Song
  2022-10-17  7:52               ` Martin Liška
  0 siblings, 2 replies; 14+ messages in thread
From: Alan Modra @ 2022-10-16  4:42 UTC (permalink / raw)
  To: Martin Liška; +Cc: Fangrui Song, binutils

On Fri, Oct 14, 2022 at 10:39:17PM +1030, Alan Modra wrote:
> On Fri, Oct 14, 2022 at 09:58:41PM +1030, Alan Modra wrote:
> > So we have a zlib-gabi .debug_info section that increases in size with
> > zstd, so much so that it's better to leave the section uncompressed.
> > Things go horribly wrong due to leaving compress_status as
> > COMPRESS_SECTION_NONE.  The section is read again off disk using the
> > uncompressed size.  So you get the zlib section again with some
> > garbage at the end.
> > 
> > Also, if the section is to be left uncompressed, the input
> > SHF_COMPRESSED flag needs to be reset otherwise it is copied to
> > output.
> > 
> > I'm not ready to commit this, just thought I'd post the results of a
> > bit of debugging.
> 
> And if I'd run the testsuite before posting, I may have posted a
> better patch..  Using COMPRESS_SECTION_DONE led to .debug -> .zdebug
> renaming of sections, so it appears we want another compress_status
> that says the final section contents are in sec->contents.

Another compress_status isn't elegant.  I'm about to commit this:

	* bfd.c (bfd_convert_section_contents): Handle zstd.
	* compress.c (bfd_compress_section_contents): When section
	contents are uncompressed set SEC_IN_MEMORY flag,
	compress_status to COMRESS_SECTION_NONE, and clear
	SHF_COMPRESSED.  Set SEC_IN_MEMORY for compressed contents.
	(bfd_get_full_section_contents): Don't check section size
	against file size when SEC_IN_MEMORY.
	(bfd_cache_section_contents): Delete function.
	* elf32-arm.c (elf32_arm_get_synthetic_symtab): Expand
	bfd_cache_section_contents here.
	* bfd-in2.h: Regenerate.

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 25e1806e689..534a46439fc 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -7964,9 +7964,6 @@ bfd_byte *bfd_simple_get_relocated_section_contents
 bool bfd_get_full_section_contents
    (bfd *abfd, asection *section, bfd_byte **ptr);
 
-void bfd_cache_section_contents
-   (asection *sec, void *contents);
-
 bool bfd_is_section_compressed_info
    (bfd *abfd, asection *section,
     int *compression_header_size_p,
diff --git a/bfd/bfd.c b/bfd/bfd.c
index 5f2033bee7a..4fca8250082 100644
--- a/bfd/bfd.c
+++ b/bfd/bfd.c
@@ -2801,14 +2801,14 @@ bfd_convert_section_contents (bfd *ibfd, sec_ptr isec, bfd *obfd,
   if (ohdr_size == sizeof (Elf32_External_Chdr))
     {
       Elf32_External_Chdr *echdr = (Elf32_External_Chdr *) contents;
-      bfd_put_32 (obfd, ELFCOMPRESS_ZLIB, &echdr->ch_type);
+      bfd_put_32 (obfd, chdr.ch_type, &echdr->ch_type);
       bfd_put_32 (obfd, chdr.ch_size, &echdr->ch_size);
       bfd_put_32 (obfd, chdr.ch_addralign, &echdr->ch_addralign);
     }
   else
     {
       Elf64_External_Chdr *echdr = (Elf64_External_Chdr *) contents;
-      bfd_put_32 (obfd, ELFCOMPRESS_ZLIB, &echdr->ch_type);
+      bfd_put_32 (obfd, chdr.ch_type, &echdr->ch_type);
       bfd_put_32 (obfd, 0, &echdr->ch_reserved);
       bfd_put_64 (obfd, chdr.ch_size, &echdr->ch_size);
       bfd_put_64 (obfd, chdr.ch_addralign, &echdr->ch_addralign);
diff --git a/bfd/compress.c b/bfd/compress.c
index 364df14142b..9608a0a6341 100644
--- a/bfd/compress.c
+++ b/bfd/compress.c
@@ -151,7 +151,8 @@ bfd_compress_section_contents (bfd *abfd, sec_ptr sec)
 	  free (input_buffer);
 	  bfd_set_section_alignment (sec, uncompressed_alignment_pow);
 	  sec->contents = buffer;
-	  sec->compress_status = COMPRESS_SECTION_DONE;
+	  sec->flags |= SEC_IN_MEMORY;
+	  sec->compress_status = COMPRESS_SECTION_NONE;
 	  sec->size = uncompressed_size;
 	  input_buffer = buffer;
 	}
@@ -206,6 +207,7 @@ bfd_compress_section_contents (bfd *abfd, sec_ptr sec)
   if (compressed_size >= uncompressed_size)
     {
       memcpy (buffer, input_buffer, uncompressed_size);
+      elf_section_flags (sec) &= ~SHF_COMPRESSED;
       sec->compress_status = COMPRESS_SECTION_NONE;
     }
   else
@@ -216,6 +218,7 @@ bfd_compress_section_contents (bfd *abfd, sec_ptr sec)
       sec->compress_status = COMPRESS_SECTION_DONE;
     }
   sec->contents = buffer;
+  sec->flags |= SEC_IN_MEMORY;
   free (input_buffer);
   return uncompressed_size;
 }
@@ -268,6 +271,7 @@ bfd_get_full_section_contents (bfd *abfd, sec_ptr sec, bfd_byte **ptr)
 	  ufile_ptr filesize = bfd_get_file_size (abfd);
 	  if (filesize > 0
 	      && filesize < sz
+	      && (bfd_section_flags (sec) & SEC_IN_MEMORY) == 0
 	      /* PR 24753: Linker created sections can be larger than
 		 the file size, eg if they are being used to hold stubs.  */
 	      && (bfd_section_flags (sec) & SEC_LINKER_CREATED) == 0
@@ -380,29 +384,6 @@ bfd_get_full_section_contents (bfd *abfd, sec_ptr sec, bfd_byte **ptr)
     }
 }
 
-/*
-FUNCTION
-	bfd_cache_section_contents
-
-SYNOPSIS
-	void bfd_cache_section_contents
-	  (asection *sec, void *contents);
-
-DESCRIPTION
-	Stash @var(contents) so any following reads of @var(sec) do
-	not need to decompress again.
-*/
-
-void
-bfd_cache_section_contents (asection *sec, void *contents)
-{
-  if (sec->compress_status == DECOMPRESS_SECTION_ZLIB
-      || sec->compress_status == DECOMPRESS_SECTION_ZSTD)
-    sec->compress_status = COMPRESS_SECTION_DONE;
-  sec->contents = contents;
-  sec->flags |= SEC_IN_MEMORY;
-}
-
 /*
 FUNCTION
 	bfd_is_section_compressed_info
diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index 0852b38ae4c..ec18a8ab362 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -20020,9 +20020,11 @@ elf32_arm_get_synthetic_symtab (bfd *abfd,
   data = plt->contents;
   if (data == NULL)
     {
-      if (!bfd_get_full_section_contents (abfd, (asection *) plt, &data) || data == NULL)
+      if (!bfd_get_full_section_contents (abfd, plt, &data)
+	  || data == NULL)
 	return -1;
-      bfd_cache_section_contents ((asection *) plt, data);
+      plt->contents = data;
+      plt->flags |= SEC_IN_MEMORY;
     }
 
   count = relplt->size / hdr->sh_entsize;


-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] readelf: support zstd compressed debug sections [PR 29640]
  2022-10-16  4:42             ` Alan Modra
@ 2022-10-16 20:46               ` Fangrui Song
  2022-10-21  9:16                 ` Alan Modra
  2022-10-17  7:52               ` Martin Liška
  1 sibling, 1 reply; 14+ messages in thread
From: Fangrui Song @ 2022-10-16 20:46 UTC (permalink / raw)
  To: Alan Modra; +Cc: Martin Liška, binutils

On 2022-10-16, Alan Modra wrote:
>On Fri, Oct 14, 2022 at 10:39:17PM +1030, Alan Modra wrote:
>> On Fri, Oct 14, 2022 at 09:58:41PM +1030, Alan Modra wrote:
>> > So we have a zlib-gabi .debug_info section that increases in size with
>> > zstd, so much so that it's better to leave the section uncompressed.
>> > Things go horribly wrong due to leaving compress_status as
>> > COMPRESS_SECTION_NONE.  The section is read again off disk using the
>> > uncompressed size.  So you get the zlib section again with some
>> > garbage at the end.
>> >
>> > Also, if the section is to be left uncompressed, the input
>> > SHF_COMPRESSED flag needs to be reset otherwise it is copied to
>> > output.
>> >
>> > I'm not ready to commit this, just thought I'd post the results of a
>> > bit of debugging.
>>
>> And if I'd run the testsuite before posting, I may have posted a
>> better patch..  Using COMPRESS_SECTION_DONE led to .debug -> .zdebug
>> renaming of sections, so it appears we want another compress_status
>> that says the final section contents are in sec->contents.
>
>Another compress_status isn't elegant.  I'm about to commit this:
>
>	* bfd.c (bfd_convert_section_contents): Handle zstd.
>	* compress.c (bfd_compress_section_contents): When section
>	contents are uncompressed set SEC_IN_MEMORY flag,
>	compress_status to COMRESS_SECTION_NONE, and clear
>	SHF_COMPRESSED.  Set SEC_IN_MEMORY for compressed contents.
>	(bfd_get_full_section_contents): Don't check section size
>	against file size when SEC_IN_MEMORY.
>	(bfd_cache_section_contents): Delete function.
>	* elf32-arm.c (elf32_arm_get_synthetic_symtab): Expand
>	bfd_cache_section_contents here.
>	* bfd-in2.h: Regenerate.

Thanks. Commit 206e9791cb09459bf92603428370c16bfde282ac
fixed the issue.

~/Dev/binutils-gdb/out/debug/binutils/objcopy -O elf32-x86-64 a.o a-x32.o --compress-debug-sections=zstd
~/Dev/binutils-gdb/out/debug/binutils/readelf -wi a-x32.o  # good


Is this readelf patch ok for installing?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] readelf: support zstd compressed debug sections [PR 29640]
  2022-10-16  4:42             ` Alan Modra
  2022-10-16 20:46               ` Fangrui Song
@ 2022-10-17  7:52               ` Martin Liška
  1 sibling, 0 replies; 14+ messages in thread
From: Martin Liška @ 2022-10-17  7:52 UTC (permalink / raw)
  To: Alan Modra; +Cc: Fangrui Song, binutils

On 10/16/22 06:42, Alan Modra wrote:
> |Another compress_status isn't elegant. I'm about to commit this:|

Thanks for help Alan!

Cheers,
Martin

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] readelf: support zstd compressed debug sections [PR 29640]
  2022-10-06  2:20 ` Fangrui Song
@ 2022-10-20  8:04   ` Martin Liška
  2022-10-20  8:05   ` Martin Liška
  1 sibling, 0 replies; 14+ messages in thread
From: Martin Liška @ 2022-10-20  8:04 UTC (permalink / raw)
  To: Fangrui Song, binutils; +Cc: Alan Modra

On 10/6/22 04:20, Fangrui Song via Binutils wrote:
> On 2022-09-30, Fangrui Song wrote:
>> ---
>> binutils/Makefile.am |   6 +--
>> binutils/Makefile.in |   6 +--
>> binutils/readelf.c   | 112 +++++++++++++++++++++++++++----------------
>> 3 files changed, 78 insertions(+), 46 deletions(-)
> 
> ping:)


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] readelf: support zstd compressed debug sections [PR 29640]
  2022-10-06  2:20 ` Fangrui Song
  2022-10-20  8:04   ` Martin Liška
@ 2022-10-20  8:05   ` Martin Liška
  1 sibling, 0 replies; 14+ messages in thread
From: Martin Liška @ 2022-10-20  8:05 UTC (permalink / raw)
  To: Fangrui Song, binutils; +Cc: Alan Modra

Alan, may I please ping this?

On 10/6/22 04:20, Fangrui Song via Binutils wrote:
> On 2022-09-30, Fangrui Song wrote:
>> ---
>> binutils/Makefile.am |   6 +--
>> binutils/Makefile.in |   6 +--
>> binutils/readelf.c   | 112 +++++++++++++++++++++++++++----------------
>> 3 files changed, 78 insertions(+), 46 deletions(-)
> 
> ping:)


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] readelf: support zstd compressed debug sections [PR 29640]
  2022-10-16 20:46               ` Fangrui Song
@ 2022-10-21  9:16                 ` Alan Modra
  0 siblings, 0 replies; 14+ messages in thread
From: Alan Modra @ 2022-10-21  9:16 UTC (permalink / raw)
  To: Fangrui Song; +Cc: Martin Liška, binutils

On Sun, Oct 16, 2022 at 01:46:52PM -0700, Fangrui Song wrote:
> Is this readelf patch ok for installing?

Yes.  Sorry for the delay in reviewing.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-10-21  9:17 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-01  6:20 [PATCH] readelf: support zstd compressed debug sections [PR 29640] Fangrui Song
2022-10-06  2:20 ` Fangrui Song
2022-10-20  8:04   ` Martin Liška
2022-10-20  8:05   ` Martin Liška
2022-10-13 11:38 ` Martin Liška
2022-10-14  3:35   ` Fangrui Song
2022-10-14  7:47     ` Martin Liška
2022-10-14  8:26       ` Martin Liška
2022-10-14 11:28         ` Alan Modra
2022-10-14 12:09           ` Alan Modra
2022-10-16  4:42             ` Alan Modra
2022-10-16 20:46               ` Fangrui Song
2022-10-21  9:16                 ` Alan Modra
2022-10-17  7:52               ` Martin Liška

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).