From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (gnu.wildebeest.org [45.83.234.184]) by sourceware.org (Postfix) with ESMTPS id AC287385841D for ; Fri, 16 Dec 2022 00:32:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AC287385841D Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org Received: from reform (deer0x16.wildebeest.org [172.31.17.152]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id B7997308B8B5; Fri, 16 Dec 2022 01:32:01 +0100 (CET) Received: by reform (Postfix, from userid 1000) id 3FE0F2E803BA; Fri, 16 Dec 2022 01:32:01 +0100 (CET) Date: Fri, 16 Dec 2022 01:32:01 +0100 From: Mark Wielaard To: Martin =?utf-8?B?TGnFoWth?= Cc: elfutils-devel@sourceware.org Subject: Re: [PATCHv2] support ZSTD compression algorithm Message-ID: References: <24d7165b-b8ac-ea5d-a046-aec2203696a9@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Spam-Status: No, score=-3038.5 required=5.0 tests=BAYES_00,GIT_PATCH_0,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 > #include > +#ifdef USE_ZSTD > +#include > +#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