[PATCHv2] support ZSTD compression algorithm

Message ID a0a4205a-2143-53a7-f3ba-6083518cd3d5@suse.cz
State Committed
Headers
Series [PATCHv2] support ZSTD compression algorithm |

Commit Message

Martin Liška Nov. 29, 2022, 12:05 p.m. UTC
  Hi.

There's second version of the patch that fully support both compression and decompression.

Changes from the v1:
- compression support added
- zstd detection is fixed
- new tests are added
- builds fine w/ and w/o the ZSTD library

What's currently missing and where I need a help:

1) When I build ./configure --without-zstd, I don't have
a reasonable error message (something similar to binutils's readelf:
readelf: Warning: section '.debug_str' has unsupported compress type: 2)
even though, __libelf_decompress returns NULL and __libelf_seterrno).
One can see a garbage in the console.

How to handle that properly?

2) How should I run my newly added tests conditionally if zstd
configuration support is enabled?

Cheers,
Martin

---
  configure.ac               |   8 +-
  libelf/Makefile.am         |   2 +-
  libelf/elf_compress.c      | 294 ++++++++++++++++++++++++++++++-------
  libelf/elf_compress_gnu.c  |   5 +-
  libelf/libelfP.h           |   4 +-
  src/elfcompress.c          | 144 ++++++++++--------
  src/readelf.c              |  18 ++-
  tests/run-compress-test.sh |  24 +++
  8 files changed, 373 insertions(+), 126 deletions(-)
  

Comments

Martin Liška Dec. 9, 2022, 10:17 a.m. UTC | #1
PING^1

