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 BC4DB3858C2C for ; Fri, 28 Oct 2022 22:21:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BC4DB3858C2C 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 (deer0x15.wildebeest.org [172.31.17.151]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id 91AFB330CD41; Sat, 29 Oct 2022 00:21:06 +0200 (CEST) Received: by reform (Postfix, from userid 1000) id 23E992E82304; Sat, 29 Oct 2022 00:21:06 +0200 (CEST) Date: Sat, 29 Oct 2022 00:21:06 +0200 From: Mark Wielaard To: Martin =?utf-8?B?TGnFoWth?= Cc: "Dmitry V. Levin" , elfutils-devel@sourceware.org, Fangrui Song Subject: Re: [PATCH][RFC] readelf: partial support of ZSTD compression Message-ID: References: <542eb279-d15d-6f17-02c0-c53fd0f33055@suse.cz> <20221024114137.GA19251@altlinux.org> <0375dd0c-2410-d1ca-8ce7-41293e8e2fa2@suse.cz> <20221024164806.GA21412@altlinux.org> <6c1ce1f1-2e45-20bb-e98d-6d35692addfb@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <6c1ce1f1-2e45-20bb-e98d-6d35692addfb@suse.cz> X-Spam-Status: No, score=-3039.1 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 Mon, Oct 24, 2022 at 08:16:09PM +0200, Martin Liška wrote: > Anyway, I'm sending V2 that works fine --with-zstd and --without-zstd as expected. > > Ready for master? The decompression part looks good. Few small nitpicks below. Although I like to also have compression working. Then we can also add support to src/elfcompress, which makes for a good testcase. See tests/run-compress.sh (which has found some subtle memory issues when run under valgrind). > From 4aea412783b9b0dcaf0f887947bf2e8ee6c5368b Mon Sep 17 00:00:00 2001 > From: Martin Liska > Date: Mon, 24 Oct 2022 11:53:13 +0200 > Subject: [PATCH] readelf: partial support of ZSTD compression > > Support decompression of ZSTD sections and add support > for it when -SWz is used: > > ... > [30] .debug_abbrev PROGBITS 0000000000000000 00001f9d 00000168 0 C 0 0 1 > [ELF ZSTD (2) 000002fc 1] > ... And for this it would be nice to have a tests/run-readelf-z.sh testcase. > ChangeLog: > > * configure.ac: Add zstd_LIBS. > > libelf/ChangeLog: > > * Makefile.am: Use zstd_LIBS. > * elf.h (ELFCOMPRESS_ZSTD): Add new value. > * elf_compress.c (__libelf_decompress): Dispatch based > on the compression algorithm. > (__libelf_decompress_zlib): New. > (__libelf_decompress_zstd): New. > (__libelf_decompress_elf): Pass type of compression to > __libelf_decompress. > * elf_compress_gnu.c (elf_compress_gnu): Use ELFCOMPRESS_ZLIB > as .z* sections can be only compressed with ZLIB. > * libelfP.h (__libelf_decompress): Change signature. > > src/ChangeLog: > > * readelf.c (elf_ch_type_name): Use switch and support ZSTD. > diff --git a/libelf/elf.h b/libelf/elf.h > index 02a1b3f5..f0f0ec7d 100644 > --- a/libelf/elf.h > +++ b/libelf/elf.h We normally sync elf.h from glibc. I'll do that in a second. > +#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 requestion zero size. This is highly requesting > + 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 * > +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 > + return 0; > +#endif return NULL for consistency? > + } > +} > + > void * > internal_function > __libelf_decompress_elf (Elf_Scn *scn, size_t *size_out, size_t *addralign) > @@ -268,7 +316,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) > { What about the ifndef USE_ZSTD case? Should this then not recognize ELFCOMPRESS_ZSTD? Thanks, Mark