Re: ☠ Buildbot (GNU Toolchain): elfutils-try-debian-armhf - failed compile (failure) (users/marxin/try-zstd-support-v2)
On 12/22/22 00:38, Mark Wielaard wrote: > Hi Martin, > > On Wed, Dec 21, 2022 at 07:29:19PM +0100, Martin Liška wrote: >>> What goes wrong is that this debian old stable arm setup has libzstd >>> containing a ZSTD_compressStream2 symbol... >>> >>> 40: d349 264 FUNCGLOBAL DEFAULT 11 ZSTD_compressStream2 >>> >>> But the zstd.h header doesn't expose it... >> >> Yeah, that's their way of how to move an API from "staging" into a stable >> state. >> It was changed in: >> https://github.com/facebook/zstd/commit/d7d89513d6a21 >> >> and so it's present in zstd since 1.4.0. Can we somehow specify library >> version >> in configure.ac? > Hi Mark. > It isn't as nice, but since there seems to be a pkgconfig libzstd.pc > you could use PKG_CHECK_MODULES. See for example how we check for > libcurl. I can see it, however, it not easily addable to eu_ZIPLIB function :/ I must confess, this autoconf machinery is always something that makes me crazy :) Do you have an elegant way how to handle it? Thanks, Martin > > Cheers, > > Mark
[PATCHv2] support ZSTD compression algorithm
Hi.
> Is the abort () at the second call site because that cannot happen? Or
> should that also goto cleanup?
Yes, it can't happen.
There's V2 (including ChangeLog) where all issues apart from the libzstd
configure.ac detection
should be addressed.
Cheers,
Martin
config/ChangeLog:
* libelf.pc.in: Add LIBLZSTD to Requires.private.
ChangeLog:
* configure.ac: Detect ZSTD streaming API.
libelf/ChangeLog:
* Makefile.am: Use zstd_LIBS.
* elf_compress.c:
(__libelf_compress): Split into ...
(__libelf_compress_zlib): ... this.
(do_zstd_cleanup): New.
(zstd_cleanup): New.
(__libelf_compress_zstd): New.
(__libelf_decompress): Switch in between zlib and zstd.
(__libelf_decompress_zlib): Renamed from __libelf_decompress.
(__libelf_decompress_zstd): New.
(__libelf_decompress_elf): Dispatch in between compression
algorithms.
(elf_compress): Likewise.
* elf_compress_gnu.c (elf_compress_gnu): Call with
ELFCOMPRESS_ZLIB.
* libelfP.h (__libelf_compress): Add new argument.
(__libelf_decompress): Add chtype argument.
src/ChangeLog:
* elfcompress.c (enum ch_type): Add ZSTD.
(parse_opt): Parse "zstd".
(get_section_chtype): New.
(process_file): Support zstd compression.
(main): Add zstd to help.
* readelf.c (elf_ch_type_name): Rewrite with switch.
tests/ChangeLog:
* Makefile.am: Add ELFUTILS_ZSTD if zstd is enabled.
* run-compress-test.sh: Test zstd compression algorithm
for debug sections.
---
config/libelf.pc.in| 2 +-
configure.ac | 4 +-
libelf/Makefile.am | 2 +-
libelf/elf_compress.c | 307 +++--
libelf/elf_compress_gnu.c | 5 +-
libelf/libelfP.h | 4 +-
src/elfcompress.c | 145 +++---
src/readelf.c | 18 ++-
tests/Makefile.am | 1 +
tests/run-compress-test.sh | 28
10 files changed, 392 insertions(+), 124 deletions(-)
diff --git a/config/libelf.pc.in b/config/libelf.pc.in
index 48f3f021..0d2ce968 100644
--- a/config/libelf.pc.in
+++ b/config/libelf.pc.in
@@ -11,4 +11,4 @@ URL: http://elfutils.org/
Libs: -L${libdir} -lelf
Cflags: -I${includedir}
-Requires.private: zlib
+Requires.private: zlib @LIBZSTD@
diff --git a/configure.ac b/configure.ac
index 59be27ac..ef16f79e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -417,9 +417,11 @@ 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)])
+eu_ZIPLIB(zstd,ZSTD,zstd,ZSTD_compressStream2,[ZSTD (zst)])
AS_IF([test "x$with_zstd" = xyes], [LIBZSTD="libzstd"], [LIBLZSTD=""])
AC_SUBST([LIBZSTD])
+zstd_LIBS="$LIBS"
+AC_SUBST([zstd_LIBS])
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..deb585b7 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,
@@ -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-dis
Re: ☠ Buildbot (GNU Toolchain): elfutils-try-debian-armhf - failed compile (failure) (users/marxin/try-zstd-support-v2)
On Thu, 2022-12-22 at 10:09 +0100, Martin Liška wrote:
> On 12/22/22 00:38, Mark Wielaard wrote:
> > Hi Martin,
> >
> > On Wed, Dec 21, 2022 at 07:29:19PM +0100, Martin Liška wrote:
> > > > What goes wrong is that this debian old stable arm setup has
> > > > libzstd
> > > > containing a ZSTD_compressStream2 symbol...
> > > >
> > > > 40: d349 264 FUNCGLOBAL DEFAULT 11
> > > > ZSTD_compressStream2
> > > >
> > > > But the zstd.h header doesn't expose it...
> > >
> > > Yeah, that's their way of how to move an API from "staging" into
> > > a stable state.
> > > It was changed in:
> > > https://github.com/facebook/zstd/commit/d7d89513d6a21
> > >
> > > and so it's present in zstd since 1.4.0. Can we somehow specify
> > > library version
> > > in configure.ac?
>
> Hi Mark.
>
> > It isn't as nice, but since there seems to be a pkgconfig
> > libzstd.pc
> > you could use PKG_CHECK_MODULES. See for example how we check for
> > libcurl.
>
> I can see it, however, it not easily addable to eu_ZIPLIB function :/
> I must confess,
> this autoconf machinery is always something that makes me crazy :)
>
> Do you have an elegant way how to handle it?
Maybe we should simply split it from the "decompress" support. Trying
to make eu_ZIPLIB handle it means we also suddenly require a newer
libzstd for the simpler dwfl zstd file decompression support.
So lets keep the "normal" zstd detection in place and add something
like the attached (on top of your latest patch).
Cheers,
Mark
diff --git a/configure.ac b/configure.ac
index ef16f79e..b2597f8b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -417,7 +417,7 @@ 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_compressStream2,[ZSTD (zst)])
+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"
@@ -426,6 +426,16 @@ zip_LIBS="$LIBS"
LIBS="$save_LIBS"
AC_SUBST([zip_LIBS])
+dnl zstd compression support requires libzstd 1.4.0+
+AS_IF([test "x$with_zstd" = xyes], [
+ PKG_PROG_PKG_CONFIG
+ PKG_CHECK_MODULES([ZSTD_COMPRESS],[libzstd >= 1.4.0],
+[with_zstd_compress="yes"],[with_zstd_compress="no"])],
+ [with_zstd_compress="no"])
+AM_CONDITIONAL(USE_ZSTD_COMPRESS, test "x$with_zstd_compress" = "xyes")
+AS_IF([test "x$with_zstd_compress" = "xyes"],
+ [AC_DEFINE([USE_ZSTD_COMPRESS], [1], [zstd compression support])])
+
AC_CHECK_DECLS([memrchr, rawmemchr],[],[],
[#define _GNU_SOURCE
#include ])
@@ -831,6 +841,7 @@ AC_MSG_NOTICE([
bzip2 support : ${with_bzlib}
lzma/xz support: ${with_lzma}
zstd support : ${with_zstd}
+zstd compression support : ${with_zstd_compress}
libstdc++ demangle support : ${enable_demangler}
File textrel check : ${enable_textrelcheck}
Symbol versioning : ${enable_symbol_versioning}
diff --git a/libelf/elf_compress.c b/libelf/elf_compress.c
index deb585b7..5a9d3da1 100644
--- a/libelf/elf_compress.c
+++ b/libelf/elf_compress.c
@@ -170,7 +170,7 @@ __libelf_compress_zlib (Elf_Scn *scn, size_t hsize, int ei_data,
return out_buf;
}
-#ifdef USE_ZSTD
+#ifdef USE_ZSTD_COMPRESS
/* Cleanup and return result. Don't leak memory. */
static void *
do_zstd_cleanup (void *result, ZSTD_CCtx * const cctx, void *out_buf,
@@ -333,7 +333,7 @@ __libelf_compress (Elf_Scn *scn, size_t hsize, int ei_data,
if (use_zstd)
{
-#ifdef USE_ZSTD
+#ifdef USE_ZSTD_COMPRESS
return __libelf_compress_zstd (scn, hsize, ei_data, orig_size,
orig_addralign, new_size, force,
data, next_data, out_buf, out_size,
@@ -406,8 +406,7 @@ __libelf_decompress_zlib (void *buf_in, size_t size_in, size_t size_out)
}
#ifdef USE_ZSTD
-void *
-internal_function
+static void *
__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
@@ -425,7 +424,7 @@ __libelf_decompress_zstd (void *buf_in, size_t size_in, size_t size_out)
if (ZSTD_isError (ret))
{
free (buf_out);
- __libelf_seterrno (ELF_E_UNKNOWN_COMPRESSION_TYPE);
+ __libelf_seterrno (ELF_E_DECOMPRESS_ERROR);
return NULL;
}
else
@@ -444,7 +443,7 @@ __libelf_decompress (int chtype, void *buf_in, size_t size_in, size_t size_out)
#ifdef USE_ZSTD
return __libelf_decompress_zstd (buf_in, size_in, size_out);
#else
-__libelf_seterrno (ELF_E_DECOMPRESS_ERROR);
+__libelf_seterrno (ELF_E_UNKNOWN_COMPRESSION_TYPE);
return NULL;
#endif
}
diff --git a/src/elfcompress.c b/src/elfcompress.c
index bfdac2b4..1f32331c 100644
--- a/src/elfcompress.c
+++ b/src/elfcompress.c
@@ -230,7 +2
Re: [PATCHv2] support ZSTD compression algorithm
Hi Martin,
On Thu, 2022-12-22 at 10:17 +0100, Martin Liška wrote:
> Is the abort () at the second call site because that cannot happen?
> > Or should that also goto cleanup?
>
> Yes, it can't happen.
Aha, because it should already have seen that section before, in which
case it would have errored out.
> There's V2 (including ChangeLog) where all issues apart from the
> libzstd configure.ac detection should be addressed.
See also my other email. I think that addresses everything. Just merge
those and push please.
> diff --git a/configure.ac b/configure.ac
> index 59be27ac..ef16f79e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -417,9 +417,11 @@ 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)])
> +eu_ZIPLIB(zstd,ZSTD,zstd,ZSTD_compressStream2,[ZSTD (zst)])
> AS_IF([test "x$with_zstd" = xyes], [LIBZSTD="libzstd"],
> [LIBLZSTD=""])
> AC_SUBST([LIBZSTD])
> +zstd_LIBS="$LIBS"
> +AC_SUBST([zstd_LIBS])
> zip_LIBS="$LIBS"
> LIBS="$save_LIBS"
> AC_SUBST([zip_LIBS])
See the other email for an idea how to keep most of the checks the
same, but just for decompress (in the case of zstd). Then define a
USE_ZSTD_COMPRESS only for new enough libzstd.
> diff --git a/libelf/elf_compress.c b/libelf/elf_compress.c
> index d7f53af2..deb585b7 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,
> @@ -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)
Missed this in my other patch, but __libelf_compress_zlib also isn't an
internal_function, it can/must just be static.
> [...]
> +#ifdef USE_ZSTD
So this is the compression part, guard that with USE_ZSTD_COMPRESS.
> +
> void *
> internal_function
> -__libelf_decompress (void *buf_in, size_t size_in, size_t size_out)
> +__libelf_decompress_zlib (void *buf_in, size_t size_in, size_t
> size_out)
> {
So this isn't an internal function, just mark it static void *.
> +#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_UNKNOWN_COMPRESSION_TYPE);
This should be 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);
And this should be ELF_E_UNKNOWN_COMPRESSION_TYPE.
> +return NULL;
> +#endif
> +}
> +}
> diff --git a/src/elfcompress.c b/src/elfcompress.c
> index eff765e8..bfdac2b4 100644
> --- a/src/elfcompress.c
> +++ b/src/elfcompress.c
> @@ -55,9 +55,10 @@ enum ch_type
>UNSET =