On 11/29/22 13:05, Martin Liška wrote:
> Hi.
> 
> There's second version of the patch that fully support both compression and decompression.
> 
> Changes from the v1:
> - compression support added
> - zstd detection is fixed
> - new tests are added
> - builds fine w/ and w/o the ZSTD library
> 
> What's currently missing and where I need a help:
> 
> 1) When I build ./configure --without-zstd, I don't have
> a reasonable error message (something similar to binutils's readelf:
> readelf: Warning: section '.debug_str' has unsupported compress type: 2)
> even though, __libelf_decompress returns NULL and __libelf_seterrno).
> One can see a garbage in the console.
> 
> How to handle that properly?
> 
> 2) How should I run my newly added tests conditionally if zstd
> configuration support is enabled?
> 
> Cheers,
> Martin
> 
> ---
>  configure.ac               |   8 +-
>  libelf/Makefile.am         |   2 +-
>  libelf/elf_compress.c      | 294 ++++++++++++++++++++++++++++++-------
>  libelf/elf_compress_gnu.c  |   5 +-
>  libelf/libelfP.h           |   4 +-
>  src/elfcompress.c          | 144 ++++++++++--------
>  src/readelf.c              |  18 ++-
>  tests/run-compress-test.sh |  24 +++
>  8 files changed, 373 insertions(+), 126 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 59be27ac..07cfa54b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -410,6 +410,11 @@ dnl Test for bzlib and xz/lzma/zstd, gives BZLIB/LZMALIB/ZSTD .am
>  dnl conditional and config.h USE_BZLIB/USE_LZMALIB/USE_ZSTD #define.
>  save_LIBS="$LIBS"
>  LIBS=
> +eu_ZIPLIB(zstd,ZSTD,zstd,ZSTD_decompress,[ZSTD (zst)])
> +AS_IF([test "x$with_zstd" = xyes], [LIBZSTD="libzstd"], [LIBLZSTD=""])
> +AC_SUBST([LIBZSTD])
> +zstd_LIBS="$LIBS"
> +AC_SUBST([zstd_LIBS])
>  eu_ZIPLIB(bzlib,BZLIB,bz2,BZ2_bzdopen,bzip2)
>  # We need this since bzip2 doesn't have a pkgconfig file.
>  BZ2_LIB="$LIBS"
> @@ -417,9 +422,6 @@ AC_SUBST([BZ2_LIB])
>  eu_ZIPLIB(lzma,LZMA,lzma,lzma_auto_decoder,[LZMA (xz)])
>  AS_IF([test "x$with_lzma" = xyes], [LIBLZMA="liblzma"], [LIBLZMA=""])
>  AC_SUBST([LIBLZMA])
> -eu_ZIPLIB(zstd,ZSTD,zstd,ZSTD_decompress,[ZSTD (zst)])
> -AS_IF([test "x$with_zstd" = xyes], [LIBZSTD="libzstd"], [LIBLZSTD=""])
> -AC_SUBST([LIBZSTD])
>  zip_LIBS="$LIBS"
>  LIBS="$save_LIBS"
>  AC_SUBST([zip_LIBS])
> diff --git a/libelf/Makefile.am b/libelf/Makefile.am
> index 560ed45f..24c25cf8 100644
> --- a/libelf/Makefile.am
> +++ b/libelf/Makefile.am
> @@ -106,7 +106,7 @@ libelf_pic_a_SOURCES =
>  am_libelf_pic_a_OBJECTS = $(libelf_a_SOURCES:.c=.os)
>  
>  libelf_so_DEPS = ../lib/libeu.a
> -libelf_so_LDLIBS = $(libelf_so_DEPS) -lz
> +libelf_so_LDLIBS = $(libelf_so_DEPS) -lz $(zstd_LIBS)
>  if USE_LOCKS
>  libelf_so_LDLIBS += -lpthread
>  endif
> diff --git a/libelf/elf_compress.c b/libelf/elf_compress.c
> index d7f53af2..7a6e37a4 100644
> --- a/libelf/elf_compress.c
> +++ b/libelf/elf_compress.c
> @@ -39,6 +39,10 @@
>  #include <string.h>
>  #include <zlib.h>
>  
> +#ifdef USE_ZSTD
> +#include <zstd.h>
> +#endif
> +
>  /* Cleanup and return result.  Don't leak memory.  */
>  static void *
>  do_deflate_cleanup (void *result, z_stream *z, void *out_buf,
> @@ -54,53 +58,14 @@ do_deflate_cleanup (void *result, z_stream *z, void *out_buf,
>  #define deflate_cleanup(result, cdata) \
>      do_deflate_cleanup(result, &z, out_buf, cdata)
>  
> -/* Given a section, uses the (in-memory) Elf_Data to extract the
> -   original data size (including the given header size) and data
> -   alignment.  Returns a buffer that has at least hsize bytes (for the
> -   caller to fill in with a header) plus zlib compressed date.  Also
> -   returns the new buffer size in new_size (hsize + compressed data
> -   size).  Returns (void *) -1 when FORCE is false and the compressed
> -   data would be bigger than the original data.  */
>  void *
>  internal_function
> -__libelf_compress (Elf_Scn *scn, size_t hsize, int ei_data,
> -           size_t *orig_size, size_t *orig_addralign,
> -           size_t *new_size, bool force)
> +__libelf_compress_zlib (Elf_Scn *scn, size_t hsize, int ei_data,
> +            size_t *orig_size, size_t *orig_addralign,
> +            size_t *new_size, bool force,
> +            Elf_Data *data, Elf_Data *next_data,
> +            void *out_buf, size_t out_size, size_t block)
>  {
> -  /* The compressed data is the on-disk data.  We simplify the
> -     implementation a bit by asking for the (converted) in-memory
> -     data (which might be all there is if the user created it with
> -     elf_newdata) and then convert back to raw if needed before
> -     compressing.  Should be made a bit more clever to directly
> -     use raw if that is directly available.  */
> -  Elf_Data *data = elf_getdata (scn, NULL);
> -  if (data == NULL)
> -    return NULL;
> -
> -  /* When not forced and we immediately know we would use more data by
> -     compressing, because of the header plus zlib overhead (five bytes
> -     per 16 KB block, plus a one-time overhead of six bytes for the
> -     entire stream), don't do anything.  */
> -  Elf_Data *next_data = elf_getdata (scn, data);
> -  if (next_data == NULL && !force
> -      && data->d_size <= hsize + 5 + 6)
> -    return (void *) -1;
> -
> -  *orig_addralign = data->d_align;
> -  *orig_size = data->d_size;
> -
> -  /* Guess an output block size. 1/8th of the original Elf_Data plus
> -     hsize.  Make the first chunk twice that size (25%), then increase
> -     by a block (12.5%) when necessary.  */
> -  size_t block = (data->d_size / 8) + hsize;
> -  size_t out_size = 2 * block;
> -  void *out_buf = malloc (out_size);
> -  if (out_buf == NULL)
> -    {
> -      __libelf_seterrno (ELF_E_NOMEM);
> -      return NULL;
> -    }
> -
>    /* Caller gets to fill in the header at the start.  Just skip it here.  */
>    size_t used = hsize;
>  
> @@ -205,9 +170,186 @@ __libelf_compress (Elf_Scn *scn, size_t hsize, int ei_data,
>    return out_buf;
>  }
>  
> +#ifdef USE_ZSTD
> +/* Cleanup and return result.  Don't leak memory.  */
> +static void *
> +do_zstd_cleanup (void *result, ZSTD_CCtx * const cctx, void *out_buf,
> +         Elf_Data *cdatap)
> +{
> +  ZSTD_freeCCtx (cctx);
> +  free (out_buf);
> +  if (cdatap != NULL)
> +    free (cdatap->d_buf);
> +  return result;
> +}
> +
> +#define zstd_cleanup(result, cdata) \
> +    do_zstd_cleanup(result, cctx, out_buf, cdata)
> +
>  void *
>  internal_function
> -__libelf_decompress (void *buf_in, size_t size_in, size_t size_out)
> +__libelf_compress_zstd (Elf_Scn *scn, size_t hsize, int ei_data,
> +            size_t *orig_size, size_t *orig_addralign,
> +            size_t *new_size, bool force,
> +            Elf_Data *data, Elf_Data *next_data,
> +            void *out_buf, size_t out_size, size_t block)
> +{
> +  /* Caller gets to fill in the header at the start.  Just skip it here.  */
> +  size_t used = hsize;
> +
> +  ZSTD_CCtx* const cctx = ZSTD_createCCtx();
> +  Elf_Data cdata;
> +  cdata.d_buf = NULL;
> +
> +  /* Loop over data buffers.  */
> +  ZSTD_EndDirective mode = ZSTD_e_continue;
> +
> +  do
> +    {
> +      /* Convert to raw if different endianness.  */
> +      cdata = *data;
> +      bool convert = ei_data != MY_ELFDATA && data->d_size > 0;
> +      if (convert)
> +    {
> +      /* Don't do this conversion in place, we might want to keep
> +         the original data around, caller decides.  */
> +      cdata.d_buf = malloc (data->d_size);
> +      if (cdata.d_buf == NULL)
> +        {
> +          __libelf_seterrno (ELF_E_NOMEM);
> +          return zstd_cleanup (NULL, NULL);
> +        }
> +      if (gelf_xlatetof (scn->elf, &cdata, data, ei_data) == NULL)
> +        return zstd_cleanup (NULL, &cdata);
> +    }
> +
> +      ZSTD_inBuffer ib = { cdata.d_buf, cdata.d_size, 0 };
> +
> +      /* Get next buffer to see if this is the last one.  */
> +      data = next_data;
> +      if (data != NULL)
> +    {
> +      *orig_addralign = MAX (*orig_addralign, data->d_align);
> +      *orig_size += data->d_size;
> +      next_data = elf_getdata (scn, data);
> +    }
> +      else
> +    mode = ZSTD_e_end;
> +
> +      /* Flush one data buffer.  */
> +      for (;;)
> +    {
> +      ZSTD_outBuffer ob = { out_buf + used, out_size - used, 0 };
> +      size_t ret = ZSTD_compressStream2 (cctx, &ob, &ib, mode);
> +      if (ZSTD_isError (ret))
> +        {
> +          __libelf_seterrno (ELF_E_COMPRESS_ERROR);
> +          return zstd_cleanup (NULL, convert ? &cdata : NULL);
> +        }
> +      used += ob.pos;
> +
> +      /* Bail out if we are sure the user doesn't want the
> +         compression forced and we are using more compressed data
> +         than original data.  */
> +      if (!force && mode == ZSTD_e_end && used >= *orig_size)
> +        return zstd_cleanup ((void *) -1, convert ? &cdata : NULL);
> +
> +      if (ret > 0)
> +        {
> +          void *bigger = realloc (out_buf, out_size + block);
> +          if (bigger == NULL)
> +        {
> +          __libelf_seterrno (ELF_E_NOMEM);
> +          return zstd_cleanup (NULL, convert ? &cdata : NULL);
> +        }
> +          out_buf = bigger;
> +          out_size += block;
> +        }
> +      else
> +        break;
> +    }
> +
> +      if (convert)
> +    {
> +      free (cdata.d_buf);
> +      cdata.d_buf = NULL;
> +    }
> +    }
> +  while (mode != ZSTD_e_end); /* More data blocks.  */
> +
> +  ZSTD_freeCCtx (cctx);
> +  *new_size = used;
> +  return out_buf;
> +}
> +#endif
> +
> +/* Given a section, uses the (in-memory) Elf_Data to extract the
> +   original data size (including the given header size) and data
> +   alignment.  Returns a buffer that has at least hsize bytes (for the
> +   caller to fill in with a header) plus zlib compressed date.  Also
> +   returns the new buffer size in new_size (hsize + compressed data
> +   size).  Returns (void *) -1 when FORCE is false and the compressed
> +   data would be bigger than the original data.  */
> +void *
> +internal_function
> +__libelf_compress (Elf_Scn *scn, size_t hsize, int ei_data,
> +           size_t *orig_size, size_t *orig_addralign,
> +           size_t *new_size, bool force, bool use_zstd)
> +{
> +  /* The compressed data is the on-disk data.  We simplify the
> +     implementation a bit by asking for the (converted) in-memory
> +     data (which might be all there is if the user created it with
> +     elf_newdata) and then convert back to raw if needed before
> +     compressing.  Should be made a bit more clever to directly
> +     use raw if that is directly available.  */
> +  Elf_Data *data = elf_getdata (scn, NULL);
> +  if (data == NULL)
> +    return NULL;
> +
> +  /* When not forced and we immediately know we would use more data by
> +     compressing, because of the header plus zlib overhead (five bytes
> +     per 16 KB block, plus a one-time overhead of six bytes for the
> +     entire stream), don't do anything.
> +     Size estimation for ZSTD compression would be similar.  */
> +  Elf_Data *next_data = elf_getdata (scn, data);
> +  if (next_data == NULL && !force
> +      && data->d_size <= hsize + 5 + 6)
> +    return (void *) -1;
> +
> +  *orig_addralign = data->d_align;
> +  *orig_size = data->d_size;
> +
> +  /* Guess an output block size. 1/8th of the original Elf_Data plus
> +     hsize.  Make the first chunk twice that size (25%), then increase
> +     by a block (12.5%) when necessary.  */
> +  size_t block = (data->d_size / 8) + hsize;
> +  size_t out_size = 2 * block;
> +  void *out_buf = malloc (out_size);
> +  if (out_buf == NULL)
> +    {
> +      __libelf_seterrno (ELF_E_NOMEM);
> +      return NULL;
> +    }
> +
> +  if (use_zstd)
> +#ifdef USE_ZSTD
> +    return __libelf_compress_zstd (scn, hsize, ei_data, orig_size,
> +                   orig_addralign, new_size, force,
> +                   data, next_data, out_buf, out_size,
> +                   block);
> +#else
> +    return NULL;
> +#endif
> +  else
> +    return __libelf_compress_zlib (scn, hsize, ei_data, orig_size,
> +                   orig_addralign, new_size, force,
> +                   data, next_data, out_buf, out_size,
> +                   block);
> +}
> +
> +void *
> +internal_function
> +__libelf_decompress_zlib (void *buf_in, size_t size_in, size_t size_out)
>  {
>    /* Catch highly unlikely compression ratios so we don't allocate
>       some giant amount of memory for nothing. The max compression
> @@ -218,7 +360,7 @@ __libelf_decompress (void *buf_in, size_t size_in, size_t size_out)
>        return NULL;
>      }
>  
> -  /* Malloc might return NULL when requestion zero size.  This is highly
> +  /* Malloc might return NULL when requesting zero size.  This is highly
>       unlikely, it would only happen when the compression was forced.
>       But we do need a non-NULL buffer to return and set as result.
>       Just make sure to always allocate at least 1 byte.  */
> @@ -260,6 +402,51 @@ __libelf_decompress (void *buf_in, size_t size_in, size_t size_out)
>    return buf_out;
>  }
>  
> +#ifdef USE_ZSTD
> +void *
> +internal_function
> +__libelf_decompress_zstd (void *buf_in, size_t size_in, size_t size_out)
> +{
> +  /* Malloc might return NULL when requesting zero size.  This is highly
> +     unlikely, it would only happen when the compression was forced.
> +     But we do need a non-NULL buffer to return and set as result.
> +     Just make sure to always allocate at least 1 byte.  */
> +  void *buf_out = malloc (size_out ?: 1);
> +  if (unlikely (buf_out == NULL))
> +    {
> +      __libelf_seterrno (ELF_E_NOMEM);
> +      return NULL;
> +    }
> +
> +  size_t ret = ZSTD_decompress (buf_out, size_out, buf_in, size_in);
> +  if (ZSTD_isError (ret))
> +    {
> +      free (buf_out);
> +      __libelf_seterrno (ELF_E_DECOMPRESS_ERROR);
> +      return NULL;
> +    }
> +  else
> +    return buf_out;
> +}
> +#endif
> +
> +void *
> +internal_function
> +__libelf_decompress (int chtype, void *buf_in, size_t size_in, size_t size_out)
> +{
> +  if (chtype == ELFCOMPRESS_ZLIB)
> +    return __libelf_decompress_zlib (buf_in, size_in, size_out);
> +  else
> +    {
> +#ifdef USE_ZSTD
> +    return __libelf_decompress_zstd (buf_in, size_in, size_out);
> +#else
> +    __libelf_seterrno (ELF_E_DECOMPRESS_ERROR);
> +    return NULL;
> +#endif
> +    }
> +}
> +
>  void *
>  internal_function
>  __libelf_decompress_elf (Elf_Scn *scn, size_t *size_out, size_t *addralign)
> @@ -268,7 +455,7 @@ __libelf_decompress_elf (Elf_Scn *scn, size_t *size_out, size_t *addralign)
>    if (gelf_getchdr (scn, &chdr) == NULL)
>      return NULL;
>  
> -  if (chdr.ch_type != ELFCOMPRESS_ZLIB)
> +  if (chdr.ch_type != ELFCOMPRESS_ZLIB && chdr.ch_type != ELFCOMPRESS_ZSTD)
>      {
>        __libelf_seterrno (ELF_E_UNKNOWN_COMPRESSION_TYPE);
>        return NULL;
> @@ -295,7 +482,9 @@ __libelf_decompress_elf (Elf_Scn *scn, size_t *size_out, size_t *addralign)
>            ? sizeof (Elf32_Chdr) : sizeof (Elf64_Chdr));
>    size_t size_in = data->d_size - hsize;
>    void *buf_in = data->d_buf + hsize;
> -  void *buf_out = __libelf_decompress (buf_in, size_in, chdr.ch_size);
> +  void *buf_out
> +    = __libelf_decompress (chdr.ch_type, buf_in, size_in, chdr.ch_size);
> +
>    *size_out = chdr.ch_size;
>    *addralign = chdr.ch_addralign;
>    return buf_out;
> @@ -394,7 +583,7 @@ elf_compress (Elf_Scn *scn, int type, unsigned int flags)
>      }
>  
>    int compressed = (sh_flags & SHF_COMPRESSED);
> -  if (type == ELFCOMPRESS_ZLIB)
> +  if (type == ELFCOMPRESS_ZLIB || type == ELFCOMPRESS_ZSTD)
>      {
>        /* Compress/Deflate.  */
>        if (compressed == 1)
> @@ -408,7 +597,8 @@ elf_compress (Elf_Scn *scn, int type, unsigned int flags)
>        size_t orig_size, orig_addralign, new_size;
>        void *out_buf = __libelf_compress (scn, hsize, elfdata,
>                       &orig_size, &orig_addralign,
> -                     &new_size, force);
> +                     &new_size, force,
> +                     type == ELFCOMPRESS_ZSTD);
>  
>        /* Compression would make section larger, don't change anything.  */
>        if (out_buf == (void *) -1)
> @@ -422,7 +612,7 @@ elf_compress (Elf_Scn *scn, int type, unsigned int flags)
>        if (elfclass == ELFCLASS32)
>      {
>        Elf32_Chdr chdr;
> -      chdr.ch_type = ELFCOMPRESS_ZLIB;
> +      chdr.ch_type = type;
>        chdr.ch_size = orig_size;
>        chdr.ch_addralign = orig_addralign;
>        if (elfdata != MY_ELFDATA)
> @@ -436,7 +626,7 @@ elf_compress (Elf_Scn *scn, int type, unsigned int flags)
>        else
>      {
>        Elf64_Chdr chdr;
> -      chdr.ch_type = ELFCOMPRESS_ZLIB;
> +      chdr.ch_type = type;
>        chdr.ch_reserved = 0;
>        chdr.ch_size = orig_size;
>        chdr.ch_addralign = sh_addralign;
> diff --git a/libelf/elf_compress_gnu.c b/libelf/elf_compress_gnu.c
> index 3d2977e7..8e20b30e 100644
> --- a/libelf/elf_compress_gnu.c
> +++ b/libelf/elf_compress_gnu.c
> @@ -103,7 +103,8 @@ elf_compress_gnu (Elf_Scn *scn, int inflate, unsigned int flags)
>        size_t orig_size, new_size, orig_addralign;
>        void *out_buf = __libelf_compress (scn, hsize, elfdata,
>                       &orig_size, &orig_addralign,
> -                     &new_size, force);
> +                     &new_size, force,
> +                     /* use_zstd */ false);
>  
>        /* Compression would make section larger, don't change anything.  */
>        if (out_buf == (void *) -1)
> @@ -178,7 +179,7 @@ elf_compress_gnu (Elf_Scn *scn, int inflate, unsigned int flags)
>        size_t size = gsize;
>        size_t size_in = data->d_size - hsize;
>        void *buf_in = data->d_buf + hsize;
> -      void *buf_out = __libelf_decompress (buf_in, size_in, size);
> +      void *buf_out = __libelf_decompress (ELFCOMPRESS_ZLIB, buf_in, size_in, size);
>        if (buf_out == NULL)
>      return -1;
>  
> diff --git a/libelf/libelfP.h b/libelf/libelfP.h
> index d88a613c..6624f38a 100644
> --- a/libelf/libelfP.h
> +++ b/libelf/libelfP.h
> @@ -574,10 +574,10 @@ extern uint32_t __libelf_crc32 (uint32_t crc, unsigned char *buf, size_t len)
>  
>  extern void * __libelf_compress (Elf_Scn *scn, size_t hsize, int ei_data,
>                   size_t *orig_size, size_t *orig_addralign,
> -                 size_t *size, bool force)
> +                 size_t *size, bool force, bool use_zstd)
>       internal_function;
>  
> -extern void * __libelf_decompress (void *buf_in, size_t size_in,
> +extern void * __libelf_decompress (int chtype, void *buf_in, size_t size_in,
>                     size_t size_out) internal_function;
>  extern void * __libelf_decompress_elf (Elf_Scn *scn,
>                         size_t *size_out, size_t *addralign)
> diff --git a/src/elfcompress.c b/src/elfcompress.c
> index eff765e8..b0f1677c 100644
> --- a/src/elfcompress.c
> +++ b/src/elfcompress.c
> @@ -55,9 +55,10 @@ enum ch_type
>    UNSET = -1,
>    NONE,
>    ZLIB,
> +  ZSTD,
>  
>    /* Maximal supported ch_type.  */
> -  MAXIMAL_CH_TYPE = ZLIB,
> +  MAXIMAL_CH_TYPE = ZSTD,
>  
>    ZLIB_GNU = 1 << 16
>  };
> @@ -139,6 +140,12 @@ parse_opt (int key, char *arg __attribute__ ((unused)),
>      type = ZLIB;
>        else if (strcmp ("zlib-gnu", arg) == 0 || strcmp ("gnu", arg) == 0)
>      type = ZLIB_GNU;
> +      else if (strcmp ("zstd", arg) == 0)
> +#ifdef USE_ZSTD
> +    type = ZSTD;
> +#else
> +    argp_error (state, N_("ZSTD support is not enabled"));
> +#endif
>        else
>      argp_error (state, N_("unknown compression type '%s'"), arg);
>        break;
> @@ -281,6 +288,44 @@ get_sections (unsigned int *sections, size_t shnum)
>    return s;
>  }
>  
> +/* Return compression type of a given section SHDR.  */
> +
> +static enum ch_type
> +get_section_chtype (Elf_Scn *scn, GElf_Shdr *shdr, const char *sname,
> +            size_t ndx)
> +{
> +  enum ch_type chtype = UNSET;
> +  if ((shdr->sh_flags & SHF_COMPRESSED) != 0)
> +    {
> +      GElf_Chdr chdr;
> +      if (gelf_getchdr (scn, &chdr) != NULL)
> +    {
> +      chtype = (enum ch_type)chdr.ch_type;
> +      if (chtype == NONE)
> +        {
> +          error (0, 0, "Compression type for section %zd"
> +             " can't be zero ", ndx);
> +          chtype = UNSET;
> +        }
> +      else if (chtype > MAXIMAL_CH_TYPE)
> +        {
> +          error (0, 0, "Compression type (%d) for section %zd"
> +             " is unsupported ", chtype, ndx);
> +          chtype = UNSET;
> +        }
> +    }
> +      else
> +    error (0, 0, "Couldn't get chdr for section %zd", ndx);
> +    }
> +  /* Set ZLIB_GNU compression manually for .zdebug* sections.  */
> +  else if (startswith (sname, ".zdebug"))
> +    chtype = ZLIB_GNU;
> +  else
> +    chtype = NONE;
> +
> +  return chtype;
> +}
> +
>  static int
>  process_file (const char *fname)
>  {
> @@ -461,26 +506,29 @@ process_file (const char *fname)
>  
>        if (section_name_matches (sname))
>      {
> -      if (!force && type == NONE
> -          && (shdr->sh_flags & SHF_COMPRESSED) == 0
> -          && !startswith (sname, ".zdebug"))
> -        {
> -          if (verbose > 0)
> -        printf ("[%zd] %s already decompressed\n", ndx, sname);
> -        }
> -      else if (!force && type == ZLIB
> -           && (shdr->sh_flags & SHF_COMPRESSED) != 0)
> -        {
> -          if (verbose > 0)
> -        printf ("[%zd] %s already compressed\n", ndx, sname);
> -        }
> -      else if (!force && type == ZLIB_GNU
> -           && startswith (sname, ".zdebug"))
> +      enum ch_type schtype = get_section_chtype (scn, shdr, sname, ndx);
> +      if (!force && verbose > 0)
>          {
> -          if (verbose > 0)
> -        printf ("[%zd] %s already GNU compressed\n", ndx, sname);
> +          /* The current compression matches the final one.  */
> +          if (type == schtype)
> +        switch (type)
> +          {
> +          case NONE:
> +            printf ("[%zd] %s already decompressed\n", ndx, sname);
> +            break;
> +          case ZLIB:
> +          case ZSTD:
> +            printf ("[%zd] %s already compressed\n", ndx, sname);
> +            break;
> +          case ZLIB_GNU:
> +            printf ("[%zd] %s already GNU compressed\n", ndx, sname);
> +            break;
> +          default:
> +            abort ();
> +          }
>          }
> -      else if (shdr->sh_type != SHT_NOBITS
> +
> +      if (shdr->sh_type != SHT_NOBITS
>            && (shdr->sh_flags & SHF_ALLOC) == 0)
>          {
>            set_section (sections, ndx);
> @@ -692,37 +740,12 @@ process_file (const char *fname)
>           (de)compressed, invalidating the string pointers.  */
>        sname = xstrdup (sname);
>  
> +
>        /* Detect source compression that is how is the section compressed
>           now.  */
> -      GElf_Chdr chdr;
> -      enum ch_type schtype = NONE;
> -      if ((shdr->sh_flags & SHF_COMPRESSED) != 0)
> -        {
> -          if (gelf_getchdr (scn, &chdr) != NULL)
> -        {
> -          schtype = (enum ch_type)chdr.ch_type;
> -          if (schtype == NONE)
> -            {
> -              error (0, 0, "Compression type for section %zd"
> -                 " can't be zero ", ndx);
> -              goto cleanup;
> -            }
> -          else if (schtype > MAXIMAL_CH_TYPE)
> -            {
> -              error (0, 0, "Compression type (%d) for section %zd"
> -                 " is unsupported ", schtype, ndx);
> -              goto cleanup;
> -            }
> -        }
> -          else
> -        {
> -          error (0, 0, "Couldn't get chdr for section %zd", ndx);
> -          goto cleanup;
> -        }
> -        }
> -      /* Set ZLIB compression manually for .zdebug* sections.  */
> -      else if (startswith (sname, ".zdebug"))
> -        schtype = ZLIB_GNU;
> +      enum ch_type schtype = get_section_chtype (scn, shdr, sname, ndx);
> +      if (schtype == UNSET)
> +        goto cleanup;
>  
>        /* We might want to decompress (and rename), but not
>           compress during this pass since we might need the section
> @@ -754,7 +777,7 @@ process_file (const char *fname)
>          case ZLIB_GNU:
>            if (startswith (sname, ".debug"))
>          {
> -          if (schtype == ZLIB)
> +          if (schtype == ZLIB || schtype == ZSTD)
>              {
>                /* First decompress to recompress GNU style.
>               Don't report even when verbose.  */
> @@ -818,19 +841,22 @@ process_file (const char *fname)
>            break;
>  
>          case ZLIB:
> -          if ((shdr->sh_flags & SHF_COMPRESSED) == 0)
> +        case ZSTD:
> +          if (schtype != type)
>          {
> -          if (schtype == ZLIB_GNU)
> +          if (schtype != NONE)
>              {
> -              /* First decompress to recompress zlib style.
> -             Don't report even when verbose.  */
> +              /* Decompress first.  */
>                if (compress_section (scn, size, sname, NULL, ndx,
>                          schtype, NONE, false) < 0)
>              goto cleanup;
>  
> -              snamebuf[0] = '.';
> -              strcpy (&snamebuf[1], &sname[2]);
> -              newname = snamebuf;
> +              if (schtype == ZLIB_GNU)
> +            {
> +              snamebuf[0] = '.';
> +              strcpy (&snamebuf[1], &sname[2]);
> +              newname = snamebuf;
> +            }
>              }
>  
>            if (skip_compress_section)
> @@ -838,7 +864,7 @@ process_file (const char *fname)
>                if (ndx == shdrstrndx)
>              {
>                shstrtab_size = size;
> -              shstrtab_compressed = ZLIB;
> +              shstrtab_compressed = type;
>                if (shstrtab_name != NULL
>                    || shstrtab_newname != NULL)
>                  {
> @@ -855,7 +881,7 @@ process_file (const char *fname)
>                else
>              {
>                symtab_size = size;
> -              symtab_compressed = ZLIB;
> +              symtab_compressed = type;
>                symtab_name = xstrdup (sname);
>                symtab_newname = (newname == NULL
>                          ? NULL : xstrdup (newname));
> @@ -1378,7 +1404,7 @@ main (int argc, char **argv)
>      N_("Place (de)compressed output into FILE"),
>      0 },
>        { "type", 't', "TYPE", 0,
> -    N_("What type of compression to apply. TYPE can be 'none' (decompress), 'zlib' (ELF ZLIB compression, the default, 'zlib-gabi' is an alias) or 'zlib-gnu' (.zdebug GNU style compression, 'gnu' is an alias)"),
> +    N_("What type of compression to apply. TYPE can be 'none' (decompress), 'zlib' (ELF ZLIB compression, the default, 'zlib-gabi' is an alias), 'zlib-gnu' (.zdebug GNU style compression, 'gnu' is an alias) or 'zstd'"),
>      0 },
>        { "name", 'n', "SECTION", 0,
>      N_("SECTION name to (de)compress, SECTION is an extended wildcard pattern (defaults to '.?(z)debug*')"),
> diff --git a/src/readelf.c b/src/readelf.c
> index cc3e0229..451f8400 100644
> --- a/src/readelf.c
> +++ b/src/readelf.c
> @@ -1238,13 +1238,17 @@ get_visibility_type (int value)
>  static const char *
>  elf_ch_type_name (unsigned int code)
>  {
> -  if (code == 0)
> -    return "NONE";
> -
> -  if (code == ELFCOMPRESS_ZLIB)
> -    return "ZLIB";
> -
> -  return "UNKNOWN";
> +  switch (code)
> +    {
> +    case 0:
> +      return "NONE";
> +    case ELFCOMPRESS_ZLIB:
> +      return "ZLIB";
> +    case ELFCOMPRESS_ZSTD:
> +      return "ZSTD";
> +    default:
> +      return "UNKNOWN";
> +    }
>  }
>  
>  /* Print the section headers.  */
> diff --git a/tests/run-compress-test.sh b/tests/run-compress-test.sh
> index a6a298f5..3f9c990e 100755
> --- a/tests/run-compress-test.sh
> +++ b/tests/run-compress-test.sh
> @@ -61,6 +61,30 @@ testrun_elfcompress_file()
>      echo "uncompress $elfcompressedfile -> $elfuncompressedfile"
>      testrun ${abs_top_builddir}/src/elfcompress -v -t none -o ${elfuncompressedfile} ${elfcompressedfile}
>      testrun ${abs_top_builddir}/src/elfcmp ${uncompressedfile} ${elfuncompressedfile}
> +
> +    outputfile="${infile}.gabi.zstd"
> +    tempfiles "$outputfile"
> +    echo "zstd compress $elfcompressedfile -> $outputfile"
> +    testrun ${abs_top_builddir}/src/elfcompress -v -t zstd -o ${outputfile} ${elfcompressedfile}
> +    testrun ${abs_top_builddir}/src/elfcmp ${uncompressedfile} ${outputfile}
> +    echo "checking compressed section header" $outputfile
> +    testrun ${abs_top_builddir}/src/readelf -Sz ${outputfile} | grep "ELF ZSTD" >/dev/null
> +
> +    zstdfile="${infile}.zstd"
> +    tempfiles "$zstdfile"
> +    echo "zstd compress $uncompressedfile -> $zstdfile"
> +    testrun ${abs_top_builddir}/src/elfcompress -v -t zstd -o ${zstdfile} ${elfuncompressedfile}
> +    testrun ${abs_top_builddir}/src/elfcmp ${uncompressedfile} ${zstdfile}
> +    echo "checking compressed section header" $zstdfile
> +    testrun ${abs_top_builddir}/src/readelf -Sz ${zstdfile} | grep "ELF ZSTD" >/dev/null
> +
> +    zstdgnufile="${infile}.zstd.gnu"
> +    tempfiles "$zstdgnufile"
> +    echo "zstd re-compress to GNU ZLIB $zstdfile -> $zstdgnufile"
> +    testrun ${abs_top_builddir}/src/elfcompress -v -t zlib-gnu -o ${zstdgnufile} ${zstdfile}
> +    testrun ${abs_top_builddir}/src/elfcmp ${uncompressedfile} ${zstdgnufile}
> +    echo "checking .zdebug section name" $zstdgnufile
> +    testrun ${abs_top_builddir}/src/readelf -S ${zstdgnufile} | grep ".zdebug" >/dev/null
>  }
>  
>  testrun_elfcompress()
  
Mark Wielaard Dec. 15, 2022, 1:17 p.m. UTC | #2
Hi Martin,

On Tue, 2022-11-29 at 13:05 +0100, Martin Liška wrote:
> There's second version of the patch that fully support both
> compression and decompression.
> 
> Changes from the v1:
> - compression support added
> - zstd detection is fixed
> - new tests are added
> - builds fine w/ and w/o the ZSTD library
> 
> What's currently missing and where I need a help:
> 
> 1) When I build ./configure --without-zstd, I don't have
> a reasonable error message (something similar to binutils's readelf:
> readelf: Warning: section '.debug_str' has unsupported compress type:
> 2)
> even though, __libelf_decompress returns NULL and __libelf_seterrno).
> One can see a garbage in the console.
> 
> How to handle that properly?

Is there a particular way you are running eu-readelf? Is it with
generic -w or -a, or decoding a specific section type?

> 2) How should I run my newly added tests conditionally if zstd
> configuration support is enabled?

eu_ZIP will define an AM_CONDITIONAL for ZSTD (note this is different
from the HAVE_ZSTD, which is the am conditional added for having the
zstd program itself). You could use it in tests/Makefile.am as:

  if ZSTD
  TESTS += run-zstd-compress-test.sh
  endif

If you had a separate test... Otherwise add some variable to
TESTS_ENVIRONMENT (and installed_TESTS_ENVIRONMENT) based on the Make
variable that is tested inside the shell script (somewhat like
USE_VALGRIND) maybe.

>   configure.ac               |   8 +-
>   libelf/Makefile.am         |   2 +-
>   libelf/elf_compress.c      | 294 ++++++++++++++++++++++++++++++--
> -----
>   libelf/elf_compress_gnu.c  |   5 +-
>   libelf/libelfP.h           |   4 +-
>   src/elfcompress.c          | 144 ++++++++++--------
>   src/readelf.c              |  18 ++-
>   tests/run-compress-test.sh |  24 +++
>   8 files changed, 373 insertions(+), 126 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 59be27ac..07cfa54b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -410,6 +410,11 @@ dnl Test for bzlib and xz/lzma/zstd, gives BZLIB/LZMALIB/ZSTD .am
>   dnl conditional and config.h USE_BZLIB/USE_LZMALIB/USE_ZSTD #define.
>   save_LIBS="$LIBS"
>   LIBS=
> +eu_ZIPLIB(zstd,ZSTD,zstd,ZSTD_decompress,[ZSTD (zst)])
> +AS_IF([test "x$with_zstd" = xyes], [LIBZSTD="libzstd"], [LIBLZSTD=""])
> +AC_SUBST([LIBZSTD])
> +zstd_LIBS="$LIBS"
> +AC_SUBST([zstd_LIBS])
>   eu_ZIPLIB(bzlib,BZLIB,bz2,BZ2_bzdopen,bzip2)
>   # We need this since bzip2 doesn't have a pkgconfig file.
>   BZ2_LIB="$LIBS"
> @@ -417,9 +422,6 @@ AC_SUBST([BZ2_LIB])
>   eu_ZIPLIB(lzma,LZMA,lzma,lzma_auto_decoder,[LZMA (xz)])
>   AS_IF([test "x$with_lzma" = xyes], [LIBLZMA="liblzma"], [LIBLZMA=""])
>   AC_SUBST([LIBLZMA])
> -eu_ZIPLIB(zstd,ZSTD,zstd,ZSTD_decompress,[ZSTD (zst)])
> -AS_IF([test "x$with_zstd" = xyes], [LIBZSTD="libzstd"], [LIBLZSTD=""])
> -AC_SUBST([LIBZSTD])
>   zip_LIBS="$LIBS"
>   LIBS="$save_LIBS"
>   AC_SUBST([zip_LIBS])

Doing AC_SUBST([zstd_LIBS]) seems correct. Why is the test moved
earlier?

You are testing for ZSTD_decompress, is that enough? Asking because I
see you are using ZSTD_compressStream2, which seems to requires libzstd
v1.4.0+.

In general how stable is the libzstd api?

You'll also need to use the AC_SUBST for LIBZSTD in config/libelf.pc.in
now, as used in the libdw.pc.in:

  Requires.private: zlib @LIBZSTD@

> diff --git a/libelf/Makefile.am b/libelf/Makefile.am
> index 560ed45f..24c25cf8 100644
> --- a/libelf/Makefile.am
> +++ b/libelf/Makefile.am
> @@ -106,7 +106,7 @@ libelf_pic_a_SOURCES =
>   am_libelf_pic_a_OBJECTS = $(libelf_a_SOURCES:.c=.os)
>   
>   libelf_so_DEPS = ../lib/libeu.a
> -libelf_so_LDLIBS = $(libelf_so_DEPS) -lz
> +libelf_so_LDLIBS = $(libelf_so_DEPS) -lz $(zstd_LIBS)
>   if USE_LOCKS
>   libelf_so_LDLIBS += -lpthread
>   endif

OK.

Haven't read the actual code yet. I'll get back to that later today.

Cheers,

Mark
  
Mark Wielaard Dec. 16, 2022, 12:32 a.m. UTC | #3
Hi Martin,

On Tue, Nov 29, 2022 at 01:05:45PM +0100, Martin Liška wrote:
> diff --git a/libelf/elf_compress.c b/libelf/elf_compress.c
> index d7f53af2..7a6e37a4 100644
> --- a/libelf/elf_compress.c
> +++ b/libelf/elf_compress.c
> @@ -39,6 +39,10 @@
>  #include <string.h>
>  #include <zlib.h>
> +#ifdef USE_ZSTD
> +#include <zstd.h>
> +#endif
> +
>  /* Cleanup and return result.  Don't leak memory.  */
>  static void *
>  do_deflate_cleanup (void *result, z_stream *z, void *out_buf,

OK.

> @@ -54,53 +58,14 @@ do_deflate_cleanup (void *result, z_stream *z, void *out_buf,
>  #define deflate_cleanup(result, cdata) \
>      do_deflate_cleanup(result, &z, out_buf, cdata)
> -/* Given a section, uses the (in-memory) Elf_Data to extract the
> -   original data size (including the given header size) and data
> -   alignment.  Returns a buffer that has at least hsize bytes (for the
> -   caller to fill in with a header) plus zlib compressed date.  Also
> -   returns the new buffer size in new_size (hsize + compressed data
> -   size).  Returns (void *) -1 when FORCE is false and the compressed
> -   data would be bigger than the original data.  */
>  void *
>  internal_function
> -__libelf_compress (Elf_Scn *scn, size_t hsize, int ei_data,
> -		   size_t *orig_size, size_t *orig_addralign,
> -		   size_t *new_size, bool force)
> +__libelf_compress_zlib (Elf_Scn *scn, size_t hsize, int ei_data,
> +			size_t *orig_size, size_t *orig_addralign,
> +			size_t *new_size, bool force,
> +			Elf_Data *data, Elf_Data *next_data,
> +			void *out_buf, size_t out_size, size_t block)
>  {
> -  /* The compressed data is the on-disk data.  We simplify the
> -     implementation a bit by asking for the (converted) in-memory
> -     data (which might be all there is if the user created it with
> -     elf_newdata) and then convert back to raw if needed before
> -     compressing.  Should be made a bit more clever to directly
> -     use raw if that is directly available.  */
> -  Elf_Data *data = elf_getdata (scn, NULL);
> -  if (data == NULL)
> -    return NULL;
> -
> -  /* When not forced and we immediately know we would use more data by
> -     compressing, because of the header plus zlib overhead (five bytes
> -     per 16 KB block, plus a one-time overhead of six bytes for the
> -     entire stream), don't do anything.  */
> -  Elf_Data *next_data = elf_getdata (scn, data);
> -  if (next_data == NULL && !force
> -      && data->d_size <= hsize + 5 + 6)
> -    return (void *) -1;
> -
> -  *orig_addralign = data->d_align;
> -  *orig_size = data->d_size;
> -
> -  /* Guess an output block size. 1/8th of the original Elf_Data plus
> -     hsize.  Make the first chunk twice that size (25%), then increase
> -     by a block (12.5%) when necessary.  */
> -  size_t block = (data->d_size / 8) + hsize;
> -  size_t out_size = 2 * block;
> -  void *out_buf = malloc (out_size);
> -  if (out_buf == NULL)
> -    {
> -      __libelf_seterrno (ELF_E_NOMEM);
> -      return NULL;
> -    }
> -
>    /* Caller gets to fill in the header at the start.  Just skip it here.  */
>    size_t used = hsize;

OK, all this is moved lower and then calls either
__libelf_compress_zlib or __libelf_compress_zstd.

> @@ -205,9 +170,186 @@ __libelf_compress (Elf_Scn *scn, size_t hsize, int ei_data,
>    return out_buf;
>  }
> +#ifdef USE_ZSTD
> +/* Cleanup and return result.  Don't leak memory.  */
> +static void *
> +do_zstd_cleanup (void *result, ZSTD_CCtx * const cctx, void *out_buf,
> +		 Elf_Data *cdatap)
> +{
> +  ZSTD_freeCCtx (cctx);
> +  free (out_buf);
> +  if (cdatap != NULL)
> +    free (cdatap->d_buf);
> +  return result;
> +}
>
> +#define zstd_cleanup(result, cdata) \
> +    do_zstd_cleanup(result, cctx, out_buf, cdata)
> +

OK, mimics do_deflate_cleanup.

>  void *
>  internal_function
> -__libelf_decompress (void *buf_in, size_t size_in, size_t size_out)
> +__libelf_compress_zstd (Elf_Scn *scn, size_t hsize, int ei_data,
> +			size_t *orig_size, size_t *orig_addralign,
> +			size_t *new_size, bool force,
> +			Elf_Data *data, Elf_Data *next_data,
> +			void *out_buf, size_t out_size, size_t block)
> +{
> +  /* Caller gets to fill in the header at the start.  Just skip it here.  */
> +  size_t used = hsize;
> +
> +  ZSTD_CCtx* const cctx = ZSTD_createCCtx();
> +  Elf_Data cdata;
> +  cdata.d_buf = NULL;
> +
> +  /* Loop over data buffers.  */
> +  ZSTD_EndDirective mode = ZSTD_e_continue;
> +
> +  do
> +    {
> +      /* Convert to raw if different endianness.  */
> +      cdata = *data;
> +      bool convert = ei_data != MY_ELFDATA && data->d_size > 0;
> +      if (convert)
> +	{
> +	  /* Don't do this conversion in place, we might want to keep
> +	     the original data around, caller decides.  */
> +	  cdata.d_buf = malloc (data->d_size);
> +	  if (cdata.d_buf == NULL)
> +	    {
> +	      __libelf_seterrno (ELF_E_NOMEM);
> +	      return zstd_cleanup (NULL, NULL);
> +	    }
> +	  if (gelf_xlatetof (scn->elf, &cdata, data, ei_data) == NULL)
> +	    return zstd_cleanup (NULL, &cdata);
> +	}
> +
> +      ZSTD_inBuffer ib = { cdata.d_buf, cdata.d_size, 0 };
> +
> +      /* Get next buffer to see if this is the last one.  */
> +      data = next_data;
> +      if (data != NULL)
> +	{
> +	  *orig_addralign = MAX (*orig_addralign, data->d_align);
> +	  *orig_size += data->d_size;
> +	  next_data = elf_getdata (scn, data);
> +	}
> +      else
> +	mode = ZSTD_e_end;
> +
> +      /* Flush one data buffer.  */
> +      for (;;)
> +	{
> +	  ZSTD_outBuffer ob = { out_buf + used, out_size - used, 0 };
> +	  size_t ret = ZSTD_compressStream2 (cctx, &ob, &ib, mode);
> +	  if (ZSTD_isError (ret))
> +	    {
> +	      __libelf_seterrno (ELF_E_COMPRESS_ERROR);
> +	      return zstd_cleanup (NULL, convert ? &cdata : NULL);
> +	    }
> +	  used += ob.pos;
> +
> +	  /* Bail out if we are sure the user doesn't want the
> +	     compression forced and we are using more compressed data
> +	     than original data.  */
> +	  if (!force && mode == ZSTD_e_end && used >= *orig_size)
> +	    return zstd_cleanup ((void *) -1, convert ? &cdata : NULL);
> +
> +	  if (ret > 0)
> +	    {
> +	      void *bigger = realloc (out_buf, out_size + block);
> +	      if (bigger == NULL)
> +		{
> +		  __libelf_seterrno (ELF_E_NOMEM);
> +		  return zstd_cleanup (NULL, convert ? &cdata : NULL);
> +		}

OK, zstd_cleanup does also free out_buf.

> +	      out_buf = bigger;
> +	      out_size += block;
> +	    }
> +	  else
> +	    break;
> +	}
> +
> +      if (convert)
> +	{
> +	  free (cdata.d_buf);
> +	  cdata.d_buf = NULL;
> +	}
> +    }
> +  while (mode != ZSTD_e_end); /* More data blocks.  */
> +
> +  ZSTD_freeCCtx (cctx);
> +  *new_size = used;
> +  return out_buf;
> +}
> +#endif

Look good.

> +/* Given a section, uses the (in-memory) Elf_Data to extract the
> +   original data size (including the given header size) and data
> +   alignment.  Returns a buffer that has at least hsize bytes (for the
> +   caller to fill in with a header) plus zlib compressed date.  Also
> +   returns the new buffer size in new_size (hsize + compressed data
> +   size).  Returns (void *) -1 when FORCE is false and the compressed
> +   data would be bigger than the original data.  */
> +void *
> +internal_function
> +__libelf_compress (Elf_Scn *scn, size_t hsize, int ei_data,
> +		   size_t *orig_size, size_t *orig_addralign,
> +		   size_t *new_size, bool force, bool use_zstd)
> +{
> +  /* The compressed data is the on-disk data.  We simplify the
> +     implementation a bit by asking for the (converted) in-memory
> +     data (which might be all there is if the user created it with
> +     elf_newdata) and then convert back to raw if needed before
> +     compressing.  Should be made a bit more clever to directly
> +     use raw if that is directly available.  */
> +  Elf_Data *data = elf_getdata (scn, NULL);
> +  if (data == NULL)
> +    return NULL;
> +
> +  /* When not forced and we immediately know we would use more data by
> +     compressing, because of the header plus zlib overhead (five bytes
> +     per 16 KB block, plus a one-time overhead of six bytes for the
> +     entire stream), don't do anything.
> +     Size estimation for ZSTD compression would be similar.  */
> +  Elf_Data *next_data = elf_getdata (scn, data);
> +  if (next_data == NULL && !force
> +      && data->d_size <= hsize + 5 + 6)
> +    return (void *) -1;
> +
> +  *orig_addralign = data->d_align;
> +  *orig_size = data->d_size;
> +
> +  /* Guess an output block size. 1/8th of the original Elf_Data plus
> +     hsize.  Make the first chunk twice that size (25%), then increase
> +     by a block (12.5%) when necessary.  */
> +  size_t block = (data->d_size / 8) + hsize;
> +  size_t out_size = 2 * block;
> +  void *out_buf = malloc (out_size);
> +  if (out_buf == NULL)
> +    {
> +      __libelf_seterrno (ELF_E_NOMEM);
> +      return NULL;
> +    }

OK, this is all just moved from earlier with an extra use_zstd flag.

> +  if (use_zstd)
> +#ifdef USE_ZSTD
> +    return __libelf_compress_zstd (scn, hsize, ei_data, orig_size,
> +				   orig_addralign, new_size, force,
> +				   data, next_data, out_buf, out_size,
> +				   block);
> +#else
> +    return NULL;

Should this also call __libelf_seterrno (ELF_E_UNKNOWN_COMPRESSION_TYPE) ?

> +#endif
> +  else
> +    return __libelf_compress_zlib (scn, hsize, ei_data, orig_size,
> +				   orig_addralign, new_size, force,
> +				   data, next_data, out_buf, out_size,
> +				   block);
> +}
> +
> +void *
> +internal_function
> +__libelf_decompress_zlib (void *buf_in, size_t size_in, size_t size_out)
>  {
>    /* Catch highly unlikely compression ratios so we don't allocate
>       some giant amount of memory for nothing. The max compression
> @@ -218,7 +360,7 @@ __libelf_decompress (void *buf_in, size_t size_in, size_t size_out)
>        return NULL;
>      }
> -  /* Malloc might return NULL when requestion zero size.  This is highly
> +  /* Malloc might return NULL when requesting zero size.  This is highly
>       unlikely, it would only happen when the compression was forced.
>       But we do need a non-NULL buffer to return and set as result.
>       Just make sure to always allocate at least 1 byte.  */

OK, typo fix.

> @@ -260,6 +402,51 @@ __libelf_decompress (void *buf_in, size_t size_in, size_t size_out)
>    return buf_out;
>  }
> +#ifdef USE_ZSTD
> +void *
> +internal_function
> +__libelf_decompress_zstd (void *buf_in, size_t size_in, size_t size_out)
> +{
> +  /* Malloc might return NULL when requesting zero size.  This is highly
> +     unlikely, it would only happen when the compression was forced.
> +     But we do need a non-NULL buffer to return and set as result.
> +     Just make sure to always allocate at least 1 byte.  */
> +  void *buf_out = malloc (size_out ?: 1);
> +  if (unlikely (buf_out == NULL))
> +    {
> +      __libelf_seterrno (ELF_E_NOMEM);
> +      return NULL;
> +    }
> +
> +  size_t ret = ZSTD_decompress (buf_out, size_out, buf_in, size_in);
> +  if (ZSTD_isError (ret))
> +    {
> +      free (buf_out);
> +      __libelf_seterrno (ELF_E_DECOMPRESS_ERROR);
> +      return NULL;
> +    }
> +  else
> +    return buf_out;
> +}
> +#endif

OK.

> +void *
> +internal_function
> +__libelf_decompress (int chtype, void *buf_in, size_t size_in, size_t size_out)
> +{
> +  if (chtype == ELFCOMPRESS_ZLIB)
> +    return __libelf_decompress_zlib (buf_in, size_in, size_out);
> +  else
> +    {
> +#ifdef USE_ZSTD
> +    return __libelf_decompress_zstd (buf_in, size_in, size_out);
> +#else
> +    __libelf_seterrno (ELF_E_DECOMPRESS_ERROR);

Maybe ELF_E_UNKNOWN_COMPRESSION_TYPE ?

> +    return NULL;
> +#endif
> +    }
> +}
> +
>  void *
>  internal_function
>  __libelf_decompress_elf (Elf_Scn *scn, size_t *size_out, size_t *addralign)
> @@ -268,7 +455,7 @@ __libelf_decompress_elf (Elf_Scn *scn, size_t *size_out, size_t *addralign)
>    if (gelf_getchdr (scn, &chdr) == NULL)
>      return NULL;
> -  if (chdr.ch_type != ELFCOMPRESS_ZLIB)
> +  if (chdr.ch_type != ELFCOMPRESS_ZLIB && chdr.ch_type != ELFCOMPRESS_ZSTD)
>      {
>        __libelf_seterrno (ELF_E_UNKNOWN_COMPRESSION_TYPE);
>        return NULL;

Should the chdr.ch_type != ELFCOMPRESS_ZSTD be guarded by #ifdef USE_ZSTD ?

> @@ -295,7 +482,9 @@ __libelf_decompress_elf (Elf_Scn *scn, size_t *size_out, size_t *addralign)
>  		  ? sizeof (Elf32_Chdr) : sizeof (Elf64_Chdr));
>    size_t size_in = data->d_size - hsize;
>    void *buf_in = data->d_buf + hsize;
> -  void *buf_out = __libelf_decompress (buf_in, size_in, chdr.ch_size);
> +  void *buf_out
> +    = __libelf_decompress (chdr.ch_type, buf_in, size_in, chdr.ch_size);
> +
>    *size_out = chdr.ch_size;
>    *addralign = chdr.ch_addralign;
>    return buf_out;
> @@ -394,7 +583,7 @@ elf_compress (Elf_Scn *scn, int type, unsigned int flags)
>      }
>    int compressed = (sh_flags & SHF_COMPRESSED);
> -  if (type == ELFCOMPRESS_ZLIB)
> +  if (type == ELFCOMPRESS_ZLIB || type == ELFCOMPRESS_ZSTD)
>      {
>        /* Compress/Deflate.  */
>        if (compressed == 1)
> @@ -408,7 +597,8 @@ elf_compress (Elf_Scn *scn, int type, unsigned int flags)
>        size_t orig_size, orig_addralign, new_size;
>        void *out_buf = __libelf_compress (scn, hsize, elfdata,
>  					 &orig_size, &orig_addralign,
> -					 &new_size, force);
> +					 &new_size, force,
> +					 type == ELFCOMPRESS_ZSTD);
>        /* Compression would make section larger, don't change anything.  */
>        if (out_buf == (void *) -1)
> @@ -422,7 +612,7 @@ elf_compress (Elf_Scn *scn, int type, unsigned int flags)
>        if (elfclass == ELFCLASS32)
>  	{
>  	  Elf32_Chdr chdr;
> -	  chdr.ch_type = ELFCOMPRESS_ZLIB;
> +	  chdr.ch_type = type;
>  	  chdr.ch_size = orig_size;
>  	  chdr.ch_addralign = orig_addralign;
>  	  if (elfdata != MY_ELFDATA)
> @@ -436,7 +626,7 @@ elf_compress (Elf_Scn *scn, int type, unsigned int flags)
>        else
>  	{
>  	  Elf64_Chdr chdr;
> -	  chdr.ch_type = ELFCOMPRESS_ZLIB;
> +	  chdr.ch_type = type;
>  	  chdr.ch_reserved = 0;
>  	  chdr.ch_size = orig_size;
>  	  chdr.ch_addralign = sh_addralign;

OK.

> diff --git a/libelf/elf_compress_gnu.c b/libelf/elf_compress_gnu.c
> index 3d2977e7..8e20b30e 100644
> --- a/libelf/elf_compress_gnu.c
> +++ b/libelf/elf_compress_gnu.c
> @@ -103,7 +103,8 @@ elf_compress_gnu (Elf_Scn *scn, int inflate, unsigned int flags)
>        size_t orig_size, new_size, orig_addralign;
>        void *out_buf = __libelf_compress (scn, hsize, elfdata,
>  					 &orig_size, &orig_addralign,
> -					 &new_size, force);
> +					 &new_size, force,
> +					 /* use_zstd */ false);
>        /* Compression would make section larger, don't change anything.  */
>        if (out_buf == (void *) -1)

OK.

> @@ -178,7 +179,7 @@ elf_compress_gnu (Elf_Scn *scn, int inflate, unsigned int flags)
>        size_t size = gsize;
>        size_t size_in = data->d_size - hsize;
>        void *buf_in = data->d_buf + hsize;
> -      void *buf_out = __libelf_decompress (buf_in, size_in, size);
> +      void *buf_out = __libelf_decompress (ELFCOMPRESS_ZLIB, buf_in, size_in, size);
>        if (buf_out == NULL)
>  	return -1;

OK.

> diff --git a/libelf/libelfP.h b/libelf/libelfP.h
> index d88a613c..6624f38a 100644
> --- a/libelf/libelfP.h
> +++ b/libelf/libelfP.h
> @@ -574,10 +574,10 @@ extern uint32_t __libelf_crc32 (uint32_t crc, unsigned char *buf, size_t len)
>  extern void * __libelf_compress (Elf_Scn *scn, size_t hsize, int ei_data,
>  				 size_t *orig_size, size_t *orig_addralign,
> -				 size_t *size, bool force)
> +				 size_t *size, bool force, bool use_zstd)
>       internal_function;
> -extern void * __libelf_decompress (void *buf_in, size_t size_in,
> +extern void * __libelf_decompress (int chtype, void *buf_in, size_t size_in,
>  				   size_t size_out) internal_function;
>  extern void * __libelf_decompress_elf (Elf_Scn *scn,
>  				       size_t *size_out, size_t *addralign)

OK.

> diff --git a/src/elfcompress.c b/src/elfcompress.c
> index eff765e8..b0f1677c 100644
> --- a/src/elfcompress.c
> +++ b/src/elfcompress.c
> @@ -55,9 +55,10 @@ enum ch_type
>    UNSET = -1,
>    NONE,
>    ZLIB,
> +  ZSTD,
>    /* Maximal supported ch_type.  */
> -  MAXIMAL_CH_TYPE = ZLIB,
> +  MAXIMAL_CH_TYPE = ZSTD,
>    ZLIB_GNU = 1 << 16
>  };

OK.

> @@ -139,6 +140,12 @@ parse_opt (int key, char *arg __attribute__ ((unused)),
>  	type = ZLIB;
>        else if (strcmp ("zlib-gnu", arg) == 0 || strcmp ("gnu", arg) == 0)
>  	type = ZLIB_GNU;
> +      else if (strcmp ("zstd", arg) == 0)
> +#ifdef USE_ZSTD
> +	type = ZSTD;
> +#else
> +	argp_error (state, N_("ZSTD support is not enabled"));
> +#endif
>        else
>  	argp_error (state, N_("unknown compression type '%s'"), arg);
>        break;

Nice error handling.

> @@ -281,6 +288,44 @@ get_sections (unsigned int *sections, size_t shnum)
>    return s;
>  }
> +/* Return compression type of a given section SHDR.  */
> +
> +static enum ch_type
> +get_section_chtype (Elf_Scn *scn, GElf_Shdr *shdr, const char *sname,
> +		    size_t ndx)
> +{
> +  enum ch_type chtype = UNSET;
> +  if ((shdr->sh_flags & SHF_COMPRESSED) != 0)
> +    {
> +      GElf_Chdr chdr;
> +      if (gelf_getchdr (scn, &chdr) != NULL)
> +	{
> +	  chtype = (enum ch_type)chdr.ch_type;
> +	  if (chtype == NONE)
> +	    {
> +	      error (0, 0, "Compression type for section %zd"
> +		     " can't be zero ", ndx);
> +	      chtype = UNSET;
> +	    }
> +	  else if (chtype > MAXIMAL_CH_TYPE)
> +	    {
> +	      error (0, 0, "Compression type (%d) for section %zd"
> +		     " is unsupported ", chtype, ndx);
> +	      chtype = UNSET;
> +	    }
> +	}
> +      else
> +	error (0, 0, "Couldn't get chdr for section %zd", ndx);

Shouldn't this error be fatal?

> +    }
> +  /* Set ZLIB_GNU compression manually for .zdebug* sections.  */
> +  else if (startswith (sname, ".zdebug"))
> +    chtype = ZLIB_GNU;
> +  else
> +    chtype = NONE;
> +
> +  return chtype;
> +}
> +
>  static int
>  process_file (const char *fname)
>  {
> @@ -461,26 +506,29 @@ process_file (const char *fname)
>        if (section_name_matches (sname))
>  	{
> -	  if (!force && type == NONE
> -	      && (shdr->sh_flags & SHF_COMPRESSED) == 0
> -	      && !startswith (sname, ".zdebug"))
> -	    {
> -	      if (verbose > 0)
> -		printf ("[%zd] %s already decompressed\n", ndx, sname);
> -	    }
> -	  else if (!force && type == ZLIB
> -		   && (shdr->sh_flags & SHF_COMPRESSED) != 0)
> -	    {
> -	      if (verbose > 0)
> -		printf ("[%zd] %s already compressed\n", ndx, sname);
> -	    }
> -	  else if (!force && type == ZLIB_GNU
> -		   && startswith (sname, ".zdebug"))
> +	  enum ch_type schtype = get_section_chtype (scn, shdr, sname, ndx);
> +	  if (!force && verbose > 0)
>  	    {
> -	      if (verbose > 0)
> -		printf ("[%zd] %s already GNU compressed\n", ndx, sname);
> +	      /* The current compression matches the final one.  */
> +	      if (type == schtype)
> +		switch (type)
> +		  {
> +		  case NONE:
> +		    printf ("[%zd] %s already decompressed\n", ndx, sname);
> +		    break;
> +		  case ZLIB:
> +		  case ZSTD:
> +		    printf ("[%zd] %s already compressed\n", ndx, sname);
> +		    break;
> +		  case ZLIB_GNU:
> +		    printf ("[%zd] %s already GNU compressed\n", ndx, sname);
> +		    break;
> +		  default:
> +		    abort ();
> +		  }
>  	    }
> -	  else if (shdr->sh_type != SHT_NOBITS
> +
> +	  if (shdr->sh_type != SHT_NOBITS
>  	      && (shdr->sh_flags & SHF_ALLOC) == 0)
>  	    {
>  	      set_section (sections, ndx);
> @@ -692,37 +740,12 @@ process_file (const char *fname)
>  	     (de)compressed, invalidating the string pointers.  */
>  	  sname = xstrdup (sname);
> +
>  	  /* Detect source compression that is how is the section compressed
>  	     now.  */
> -	  GElf_Chdr chdr;
> -	  enum ch_type schtype = NONE;
> -	  if ((shdr->sh_flags & SHF_COMPRESSED) != 0)
> -	    {
> -	      if (gelf_getchdr (scn, &chdr) != NULL)
> -		{
> -		  schtype = (enum ch_type)chdr.ch_type;
> -		  if (schtype == NONE)
> -		    {
> -		      error (0, 0, "Compression type for section %zd"
> -			     " can't be zero ", ndx);
> -		      goto cleanup;
> -		    }
> -		  else if (schtype > MAXIMAL_CH_TYPE)
> -		    {
> -		      error (0, 0, "Compression type (%d) for section %zd"
> -			     " is unsupported ", schtype, ndx);
> -		      goto cleanup;
> -		    }
> -		}
> -	      else
> -		{
> -		  error (0, 0, "Couldn't get chdr for section %zd", ndx);
> -		  goto cleanup;
> -		}
> -	    }
> -	  /* Set ZLIB compression manually for .zdebug* sections.  */
> -	  else if (startswith (sname, ".zdebug"))
> -	    schtype = ZLIB_GNU;
> +	  enum ch_type schtype = get_section_chtype (scn, shdr, sname, ndx);
> +	  if (schtype == UNSET)
> +	    goto cleanup;
>  	  /* We might want to decompress (and rename), but not
>  	     compress during this pass since we might need the section
> @@ -754,7 +777,7 @@ process_file (const char *fname)
>  	    case ZLIB_GNU:
>  	      if (startswith (sname, ".debug"))
>  		{
> -		  if (schtype == ZLIB)
> +		  if (schtype == ZLIB || schtype == ZSTD)
>  		    {
>  		      /* First decompress to recompress GNU style.
>  			 Don't report even when verbose.  */

OK.

> @@ -818,19 +841,22 @@ process_file (const char *fname)
>  	      break;
>  	    case ZLIB:
> -	      if ((shdr->sh_flags & SHF_COMPRESSED) == 0)
> +	    case ZSTD:
> +	      if (schtype != type)
>  		{
> -		  if (schtype == ZLIB_GNU)
> +		  if (schtype != NONE)
>  		    {
> -		      /* First decompress to recompress zlib style.
> -			 Don't report even when verbose.  */
> +		      /* Decompress first.  */
>  		      if (compress_section (scn, size, sname, NULL, ndx,
>  					    schtype, NONE, false) < 0)
>  			goto cleanup;
> -		      snamebuf[0] = '.';
> -		      strcpy (&snamebuf[1], &sname[2]);
> -		      newname = snamebuf;
> +		      if (schtype == ZLIB_GNU)
> +			{
> +			  snamebuf[0] = '.';
> +			  strcpy (&snamebuf[1], &sname[2]);
> +			  newname = snamebuf;
> +			}
>  		    }
>  		  if (skip_compress_section)

OK.

> @@ -838,7 +864,7 @@ process_file (const char *fname)
>  		      if (ndx == shdrstrndx)
>  			{
>  			  shstrtab_size = size;
> -			  shstrtab_compressed = ZLIB;
> +			  shstrtab_compressed = type;
>  			  if (shstrtab_name != NULL
>  			      || shstrtab_newname != NULL)
>  			    {
> @@ -855,7 +881,7 @@ process_file (const char *fname)
>  		      else
>  			{
>  			  symtab_size = size;
> -			  symtab_compressed = ZLIB;
> +			  symtab_compressed = type;
>  			  symtab_name = xstrdup (sname);
>  			  symtab_newname = (newname == NULL
>  					    ? NULL : xstrdup (newname));

OK.

> @@ -1378,7 +1404,7 @@ main (int argc, char **argv)
>  	N_("Place (de)compressed output into FILE"),
>  	0 },
>        { "type", 't', "TYPE", 0,
> -	N_("What type of compression to apply. TYPE can be 'none' (decompress), 'zlib' (ELF ZLIB compression, the default, 'zlib-gabi' is an alias) or 'zlib-gnu' (.zdebug GNU style compression, 'gnu' is an alias)"),
> +	N_("What type of compression to apply. TYPE can be 'none' (decompress), 'zlib' (ELF ZLIB compression, the default, 'zlib-gabi' is an alias), 'zlib-gnu' (.zdebug GNU style compression, 'gnu' is an alias) or 'zstd'"),
>  	0 },
>        { "name", 'n', "SECTION", 0,
>  	N_("SECTION name to (de)compress, SECTION is an extended wildcard pattern (defaults to '.?(z)debug*')"),

I would say or 'zstd' (ELF ZSTD compression)" to match the 'zlib; type
description.

> diff --git a/src/readelf.c b/src/readelf.c
> index cc3e0229..451f8400 100644
> --- a/src/readelf.c
> +++ b/src/readelf.c
> @@ -1238,13 +1238,17 @@ get_visibility_type (int value)
>  static const char *
>  elf_ch_type_name (unsigned int code)
>  {
> -  if (code == 0)
> -    return "NONE";
> -
> -  if (code == ELFCOMPRESS_ZLIB)
> -    return "ZLIB";
> -
> -  return "UNKNOWN";
> +  switch (code)
> +    {
> +    case 0:
> +      return "NONE";
> +    case ELFCOMPRESS_ZLIB:
> +      return "ZLIB";
> +    case ELFCOMPRESS_ZSTD:
> +      return "ZSTD";
> +    default:
> +      return "UNKNOWN";
> +    }
>  }
>  /* Print the section headers.  */

OK.

> diff --git a/tests/run-compress-test.sh b/tests/run-compress-test.sh
> index a6a298f5..3f9c990e 100755
> --- a/tests/run-compress-test.sh
> +++ b/tests/run-compress-test.sh
> @@ -61,6 +61,30 @@ testrun_elfcompress_file()
>      echo "uncompress $elfcompressedfile -> $elfuncompressedfile"
>      testrun ${abs_top_builddir}/src/elfcompress -v -t none -o ${elfuncompressedfile} ${elfcompressedfile}
>      testrun ${abs_top_builddir}/src/elfcmp ${uncompressedfile} ${elfuncompressedfile}
> +
> +    outputfile="${infile}.gabi.zstd"
> +    tempfiles "$outputfile"
> +    echo "zstd compress $elfcompressedfile -> $outputfile"
> +    testrun ${abs_top_builddir}/src/elfcompress -v -t zstd -o ${outputfile} ${elfcompressedfile}
> +    testrun ${abs_top_builddir}/src/elfcmp ${uncompressedfile} ${outputfile}
> +    echo "checking compressed section header" $outputfile
> +    testrun ${abs_top_builddir}/src/readelf -Sz ${outputfile} | grep "ELF ZSTD" >/dev/null
> +
> +    zstdfile="${infile}.zstd"
> +    tempfiles "$zstdfile"
> +    echo "zstd compress $uncompressedfile -> $zstdfile"
> +    testrun ${abs_top_builddir}/src/elfcompress -v -t zstd -o ${zstdfile} ${elfuncompressedfile}
> +    testrun ${abs_top_builddir}/src/elfcmp ${uncompressedfile} ${zstdfile}
> +    echo "checking compressed section header" $zstdfile
> +    testrun ${abs_top_builddir}/src/readelf -Sz ${zstdfile} | grep "ELF ZSTD" >/dev/null
> +
> +    zstdgnufile="${infile}.zstd.gnu"
> +    tempfiles "$zstdgnufile"
> +    echo "zstd re-compress to GNU ZLIB $zstdfile -> $zstdgnufile"
> +    testrun ${abs_top_builddir}/src/elfcompress -v -t zlib-gnu -o ${zstdgnufile} ${zstdfile}
> +    testrun ${abs_top_builddir}/src/elfcmp ${uncompressedfile} ${zstdgnufile}
> +    echo "checking .zdebug section name" $zstdgnufile
> +    testrun ${abs_top_builddir}/src/readelf -S ${zstdgnufile} | grep ".zdebug" >/dev/null
>  }
>  testrun_elfcompress()

You might add these to a separate run test file or pass some ZSTD flag
through the test environment to conditionally run these new tests.

Cheers,

Mark
  
Martin Liška Dec. 19, 2022, 2:19 p.m. UTC | #4
On 12/15/22 14:17, Mark Wielaard wrote:
> Hi Martin,
> 
> On Tue, 2022-11-29 at 13:05 +0100, Martin Liška wrote:
>> There's second version of the patch that fully support both
>> compression and decompression.
>>
>> Changes from the v1:
>> - compression support added
>> - zstd detection is fixed
>> - new tests are added
>> - builds fine w/ and w/o the ZSTD library
>>
>> What's currently missing and where I need a help:
>>
>> 1) When I build ./configure --without-zstd, I don't have
>> a reasonable error message (something similar to binutils's readelf:
>> readelf: Warning: section '.debug_str' has unsupported compress type:
>> 2)
>> even though, __libelf_decompress returns NULL and __libelf_seterrno).
>> One can see a garbage in the console.
>>
>> How to handle that properly?
> 
> Is there a particular way you are running eu-readelf? Is it with
> generic -w or -a, or decoding a specific section type?

Hello.

$ LD_LIBRARY_PATH=./libelf ./src/readelf -w ~/Programming/testcases/a.out

where I get:

./src/readelf: cannot get debug context descriptor: No DWARF information found

DWARF section [37] '.debug_info' at offset 0x1ab2:
  [Offset]
./src/readelf: cannot get next unit: no error

Call frame information section [13] '.eh_frame' at offset 0x4a8:
...
                                                                                                                                                t��o5��=I�iAp@a����S^R/<�����^�qi�ַp@
                                                                                                                                                                                    E���Z
�O��Š�w��/#�!���7�6�#�I����*���굮R�v妗�/Z���O����Oի���E��z�����K��(��9���:{ѧ�zOa;̥�O޾�����+O�̰҆ˑ}��C��ۋ_�k9��
                                                                                                           Jv�>;)`F��	:!�-�ˏQ@�L,
V��6cI�Ъ^�6z��6�4�Fz���n���}~�U��R�])��zF��#�V��E�eȹ/�
                                                       Z�A!DJP%"�H�$i� Bc3�"
  *** error, missing string terminator

So basically a garbage. And I don't know how to bail out properly?

> 
>> 2) How should I run my newly added tests conditionally if zstd
>> configuration support is enabled?
> 
> eu_ZIP will define an AM_CONDITIONAL for ZSTD (note this is different
> from the HAVE_ZSTD, which is the am conditional added for having the
> zstd program itself). You could use it in tests/Makefile.am as:
> 
>    if ZSTD
>    TESTS += run-zstd-compress-test.sh
>    endif
> 
> If you had a separate test... Otherwise add some variable to
> TESTS_ENVIRONMENT (and installed_TESTS_ENVIRONMENT) based on the Make
> variable that is tested inside the shell script (somewhat like
> USE_VALGRIND) maybe.

All right, I have a working version where I utilize an env. variable.

> 
>>    configure.ac               |   8 +-
>>    libelf/Makefile.am         |   2 +-
>>    libelf/elf_compress.c      | 294 ++++++++++++++++++++++++++++++--
>> -----
>>    libelf/elf_compress_gnu.c  |   5 +-
>>    libelf/libelfP.h           |   4 +-
>>    src/elfcompress.c          | 144 ++++++++++--------
>>    src/readelf.c              |  18 ++-
>>    tests/run-compress-test.sh |  24 +++
>>    8 files changed, 373 insertions(+), 126 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 59be27ac..07cfa54b 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -410,6 +410,11 @@ dnl Test for bzlib and xz/lzma/zstd, gives BZLIB/LZMALIB/ZSTD .am
>>    dnl conditional and config.h USE_BZLIB/USE_LZMALIB/USE_ZSTD #define.
>>    save_LIBS="$LIBS"
>>    LIBS=
>> +eu_ZIPLIB(zstd,ZSTD,zstd,ZSTD_decompress,[ZSTD (zst)])
>> +AS_IF([test "x$with_zstd" = xyes], [LIBZSTD="libzstd"], [LIBLZSTD=""])
>> +AC_SUBST([LIBZSTD])
>> +zstd_LIBS="$LIBS"
>> +AC_SUBST([zstd_LIBS])
>>    eu_ZIPLIB(bzlib,BZLIB,bz2,BZ2_bzdopen,bzip2)
>>    # We need this since bzip2 doesn't have a pkgconfig file.
>>    BZ2_LIB="$LIBS"
>> @@ -417,9 +422,6 @@ AC_SUBST([BZ2_LIB])
>>    eu_ZIPLIB(lzma,LZMA,lzma,lzma_auto_decoder,[LZMA (xz)])
>>    AS_IF([test "x$with_lzma" = xyes], [LIBLZMA="liblzma"], [LIBLZMA=""])
>>    AC_SUBST([LIBLZMA])
>> -eu_ZIPLIB(zstd,ZSTD,zstd,ZSTD_decompress,[ZSTD (zst)])
>> -AS_IF([test "x$with_zstd" = xyes], [LIBZSTD="libzstd"], [LIBLZSTD=""])
>> -AC_SUBST([LIBZSTD])
>>    zip_LIBS="$LIBS"
>>    LIBS="$save_LIBS"
>>    AC_SUBST([zip_LIBS])
> 
> Doing AC_SUBST([zstd_LIBS]) seems correct. Why is the test moved
> earlier?
> 
> You are testing for ZSTD_decompress, is that enough? Asking because I
> see you are using ZSTD_compressStream2, which seems to requires libzstd
> v1.4.0+.
> 
> In general how stable is the libzstd api?

It's hopefully stable, but yes, I should check for ZSTD_compressStream2 in configure.ac.
I'm going to update that.

> 
> You'll also need to use the AC_SUBST for LIBZSTD in config/libelf.pc.in
> now, as used in the libdw.pc.in:
> 
>    Requires.private: zlib @LIBZSTD@

Oh, thanks.

Cheers,
Martin

> 
>> diff --git a/libelf/Makefile.am b/libelf/Makefile.am
>> index 560ed45f..24c25cf8 100644
>> --- a/libelf/Makefile.am
>> +++ b/libelf/Makefile.am
>> @@ -106,7 +106,7 @@ libelf_pic_a_SOURCES =
>>    am_libelf_pic_a_OBJECTS = $(libelf_a_SOURCES:.c=.os)
>>    
>>    libelf_so_DEPS = ../lib/libeu.a
>> -libelf_so_LDLIBS = $(libelf_so_DEPS) -lz
>> +libelf_so_LDLIBS = $(libelf_so_DEPS) -lz $(zstd_LIBS)
>>    if USE_LOCKS
>>    libelf_so_LDLIBS += -lpthread
>>    endif
> 
> OK.
> 
> Haven't read the actual code yet. I'll get back to that later today.
> 
> Cheers,
> 
> Mark
  
Martin Liška Dec. 19, 2022, 2:21 p.m. UTC | #5
Hi.

>> +  if (use_zstd)
>> +#ifdef USE_ZSTD
>> +    return __libelf_compress_zstd (scn, hsize, ei_data, orig_size,
>> +				   orig_addralign, new_size, force,
>> +				   data, next_data, out_buf, out_size,
>> +				   block);
>> +#else
>> +    return NULL;
> 
> Should this also call __libelf_seterrno (ELF_E_UNKNOWN_COMPRESSION_TYPE) ?

Yes, will fix that.

> 
>> +#endif
>> +  else
>> +    return __libelf_compress_zlib (scn, hsize, ei_data, orig_size,
>> +				   orig_addralign, new_size, force,
>> +				   data, next_data, out_buf, out_size,
>> +				   block);
>> +}
>> +
>> +void *
>> +internal_function
>> +__libelf_decompress_zlib (void *buf_in, size_t size_in, size_t size_out)
>>   {
>>     /* Catch highly unlikely compression ratios so we don't allocate
>>        some giant amount of memory for nothing. The max compression
>> @@ -218,7 +360,7 @@ __libelf_decompress (void *buf_in, size_t size_in, size_t size_out)
>>         return NULL;
>>       }
>> -  /* Malloc might return NULL when requestion zero size.  This is highly
>> +  /* Malloc might return NULL when requesting zero size.  This is highly
>>        unlikely, it would only happen when the compression was forced.
>>        But we do need a non-NULL buffer to return and set as result.
>>        Just make sure to always allocate at least 1 byte.  */
> 
> OK, typo fix.
> 
>> @@ -260,6 +402,51 @@ __libelf_decompress (void *buf_in, size_t size_in, size_t size_out)
>>     return buf_out;
>>   }
>> +#ifdef USE_ZSTD
>> +void *
>> +internal_function
>> +__libelf_decompress_zstd (void *buf_in, size_t size_in, size_t size_out)
>> +{
>> +  /* Malloc might return NULL when requesting zero size.  This is highly
>> +     unlikely, it would only happen when the compression was forced.
>> +     But we do need a non-NULL buffer to return and set as result.
>> +     Just make sure to always allocate at least 1 byte.  */
>> +  void *buf_out = malloc (size_out ?: 1);
>> +  if (unlikely (buf_out == NULL))
>> +    {
>> +      __libelf_seterrno (ELF_E_NOMEM);
>> +      return NULL;
>> +    }
>> +
>> +  size_t ret = ZSTD_decompress (buf_out, size_out, buf_in, size_in);
>> +  if (ZSTD_isError (ret))
>> +    {
>> +      free (buf_out);
>> +      __libelf_seterrno (ELF_E_DECOMPRESS_ERROR);
>> +      return NULL;
>> +    }
>> +  else
>> +    return buf_out;
>> +}
>> +#endif
> 
> OK.
> 
>> +void *
>> +internal_function
>> +__libelf_decompress (int chtype, void *buf_in, size_t size_in, size_t size_out)
>> +{
>> +  if (chtype == ELFCOMPRESS_ZLIB)
>> +    return __libelf_decompress_zlib (buf_in, size_in, size_out);
>> +  else
>> +    {
>> +#ifdef USE_ZSTD
>> +    return __libelf_decompress_zstd (buf_in, size_in, size_out);
>> +#else
>> +    __libelf_seterrno (ELF_E_DECOMPRESS_ERROR);
> 
> Maybe ELF_E_UNKNOWN_COMPRESSION_TYPE ?

Yes, will fix that.

> 
>> +    return NULL;
>> +#endif
>> +    }
>> +}
>> +
>>   void *
>>   internal_function
>>   __libelf_decompress_elf (Elf_Scn *scn, size_t *size_out, size_t *addralign)
>> @@ -268,7 +455,7 @@ __libelf_decompress_elf (Elf_Scn *scn, size_t *size_out, size_t *addralign)
>>     if (gelf_getchdr (scn, &chdr) == NULL)
>>       return NULL;
>> -  if (chdr.ch_type != ELFCOMPRESS_ZLIB)
>> +  if (chdr.ch_type != ELFCOMPRESS_ZLIB && chdr.ch_type != ELFCOMPRESS_ZSTD)
>>       {
>>         __libelf_seterrno (ELF_E_UNKNOWN_COMPRESSION_TYPE);
>>         return NULL;
> 
> Should the chdr.ch_type != ELFCOMPRESS_ZSTD be guarded by #ifdef USE_ZSTD ?

Yes, will fix that.
>> +      else
>> +	error (0, 0, "Couldn't get chdr for section %zd", ndx);
> 
> Shouldn't this error be fatal?

What do you use for fatal errors?
> 
>> @@ -1378,7 +1404,7 @@ main (int argc, char **argv)
>>   	N_("Place (de)compressed output into FILE"),
>>   	0 },
>>         { "type", 't', "TYPE", 0,
>> -	N_("What type of compression to apply. TYPE can be 'none' (decompress), 'zlib' (ELF ZLIB compression, the default, 'zlib-gabi' is an alias) or 'zlib-gnu' (.zdebug GNU style compression, 'gnu' is an alias)"),
>> +	N_("What type of compression to apply. TYPE can be 'none' (decompress), 'zlib' (ELF ZLIB compression, the default, 'zlib-gabi' is an alias), 'zlib-gnu' (.zdebug GNU style compression, 'gnu' is an alias) or 'zstd'"),
>>   	0 },
>>         { "name", 'n', "SECTION", 0,
>>   	N_("SECTION name to (de)compress, SECTION is an extended wildcard pattern (defaults to '.?(z)debug*')"),
> 
> I would say or 'zstd' (ELF ZSTD compression)" to match the 'zlib; type
> description.

Works for me.

Cheers,
Martin

> 
>> diff --git a/src/readelf.c b/src/readelf.c
>> index cc3e0229..451f8400 100644
>> --- a/src/readelf.c
>> +++ b/src/readelf.c
>> @@ -1238,13 +1238,17 @@ get_visibility_type (int value)
>>   static const char *
>>   elf_ch_type_name (unsigned int code)
>>   {
>> -  if (code == 0)
>> -    return "NONE";
>> -
>> -  if (code == ELFCOMPRESS_ZLIB)
>> -    return "ZLIB";
>> -
>> -  return "UNKNOWN";
>> +  switch (code)
>> +    {
>> +    case 0:
>> +      return "NONE";
>> +    case ELFCOMPRESS_ZLIB:
>> +      return "ZLIB";
>> +    case ELFCOMPRESS_ZSTD:
>> +      return "ZSTD";
>> +    default:
>> +      return "UNKNOWN";
>> +    }
>>   }
>>   /* Print the section headers.  */
> 
> OK.
> 
>> diff --git a/tests/run-compress-test.sh b/tests/run-compress-test.sh
>> index a6a298f5..3f9c990e 100755
>> --- a/tests/run-compress-test.sh
>> +++ b/tests/run-compress-test.sh
>> @@ -61,6 +61,30 @@ testrun_elfcompress_file()
>>       echo "uncompress $elfcompressedfile -> $elfuncompressedfile"
>>       testrun ${abs_top_builddir}/src/elfcompress -v -t none -o ${elfuncompressedfile} ${elfcompressedfile}
>>       testrun ${abs_top_builddir}/src/elfcmp ${uncompressedfile} ${elfuncompressedfile}
>> +
>> +    outputfile="${infile}.gabi.zstd"
>> +    tempfiles "$outputfile"
>> +    echo "zstd compress $elfcompressedfile -> $outputfile"
>> +    testrun ${abs_top_builddir}/src/elfcompress -v -t zstd -o ${outputfile} ${elfcompressedfile}
>> +    testrun ${abs_top_builddir}/src/elfcmp ${uncompressedfile} ${outputfile}
>> +    echo "checking compressed section header" $outputfile
>> +    testrun ${abs_top_builddir}/src/readelf -Sz ${outputfile} | grep "ELF ZSTD" >/dev/null
>> +
>> +    zstdfile="${infile}.zstd"
>> +    tempfiles "$zstdfile"
>> +    echo "zstd compress $uncompressedfile -> $zstdfile"
>> +    testrun ${abs_top_builddir}/src/elfcompress -v -t zstd -o ${zstdfile} ${elfuncompressedfile}
>> +    testrun ${abs_top_builddir}/src/elfcmp ${uncompressedfile} ${zstdfile}
>> +    echo "checking compressed section header" $zstdfile
>> +    testrun ${abs_top_builddir}/src/readelf -Sz ${zstdfile} | grep "ELF ZSTD" >/dev/null
>> +
>> +    zstdgnufile="${infile}.zstd.gnu"
>> +    tempfiles "$zstdgnufile"
>> +    echo "zstd re-compress to GNU ZLIB $zstdfile -> $zstdgnufile"
>> +    testrun ${abs_top_builddir}/src/elfcompress -v -t zlib-gnu -o ${zstdgnufile} ${zstdfile}
>> +    testrun ${abs_top_builddir}/src/elfcmp ${uncompressedfile} ${zstdgnufile}
>> +    echo "checking .zdebug section name" $zstdgnufile
>> +    testrun ${abs_top_builddir}/src/readelf -S ${zstdgnufile} | grep ".zdebug" >/dev/null
>>   }
>>   testrun_elfcompress()
> 
> You might add these to a separate run test file or pass some ZSTD flag
> through the test environment to conditionally run these new tests.
> 
> Cheers,
> 
> Mark
  
Mark Wielaard Dec. 19, 2022, 3:09 p.m. UTC | #6
Hi Martin,

On Mon, 2022-12-19 at 15:19 +0100, Martin Liška wrote:
> On 12/15/22 14:17, Mark Wielaard wrote:
> > Is there a particular way you are running eu-readelf? Is it with
> > generic -w or -a, or decoding a specific section type?
> 
> Hello.
> 
> $ LD_LIBRARY_PATH=./libelf ./src/readelf -w
> ~/Programming/testcases/a.out
> 
> where I get:
> 
> ./src/readelf: cannot get debug context descriptor: No DWARF
> information found
> 
> DWARF section [37] '.debug_info' at offset 0x1ab2:
>   [Offset]
> ./src/readelf: cannot get next unit: no error
> 
> Call frame information section [13] '.eh_frame' at offset 0x4a8:
> ...
>                                                                      
>                                                                      
>       t��o5��=I�iAp@a����S^R/<�����^�qi�ַp@

[...]

> So basically a garbage. And I don't know how to bail out properly?

Aha. If you have that a.out somewhere I can take a look. I suspect this
is because we expect all .debug sections to have been decompressed in
libdw/dwarf_begin_elf.c, but that isn't really true, see check_section
in that file which has:

  if ((shdr->sh_flags & SHF_COMPRESSED) != 0)
    {
      if (elf_compress (scn, 0, 0) < 0)
        {
          /* It would be nice if we could fail with a specific error.
             But we don't know if this was an essential section or not.
             So just continue for now. See also valid_p().  */
          return result;
        }
    }

We should probably adjust valid_p so it produces a more appropriate
error message and/or add additional checks in readlelf.c.

But lets do that after this patch goes in.

Cheers,

Mark
  
Mark Wielaard Dec. 19, 2022, 3:16 p.m. UTC | #7
Hi Martin,

On Mon, 2022-12-19 at 15:21 +0100, Martin Liška wrote:
> > > +      else
> > > +    error (0, 0, "Couldn't get chdr for section %zd", ndx);
> > 
> > Shouldn't this error be fatal?
> 
> What do you use for fatal errors?

Depends a bit on context. It might be that this error isn't fatal, then
zero as first (status) argument is fine, just know that the program
will just continue. And it looked like not all callers were prepared
for this function to return with a bogus return value.

If it is fatal then depending on context you either call error_exit (0,
"Couldn't get chdr for section %zd", ndx); [see system.h, this really
is just error (EXIT_FAILURE, 0, ...)] if the program needs to terminate
right now.

Or you return a special value from the function (assuming all callers
check for an error here). And/Or if the program needs a cleanup you'll
goto cleanup (as is done in process_file).

Cheers,

Mark
  
Martin Liška Dec. 21, 2022, 11:09 a.m. UTC | #8
On 12/19/22 16:16, Mark Wielaard wrote:
> Hi Martin,
> 
> On Mon, 2022-12-19 at 15:21 +0100, Martin Liška wrote:
>>>> +      else
>>>> +    error (0, 0, "Couldn't get chdr for section %zd", ndx);
>>>
>>> Shouldn't this error be fatal?
>>
>> What do you use for fatal errors?
> 
> Depends a bit on context. It might be that this error isn't fatal, then
> zero as first (status) argument is fine, just know that the program
> will just continue. And it looked like not all callers were prepared
> for this function to return with a bogus return value.
> 
> If it is fatal then depending on context you either call error_exit (0,
> "Couldn't get chdr for section %zd", ndx); [see system.h, this really
> is just error (EXIT_FAILURE, 0, ...)] if the program needs to terminate
> right now.
> 
> Or you return a special value from the function (assuming all callers
> check for an error here). And/Or if the program needs a cleanup you'll
> goto cleanup (as is done in process_file).

I think it's fine as we return UNSET in that case and the caller goes directly
to cleanup (or abort is called for the second call site):

	  enum ch_type schtype = get_section_chtype (scn, shdr, sname, ndx);
	  if (schtype == UNSET)
	    goto cleanup;

Martin

> 
> Cheers,
> 
> Mark
  
Martin Liška Dec. 21, 2022, 11:13 a.m. UTC | #9
On 12/19/22 16:09, Mark Wielaard wrote:
> Hi Martin,
> 
> On Mon, 2022-12-19 at 15:19 +0100, Martin Liška wrote:
>> On 12/15/22 14:17, Mark Wielaard wrote:
>>> Is there a particular way you are running eu-readelf? Is it with
>>> generic -w or -a, or decoding a specific section type?
>>
>> Hello.
>>
>> $ LD_LIBRARY_PATH=./libelf ./src/readelf -w
>> ~/Programming/testcases/a.out
>>
>> where I get:
>>
>> ./src/readelf: cannot get debug context descriptor: No DWARF
>> information found
>>
>> DWARF section [37] '.debug_info' at offset 0x1ab2:
>>   [Offset]
>> ./src/readelf: cannot get next unit: no error
>>
>> Call frame information section [13] '.eh_frame' at offset 0x4a8:
>> ...
>>                                                                      
>>                                                                      
>>       t��o5��=I�iAp@a����S^R/<�����^�qi�ַp@
> 
> [...]
> 
>> So basically a garbage. And I don't know how to bail out properly?
> 
> Aha. If you have that a.out somewhere I can take a look. I suspect this
> is because we expect all .debug sections to have been decompressed in
> libdw/dwarf_begin_elf.c, but that isn't really true, see check_section
> in that file which has:
> 
>   if ((shdr->sh_flags & SHF_COMPRESSED) != 0)
>     {
>       if (elf_compress (scn, 0, 0) < 0)
>         {
>           /* It would be nice if we could fail with a specific error.
>              But we don't know if this was an essential section or not.
>              So just continue for now. See also valid_p().  */
>           return result;
>         }
>     }

Sure, there's a file example:
https://splichal.eu/tmp/zstd.out

$ ./src/readelf -Sz zstd.out
...
[35] .debug_abbrev        PROGBITS     0000000000000000 000018b4 00000177  0 C      0   0  1
     [ELF ZSTD (2) 00000318  1]

$ LD_LIBRARY_PATH=libelf/ ./src/readelf -w zstd.out
(prints garbage if not configured with zstd)

Please use code from my branch:
https://sourceware.org/git/?p=elfutils.git;a=shortlog;h=refs/heads/users/marxin/try-zstd-support-v2

Cheers,
Martin

> 
> We should probably adjust valid_p so it produces a more appropriate
> error message and/or add additional checks in readlelf.c.
> 
> But lets do that after this patch goes in.
> 
> Cheers,
> 
> Mark
  
Mark Wielaard Dec. 21, 2022, 11:14 p.m. UTC | #10
Hi Martin,

On Wed, Dec 21, 2022 at 12:09:21PM +0100, Martin Liška wrote:
> > Or you return a special value from the function (assuming all callers
> > check for an error here). And/Or if the program needs a cleanup you'll
> > goto cleanup (as is done in process_file).
> 
> I think it's fine as we return UNSET in that case and the caller goes directly
> to cleanup (or abort is called for the second call site):
> 
> 	  enum ch_type schtype = get_section_chtype (scn, shdr, sname, ndx);
> 	  if (schtype == UNSET)
> 	    goto cleanup;

O, that is good.

Is the abort () at the second call site because that cannot happen? Or
should that also goto cleanup?

Cheers,

Mark
  

Patch

diff --git a/configure.ac b/configure.ac
index 59be27ac..07cfa54b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -410,6 +410,11 @@  dnl Test for bzlib and xz/lzma/zstd, gives BZLIB/LZMALIB/ZSTD .am
  dnl conditional and config.h USE_BZLIB/USE_LZMALIB/USE_ZSTD #define.
  save_LIBS="$LIBS"
  LIBS=
+eu_ZIPLIB(zstd,ZSTD,zstd,ZSTD_decompress,[ZSTD (zst)])
+AS_IF([test "x$with_zstd" = xyes], [LIBZSTD="libzstd"], [LIBLZSTD=""])
+AC_SUBST([LIBZSTD])
+zstd_LIBS="$LIBS"
+AC_SUBST([zstd_LIBS])
  eu_ZIPLIB(bzlib,BZLIB,bz2,BZ2_bzdopen,bzip2)
  # We need this since bzip2 doesn't have a pkgconfig file.
  BZ2_LIB="$LIBS"
@@ -417,9 +422,6 @@  AC_SUBST([BZ2_LIB])
  eu_ZIPLIB(lzma,LZMA,lzma,lzma_auto_decoder,[LZMA (xz)])
  AS_IF([test "x$with_lzma" = xyes], [LIBLZMA="liblzma"], [LIBLZMA=""])
  AC_SUBST([LIBLZMA])
-eu_ZIPLIB(zstd,ZSTD,zstd,ZSTD_decompress,[ZSTD (zst)])
-AS_IF([test "x$with_zstd" = xyes], [LIBZSTD="libzstd"], [LIBLZSTD=""])
-AC_SUBST([LIBZSTD])
  zip_LIBS="$LIBS"
  LIBS="$save_LIBS"
  AC_SUBST([zip_LIBS])
diff --git a/libelf/Makefile.am b/libelf/Makefile.am
index 560ed45f..24c25cf8 100644
--- a/libelf/Makefile.am
+++ b/libelf/Makefile.am
@@ -106,7 +106,7 @@  libelf_pic_a_SOURCES =
  am_libelf_pic_a_OBJECTS = $(libelf_a_SOURCES:.c=.os)
  
  libelf_so_DEPS = ../lib/libeu.a
-libelf_so_LDLIBS = $(libelf_so_DEPS) -lz
+libelf_so_LDLIBS = $(libelf_so_DEPS) -lz $(zstd_LIBS)
  if USE_LOCKS
  libelf_so_LDLIBS += -lpthread
  endif
diff --git a/libelf/elf_compress.c b/libelf/elf_compress.c
index d7f53af2..7a6e37a4 100644
--- a/libelf/elf_compress.c
+++ b/libelf/elf_compress.c
@@ -39,6 +39,10 @@ 
  #include <string.h>
  #include <zlib.h>
  
+#ifdef USE_ZSTD
+#include <zstd.h>
+#endif
+
  /* Cleanup and return result.  Don't leak memory.  */
  static void *
  do_deflate_cleanup (void *result, z_stream *z, void *out_buf,
@@ -54,53 +58,14 @@  do_deflate_cleanup (void *result, z_stream *z, void *out_buf,
  #define deflate_cleanup(result, cdata) \
      do_deflate_cleanup(result, &z, out_buf, cdata)
  
-/* Given a section, uses the (in-memory) Elf_Data to extract the
-   original data size (including the given header size) and data
-   alignment.  Returns a buffer that has at least hsize bytes (for the
-   caller to fill in with a header) plus zlib compressed date.  Also
-   returns the new buffer size in new_size (hsize + compressed data
-   size).  Returns (void *) -1 when FORCE is false and the compressed
-   data would be bigger than the original data.  */
  void *
  internal_function
-__libelf_compress (Elf_Scn *scn, size_t hsize, int ei_data,
-		   size_t *orig_size, size_t *orig_addralign,
-		   size_t *new_size, bool force)
+__libelf_compress_zlib (Elf_Scn *scn, size_t hsize, int ei_data,
+			size_t *orig_size, size_t *orig_addralign,
+			size_t *new_size, bool force,
+			Elf_Data *data, Elf_Data *next_data,
+			void *out_buf, size_t out_size, size_t block)
  {
-  /* The compressed data is the on-disk data.  We simplify the
-     implementation a bit by asking for the (converted) in-memory
-     data (which might be all there is if the user created it with
-     elf_newdata) and then convert back to raw if needed before
-     compressing.  Should be made a bit more clever to directly
-     use raw if that is directly available.  */
-  Elf_Data *data = elf_getdata (scn, NULL);
-  if (data == NULL)
-    return NULL;
-
-  /* When not forced and we immediately know we would use more data by
-     compressing, because of the header plus zlib overhead (five bytes
-     per 16 KB block, plus a one-time overhead of six bytes for the
-     entire stream), don't do anything.  */
-  Elf_Data *next_data = elf_getdata (scn, data);
-  if (next_data == NULL && !force
-      && data->d_size <= hsize + 5 + 6)
-    return (void *) -1;
-
-  *orig_addralign = data->d_align;
-  *orig_size = data->d_size;
-
-  /* Guess an output block size. 1/8th of the original Elf_Data plus
-     hsize.  Make the first chunk twice that size (25%), then increase
-     by a block (12.5%) when necessary.  */
-  size_t block = (data->d_size / 8) + hsize;
-  size_t out_size = 2 * block;
-  void *out_buf = malloc (out_size);
-  if (out_buf == NULL)
-    {
-      __libelf_seterrno (ELF_E_NOMEM);
-      return NULL;
-    }
-
    /* Caller gets to fill in the header at the start.  Just skip it here.  */
    size_t used = hsize;
  
@@ -205,9 +170,186 @@  __libelf_compress (Elf_Scn *scn, size_t hsize, int ei_data,
    return out_buf;
  }
  
+#ifdef USE_ZSTD
+/* Cleanup and return result.  Don't leak memory.  */
+static void *
+do_zstd_cleanup (void *result, ZSTD_CCtx * const cctx, void *out_buf,
+		 Elf_Data *cdatap)
+{
+  ZSTD_freeCCtx (cctx);
+  free (out_buf);
+  if (cdatap != NULL)
+    free (cdatap->d_buf);
+  return result;
+}
+
+#define zstd_cleanup(result, cdata) \
+    do_zstd_cleanup(result, cctx, out_buf, cdata)
+
  void *
  internal_function
-__libelf_decompress (void *buf_in, size_t size_in, size_t size_out)
+__libelf_compress_zstd (Elf_Scn *scn, size_t hsize, int ei_data,
+			size_t *orig_size, size_t *orig_addralign,
+			size_t *new_size, bool force,
+			Elf_Data *data, Elf_Data *next_data,
+			void *out_buf, size_t out_size, size_t block)
+{
+  /* Caller gets to fill in the header at the start.  Just skip it here.  */
+  size_t used = hsize;
+
+  ZSTD_CCtx* const cctx = ZSTD_createCCtx();
+  Elf_Data cdata;
+  cdata.d_buf = NULL;
+
+  /* Loop over data buffers.  */
+  ZSTD_EndDirective mode = ZSTD_e_continue;
+
+  do
+    {
+      /* Convert to raw if different endianness.  */
+      cdata = *data;
+      bool convert = ei_data != MY_ELFDATA && data->d_size > 0;
+      if (convert)
+	{
+	  /* Don't do this conversion in place, we might want to keep
+	     the original data around, caller decides.  */
+	  cdata.d_buf = malloc (data->d_size);
+	  if (cdata.d_buf == NULL)
+	    {
+	      __libelf_seterrno (ELF_E_NOMEM);
+	      return zstd_cleanup (NULL, NULL);
+	    }
+	  if (gelf_xlatetof (scn->elf, &cdata, data, ei_data) == NULL)
+	    return zstd_cleanup (NULL, &cdata);
+	}
+
+      ZSTD_inBuffer ib = { cdata.d_buf, cdata.d_size, 0 };
+
+      /* Get next buffer to see if this is the last one.  */
+      data = next_data;
+      if (data != NULL)
+	{
+	  *orig_addralign = MAX (*orig_addralign, data->d_align);
+	  *orig_size += data->d_size;
+	  next_data = elf_getdata (scn, data);
+	}
+      else
+	mode = ZSTD_e_end;
+
+      /* Flush one data buffer.  */
+      for (;;)
+	{
+	  ZSTD_outBuffer ob = { out_buf + used, out_size - used, 0 };
+	  size_t ret = ZSTD_compressStream2 (cctx, &ob, &ib, mode);
+	  if (ZSTD_isError (ret))
+	    {
+	      __libelf_seterrno (ELF_E_COMPRESS_ERROR);
+	      return zstd_cleanup (NULL, convert ? &cdata : NULL);
+	    }
+	  used += ob.pos;
+
+	  /* Bail out if we are sure the user doesn't want the
+	     compression forced and we are using more compressed data
+	     than original data.  */
+	  if (!force && mode == ZSTD_e_end && used >= *orig_size)
+	    return zstd_cleanup ((void *) -1, convert ? &cdata : NULL);
+
+	  if (ret > 0)
+	    {
+	      void *bigger = realloc (out_buf, out_size + block);
+	      if (bigger == NULL)
+		{
+		  __libelf_seterrno (ELF_E_NOMEM);
+		  return zstd_cleanup (NULL, convert ? &cdata : NULL);
+		}
+	      out_buf = bigger;
+	      out_size += block;
+	    }
+	  else
+	    break;
+	}
+
+      if (convert)
+	{
+	  free (cdata.d_buf);
+	  cdata.d_buf = NULL;
+	}
+    }
+  while (mode != ZSTD_e_end); /* More data blocks.  */
+
+  ZSTD_freeCCtx (cctx);
+  *new_size = used;
+  return out_buf;
+}
+#endif
+
+/* Given a section, uses the (in-memory) Elf_Data to extract the
+   original data size (including the given header size) and data
+   alignment.  Returns a buffer that has at least hsize bytes (for the
+   caller to fill in with a header) plus zlib compressed date.  Also
+   returns the new buffer size in new_size (hsize + compressed data
+   size).  Returns (void *) -1 when FORCE is false and the compressed
+   data would be bigger than the original data.  */
+void *
+internal_function
+__libelf_compress (Elf_Scn *scn, size_t hsize, int ei_data,
+		   size_t *orig_size, size_t *orig_addralign,
+		   size_t *new_size, bool force, bool use_zstd)
+{
+  /* The compressed data is the on-disk data.  We simplify the
+     implementation a bit by asking for the (converted) in-memory
+     data (which might be all there is if the user created it with
+     elf_newdata) and then convert back to raw if needed before
+     compressing.  Should be made a bit more clever to directly
+     use raw if that is directly available.  */
+  Elf_Data *data = elf_getdata (scn, NULL);
+  if (data == NULL)
+    return NULL;
+
+  /* When not forced and we immediately know we would use more data by
+     compressing, because of the header plus zlib overhead (five bytes
+     per 16 KB block, plus a one-time overhead of six bytes for the
+     entire stream), don't do anything.
+     Size estimation for ZSTD compression would be similar.  */
+  Elf_Data *next_data = elf_getdata (scn, data);
+  if (next_data == NULL && !force
+      && data->d_size <= hsize + 5 + 6)
+    return (void *) -1;
+
+  *orig_addralign = data->d_align;
+  *orig_size = data->d_size;
+
+  /* Guess an output block size. 1/8th of the original Elf_Data plus
+     hsize.  Make the first chunk twice that size (25%), then increase
+     by a block (12.5%) when necessary.  */
+  size_t block = (data->d_size / 8) + hsize;
+  size_t out_size = 2 * block;
+  void *out_buf = malloc (out_size);
+  if (out_buf == NULL)
+    {
+      __libelf_seterrno (ELF_E_NOMEM);
+      return NULL;
+    }
+
+  if (use_zstd)
+#ifdef USE_ZSTD
+    return __libelf_compress_zstd (scn, hsize, ei_data, orig_size,
+				   orig_addralign, new_size, force,
+				   data, next_data, out_buf, out_size,
+				   block);
+#else
+    return NULL;
+#endif
+  else
+    return __libelf_compress_zlib (scn, hsize, ei_data, orig_size,
+				   orig_addralign, new_size, force,
+				   data, next_data, out_buf, out_size,
+				   block);
+}
+
+void *
+internal_function
+__libelf_decompress_zlib (void *buf_in, size_t size_in, size_t size_out)
  {
    /* Catch highly unlikely compression ratios so we don't allocate
       some giant amount of memory for nothing. The max compression
@@ -218,7 +360,7 @@  __libelf_decompress (void *buf_in, size_t size_in, size_t size_out)
        return NULL;
      }
  
-  /* Malloc might return NULL when requestion zero size.  This is highly
+  /* Malloc might return NULL when requesting zero size.  This is highly
       unlikely, it would only happen when the compression was forced.
       But we do need a non-NULL buffer to return and set as result.
       Just make sure to always allocate at least 1 byte.  */
@@ -260,6 +402,51 @@  __libelf_decompress (void *buf_in, size_t size_in, size_t size_out)
    return buf_out;
  }
  
+#ifdef USE_ZSTD
+void *
+internal_function
+__libelf_decompress_zstd (void *buf_in, size_t size_in, size_t size_out)
+{
+  /* Malloc might return NULL when requesting zero size.  This is highly
+     unlikely, it would only happen when the compression was forced.
+     But we do need a non-NULL buffer to return and set as result.
+     Just make sure to always allocate at least 1 byte.  */
+  void *buf_out = malloc (size_out ?: 1);
+  if (unlikely (buf_out == NULL))
+    {
+      __libelf_seterrno (ELF_E_NOMEM);
+      return NULL;
+    }
+
+  size_t ret = ZSTD_decompress (buf_out, size_out, buf_in, size_in);
+  if (ZSTD_isError (ret))
+    {
+      free (buf_out);
+      __libelf_seterrno (ELF_E_DECOMPRESS_ERROR);
+      return NULL;
+    }
+  else
+    return buf_out;
+}
+#endif
+
+void *
+internal_function
+__libelf_decompress (int chtype, void *buf_in, size_t size_in, size_t size_out)
+{
+  if (chtype == ELFCOMPRESS_ZLIB)
+    return __libelf_decompress_zlib (buf_in, size_in, size_out);
+  else
+    {
+#ifdef USE_ZSTD
+    return __libelf_decompress_zstd (buf_in, size_in, size_out);
+#else
+    __libelf_seterrno (ELF_E_DECOMPRESS_ERROR);
+    return NULL;
+#endif
+    }
+}
+
  void *
  internal_function
  __libelf_decompress_elf (Elf_Scn *scn, size_t *size_out, size_t *addralign)
@@ -268,7 +455,7 @@  __libelf_decompress_elf (Elf_Scn *scn, size_t *size_out, size_t *addralign)
    if (gelf_getchdr (scn, &chdr) == NULL)
      return NULL;
  
-  if (chdr.ch_type != ELFCOMPRESS_ZLIB)
+  if (chdr.ch_type != ELFCOMPRESS_ZLIB && chdr.ch_type != ELFCOMPRESS_ZSTD)
      {
        __libelf_seterrno (ELF_E_UNKNOWN_COMPRESSION_TYPE);
        return NULL;
@@ -295,7 +482,9 @@  __libelf_decompress_elf (Elf_Scn *scn, size_t *size_out, size_t *addralign)
  		  ? sizeof (Elf32_Chdr) : sizeof (Elf64_Chdr));
    size_t size_in = data->d_size - hsize;
    void *buf_in = data->d_buf + hsize;
-  void *buf_out = __libelf_decompress (buf_in, size_in, chdr.ch_size);
+  void *buf_out
+    = __libelf_decompress (chdr.ch_type, buf_in, size_in, chdr.ch_size);
+
    *size_out = chdr.ch_size;
    *addralign = chdr.ch_addralign;
    return buf_out;
@@ -394,7 +583,7 @@  elf_compress (Elf_Scn *scn, int type, unsigned int flags)
      }
  
    int compressed = (sh_flags & SHF_COMPRESSED);
-  if (type == ELFCOMPRESS_ZLIB)
+  if (type == ELFCOMPRESS_ZLIB || type == ELFCOMPRESS_ZSTD)
      {
        /* Compress/Deflate.  */
        if (compressed == 1)
@@ -408,7 +597,8 @@  elf_compress (Elf_Scn *scn, int type, unsigned int flags)
        size_t orig_size, orig_addralign, new_size;
        void *out_buf = __libelf_compress (scn, hsize, elfdata,
  					 &orig_size, &orig_addralign,
-					 &new_size, force);
+					 &new_size, force,
+					 type == ELFCOMPRESS_ZSTD);
  
        /* Compression would make section larger, don't change anything.  */
        if (out_buf == (void *) -1)
@@ -422,7 +612,7 @@  elf_compress (Elf_Scn *scn, int type, unsigned int flags)
        if (elfclass == ELFCLASS32)
  	{
  	  Elf32_Chdr chdr;
-	  chdr.ch_type = ELFCOMPRESS_ZLIB;
+	  chdr.ch_type = type;
  	  chdr.ch_size = orig_size;
  	  chdr.ch_addralign = orig_addralign;
  	  if (elfdata != MY_ELFDATA)
@@ -436,7 +626,7 @@  elf_compress (Elf_Scn *scn, int type, unsigned int flags)
        else
  	{
  	  Elf64_Chdr chdr;
-	  chdr.ch_type = ELFCOMPRESS_ZLIB;
+	  chdr.ch_type = type;
  	  chdr.ch_reserved = 0;
  	  chdr.ch_size = orig_size;
  	  chdr.ch_addralign = sh_addralign;
diff --git a/libelf/elf_compress_gnu.c b/libelf/elf_compress_gnu.c
index 3d2977e7..8e20b30e 100644
--- a/libelf/elf_compress_gnu.c
+++ b/libelf/elf_compress_gnu.c
@@ -103,7 +103,8 @@  elf_compress_gnu (Elf_Scn *scn, int inflate, unsigned int flags)
        size_t orig_size, new_size, orig_addralign;
        void *out_buf = __libelf_compress (scn, hsize, elfdata,
  					 &orig_size, &orig_addralign,
-					 &new_size, force);
+					 &new_size, force,
+					 /* use_zstd */ false);
  
        /* Compression would make section larger, don't change anything.  */
        if (out_buf == (void *) -1)
@@ -178,7 +179,7 @@  elf_compress_gnu (Elf_Scn *scn, int inflate, unsigned int flags)
        size_t size = gsize;
        size_t size_in = data->d_size - hsize;
        void *buf_in = data->d_buf + hsize;
-      void *buf_out = __libelf_decompress (buf_in, size_in, size);
+      void *buf_out = __libelf_decompress (ELFCOMPRESS_ZLIB, buf_in, size_in, size);
        if (buf_out == NULL)
  	return -1;
  
diff --git a/libelf/libelfP.h b/libelf/libelfP.h
index d88a613c..6624f38a 100644
--- a/libelf/libelfP.h
+++ b/libelf/libelfP.h
@@ -574,10 +574,10 @@  extern uint32_t __libelf_crc32 (uint32_t crc, unsigned char *buf, size_t len)
  
  extern void * __libelf_compress (Elf_Scn *scn, size_t hsize, int ei_data,
  				 size_t *orig_size, size_t *orig_addralign,
-				 size_t *size, bool force)
+				 size_t *size, bool force, bool use_zstd)
       internal_function;
  
-extern void * __libelf_decompress (void *buf_in, size_t size_in,
+extern void * __libelf_decompress (int chtype, void *buf_in, size_t size_in,
  				   size_t size_out) internal_function;
  extern void * __libelf_decompress_elf (Elf_Scn *scn,
  				       size_t *size_out, size_t *addralign)
diff --git a/src/elfcompress.c b/src/elfcompress.c
index eff765e8..b0f1677c 100644
--- a/src/elfcompress.c
+++ b/src/elfcompress.c
@@ -55,9 +55,10 @@  enum ch_type
    UNSET = -1,
    NONE,
    ZLIB,
+  ZSTD,
  
    /* Maximal supported ch_type.  */
-  MAXIMAL_CH_TYPE = ZLIB,
+  MAXIMAL_CH_TYPE = ZSTD,
  
    ZLIB_GNU = 1 << 16
  };
@@ -139,6 +140,12 @@  parse_opt (int key, char *arg __attribute__ ((unused)),
  	type = ZLIB;
        else if (strcmp ("zlib-gnu", arg) == 0 || strcmp ("gnu", arg) == 0)
  	type = ZLIB_GNU;
+      else if (strcmp ("zstd", arg) == 0)
+#ifdef USE_ZSTD
+	type = ZSTD;
+#else
+	argp_error (state, N_("ZSTD support is not enabled"));
+#endif
        else
  	argp_error (state, N_("unknown compression type '%s'"), arg);
        break;
@@ -281,6 +288,44 @@  get_sections (unsigned int *sections, size_t shnum)
    return s;
  }
  
+/* Return compression type of a given section SHDR.  */
+
+static enum ch_type
+get_section_chtype (Elf_Scn *scn, GElf_Shdr *shdr, const char *sname,
+		    size_t ndx)
+{
+  enum ch_type chtype = UNSET;
+  if ((shdr->sh_flags & SHF_COMPRESSED) != 0)
+    {
+      GElf_Chdr chdr;
+      if (gelf_getchdr (scn, &chdr) != NULL)
+	{
+	  chtype = (enum ch_type)chdr.ch_type;
+	  if (chtype == NONE)
+	    {
+	      error (0, 0, "Compression type for section %zd"
+		     " can't be zero ", ndx);
+	      chtype = UNSET;
+	    }
+	  else if (chtype > MAXIMAL_CH_TYPE)
+	    {
+	      error (0, 0, "Compression type (%d) for section %zd"
+		     " is unsupported ", chtype, ndx);
+	      chtype = UNSET;
+	    }
+	}
+      else
+	error (0, 0, "Couldn't get chdr for section %zd", ndx);
+    }
+  /* Set ZLIB_GNU compression manually for .zdebug* sections.  */
+  else if (startswith (sname, ".zdebug"))
+    chtype = ZLIB_GNU;
+  else
+    chtype = NONE;
+
+  return chtype;
+}
+
  static int
  process_file (const char *fname)
  {
@@ -461,26 +506,29 @@  process_file (const char *fname)
  
        if (section_name_matches (sname))
  	{
-	  if (!force && type == NONE
-	      && (shdr->sh_flags & SHF_COMPRESSED) == 0
-	      && !startswith (sname, ".zdebug"))
-	    {
-	      if (verbose > 0)
-		printf ("[%zd] %s already decompressed\n", ndx, sname);
-	    }
-	  else if (!force && type == ZLIB
-		   && (shdr->sh_flags & SHF_COMPRESSED) != 0)
-	    {
-	      if (verbose > 0)
-		printf ("[%zd] %s already compressed\n", ndx, sname);
-	    }
-	  else if (!force && type == ZLIB_GNU
-		   && startswith (sname, ".zdebug"))
+	  enum ch_type schtype = get_section_chtype (scn, shdr, sname, ndx);
+	  if (!force && verbose > 0)
  	    {
-	      if (verbose > 0)
-		printf ("[%zd] %s already GNU compressed\n", ndx, sname);
+	      /* The current compression matches the final one.  */
+	      if (type == schtype)
+		switch (type)
+		  {
+		  case NONE:
+		    printf ("[%zd] %s already decompressed\n", ndx, sname);
+		    break;
+		  case ZLIB:
+		  case ZSTD:
+		    printf ("[%zd] %s already compressed\n", ndx, sname);
+		    break;
+		  case ZLIB_GNU:
+		    printf ("[%zd] %s already GNU compressed\n", ndx, sname);
+		    break;
+		  default:
+		    abort ();
+		  }
  	    }
-	  else if (shdr->sh_type != SHT_NOBITS
+
+	  if (shdr->sh_type != SHT_NOBITS
  	      && (shdr->sh_flags & SHF_ALLOC) == 0)
  	    {
  	      set_section (sections, ndx);
@@ -692,37 +740,12 @@  process_file (const char *fname)
  	     (de)compressed, invalidating the string pointers.  */
  	  sname = xstrdup (sname);
  
+
  	  /* Detect source compression that is how is the section compressed
  	     now.  */
-	  GElf_Chdr chdr;
-	  enum ch_type schtype = NONE;
-	  if ((shdr->sh_flags & SHF_COMPRESSED) != 0)
-	    {
-	      if (gelf_getchdr (scn, &chdr) != NULL)
-		{
-		  schtype = (enum ch_type)chdr.ch_type;
-		  if (schtype == NONE)
-		    {
-		      error (0, 0, "Compression type for section %zd"
-			     " can't be zero ", ndx);
-		      goto cleanup;
-		    }
-		  else if (schtype > MAXIMAL_CH_TYPE)
-		    {
-		      error (0, 0, "Compression type (%d) for section %zd"
-			     " is unsupported ", schtype, ndx);
-		      goto cleanup;
-		    }
-		}
-	      else
-		{
-		  error (0, 0, "Couldn't get chdr for section %zd", ndx);
-		  goto cleanup;
-		}
-	    }
-	  /* Set ZLIB compression manually for .zdebug* sections.  */
-	  else if (startswith (sname, ".zdebug"))
-	    schtype = ZLIB_GNU;
+	  enum ch_type schtype = get_section_chtype (scn, shdr, sname, ndx);
+	  if (schtype == UNSET)
+	    goto cleanup;
  
  	  /* We might want to decompress (and rename), but not
  	     compress during this pass since we might need the section
@@ -754,7 +777,7 @@  process_file (const char *fname)
  	    case ZLIB_GNU:
  	      if (startswith (sname, ".debug"))
  		{
-		  if (schtype == ZLIB)
+		  if (schtype == ZLIB || schtype == ZSTD)
  		    {
  		      /* First decompress to recompress GNU style.
  			 Don't report even when verbose.  */
@@ -818,19 +841,22 @@  process_file (const char *fname)
  	      break;
  
  	    case ZLIB:
-	      if ((shdr->sh_flags & SHF_COMPRESSED) == 0)
+	    case ZSTD:
+	      if (schtype != type)
  		{
-		  if (schtype == ZLIB_GNU)
+		  if (schtype != NONE)
  		    {
-		      /* First decompress to recompress zlib style.
-			 Don't report even when verbose.  */
+		      /* Decompress first.  */
  		      if (compress_section (scn, size, sname, NULL, ndx,
  					    schtype, NONE, false) < 0)
  			goto cleanup;
  
-		      snamebuf[0] = '.';
-		      strcpy (&snamebuf[1], &sname[2]);
-		      newname = snamebuf;
+		      if (schtype == ZLIB_GNU)
+			{
+			  snamebuf[0] = '.';
+			  strcpy (&snamebuf[1], &sname[2]);
+			  newname = snamebuf;
+			}
  		    }
  
  		  if (skip_compress_section)
@@ -838,7 +864,7 @@  process_file (const char *fname)
  		      if (ndx == shdrstrndx)
  			{
  			  shstrtab_size = size;
-			  shstrtab_compressed = ZLIB;
+			  shstrtab_compressed = type;
  			  if (shstrtab_name != NULL
  			      || shstrtab_newname != NULL)
  			    {
@@ -855,7 +881,7 @@  process_file (const char *fname)
  		      else
  			{
  			  symtab_size = size;
-			  symtab_compressed = ZLIB;
+			  symtab_compressed = type;
  			  symtab_name = xstrdup (sname);
  			  symtab_newname = (newname == NULL
  					    ? NULL : xstrdup (newname));
@@ -1378,7 +1404,7 @@  main (int argc, char **argv)
  	N_("Place (de)compressed output into FILE"),
  	0 },
        { "type", 't', "TYPE", 0,
-	N_("What type of compression to apply. TYPE can be 'none' (decompress), 'zlib' (ELF ZLIB compression, the default, 'zlib-gabi' is an alias) or 'zlib-gnu' (.zdebug GNU style compression, 'gnu' is an alias)"),
+	N_("What type of compression to apply. TYPE can be 'none' (decompress), 'zlib' (ELF ZLIB compression, the default, 'zlib-gabi' is an alias), 'zlib-gnu' (.zdebug GNU style compression, 'gnu' is an alias) or 'zstd'"),
  	0 },
        { "name", 'n', "SECTION", 0,
  	N_("SECTION name to (de)compress, SECTION is an extended wildcard pattern (defaults to '.?(z)debug*')"),
diff --git a/src/readelf.c b/src/readelf.c
index cc3e0229..451f8400 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -1238,13 +1238,17 @@  get_visibility_type (int value)
  static const char *
  elf_ch_type_name (unsigned int code)
  {
-  if (code == 0)
-    return "NONE";
-
-  if (code == ELFCOMPRESS_ZLIB)
-    return "ZLIB";
-
-  return "UNKNOWN";
+  switch (code)
+    {
+    case 0:
+      return "NONE";
+    case ELFCOMPRESS_ZLIB:
+      return "ZLIB";
+    case ELFCOMPRESS_ZSTD:
+      return "ZSTD";
+    default:
+      return "UNKNOWN";
+    }
  }
  
  /* Print the section headers.  */
diff --git a/tests/run-compress-test.sh b/tests/run-compress-test.sh
index a6a298f5..3f9c990e 100755
--- a/tests/run-compress-test.sh
+++ b/tests/run-compress-test.sh
@@ -61,6 +61,30 @@  testrun_elfcompress_file()
      echo "uncompress $elfcompressedfile -> $elfuncompressedfile"
      testrun ${abs_top_builddir}/src/elfcompress -v -t none -o ${elfuncompressedfile} ${elfcompressedfile}
      testrun ${abs_top_builddir}/src/elfcmp ${uncompressedfile} ${elfuncompressedfile}
+
+    outputfile="${infile}.gabi.zstd"
+    tempfiles "$outputfile"
+    echo "zstd compress $elfcompressedfile -> $outputfile"
+    testrun ${abs_top_builddir}/src/elfcompress -v -t zstd -o ${outputfile} ${elfcompressedfile}
+    testrun ${abs_top_builddir}/src/elfcmp ${uncompressedfile} ${outputfile}
+    echo "checking compressed section header" $outputfile
+    testrun ${abs_top_builddir}/src/readelf -Sz ${outputfile} | grep "ELF ZSTD" >/dev/null
+
+    zstdfile="${infile}.zstd"
+    tempfiles "$zstdfile"
+    echo "zstd compress $uncompressedfile -> $zstdfile"
+    testrun ${abs_top_builddir}/src/elfcompress -v -t zstd -o ${zstdfile} ${elfuncompressedfile}
+    testrun ${abs_top_builddir}/src/elfcmp ${uncompressedfile} ${zstdfile}
+    echo "checking compressed section header" $zstdfile
+    testrun ${abs_top_builddir}/src/readelf -Sz ${zstdfile} | grep "ELF ZSTD" >/dev/null
+
+    zstdgnufile="${infile}.zstd.gnu"
+    tempfiles "$zstdgnufile"
+    echo "zstd re-compress to GNU ZLIB $zstdfile -> $zstdgnufile"
+    testrun ${abs_top_builddir}/src/elfcompress -v -t zlib-gnu -o ${zstdgnufile} ${zstdfile}
+    testrun ${abs_top_builddir}/src/elfcmp ${uncompressedfile} ${zstdgnufile}
+    echo "checking .zdebug section name" $zstdgnufile
+    testrun ${abs_top_builddir}/src/readelf -S ${zstdgnufile} | grep ".zdebug" >/dev/null
  }
  
  testrun_elfcompress()