[PATCH] tests: Create random test_dir name
The testsuite relies on there being no files in the test directory
after the test finishes. A test will fail if the test dir cannot be
removed. But the test dir isn't really random, it uses the pid of the
shell script that executes the test. On some of the buildbots that
execute a lot of tests it can happen that the pid number wraps around
and a pid of a previous pid is reused. To prevent that happening
generate a real random number (8 bytes) using od /dev/urandom and
xargs (to trim away spaces left by od).
* tests/test-subr.sh: Define test_name and random_number and use
those to define test_dir.
Signed-off-by: Mark Wielaard
---
tests/test-subr.sh | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tests/test-subr.sh b/tests/test-subr.sh
index ea80cbec3bc7..2a956b47de2f 100644
--- a/tests/test-subr.sh
+++ b/tests/test-subr.sh
@@ -23,7 +23,9 @@
set -e
# Each test runs in its own directory to make sure they can run in parallel.
-test_dir="${TMPDIR-/var/tmp}/elfutils-test-$$"
+test_name=$(basename $0)
+random_number=$(od -An -N8 -tx8 /dev/urandom | xargs)
+test_dir="${TMPDIR-/var/tmp}/elfutils-test-$test_name.${random_number}"
mkdir -p "$test_dir"
orig_dir="${PWD}"
cd "$test_dir"
--
2.49.0
Re: [PATCH v2] src/readelf.c: Access symbol and version data only if available
Hi Aaron, Hi Constantine,
On Mon, 2025-05-05 at 10:44 -0400, Aaron Merey wrote:
> handle_dynamic_symtab can attempt to read symbol and version data from
> file offset of 0 or address of 0 if the associated DT_ tags aren't found.
>
> Fix this by only reading symbol and version data when non-zero file
> offsets/addresses have been found.
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=32864
>
> Suggested-by: Constantine Bytensky
> Signed-off-by: Aaron Merey
> ---
> v2 changes: Added checks for unset addrs and missing symbol version data.
>
> src/readelf.c | 82 ++-
> 1 file changed, 49 insertions(+), 33 deletions(-)
>
> diff --git a/src/readelf.c b/src/readelf.c
> index 8603b3c4..e9df0344 100644
> --- a/src/readelf.c
> +++ b/src/readelf.c
> @@ -3047,53 +3047,69 @@ handle_dynamic_symtab (Ebl *ebl)
>Elf_Data *verdef_data = NULL;
>Elf_Data *verneed_data = NULL;
>
> - symdata = elf_getdata_rawchunk (
> - ebl->elf, offs[i_symtab],
> - gelf_fsize (ebl->elf, ELF_T_SYM, syments, EV_CURRENT), ELF_T_SYM);
> - symstrdata = elf_getdata_rawchunk (ebl->elf, offs[i_strtab],
> addrs[i_strsz],
> - ELF_T_BYTE);
> - versym_data = elf_getdata_rawchunk (
> - ebl->elf, offs[i_versym], syments * sizeof (Elf64_Half), ELF_T_HALF);
> + if (offs[i_symtab] != 0)
> +symdata = elf_getdata_rawchunk (
> + ebl->elf, offs[i_symtab],
> + gelf_fsize (ebl->elf, ELF_T_SYM, syments, EV_CURRENT), ELF_T_SYM);
> +
> + if (offs[i_strtab] != 0 && addrs[i_strsz] != 0)
> +symstrdata = elf_getdata_rawchunk (ebl->elf, offs[i_strtab],
> addrs[i_strsz],
> +ELF_T_BYTE);
> +
> + if (offs[i_versym] != 0)
> +versym_data = elf_getdata_rawchunk (
> + ebl->elf, offs[i_versym], syments * sizeof (Elf64_Half), ELF_T_HALF);
>
>/* Get the verneed_data without vernaux. */
> - verneed_data = elf_getdata_rawchunk (
> - ebl->elf, offs[i_verneed], addrs[i_verneednum] * sizeof
> (Elf64_Verneed),
> - ELF_T_VNEED);
> + if (offs[i_verneed] != 0 && addrs[i_verneednum] != 0)
> +verneed_data = elf_getdata_rawchunk (
> + ebl->elf, offs[i_verneed], addrs[i_verneednum] * sizeof (Elf64_Verneed),
> + ELF_T_VNEED);
> +
All looks good.
>size_t vernauxnum = 0;
>size_t vn_next_offset = 0;
>
> - for (size_t i = 0; i < addrs[i_verneednum]; i++)
> -{
> - GElf_Verneed *verneed
> - = (GElf_Verneed *)(verneed_data->d_buf + vn_next_offset);
> - vernauxnum += verneed->vn_cnt;
> - vn_next_offset += verneed->vn_next;
> -}
> + if (verneed_data != NULL && verneed_data->d_buf != NULL
> + && addrs[i_verneednum] != 0)
> +for (size_t i = 0; i < addrs[i_verneednum]; i++)
> + {
> + GElf_Verneed *verneed
> + = (GElf_Verneed *)(verneed_data->d_buf + vn_next_offset);
> + vernauxnum += verneed->vn_cnt;
> + vn_next_offset += verneed->vn_next;
> + }
The last && addrs[i_verneednum] != 0 is ok, but redundant because of
the i < addrs[i_verneednum] (which would immediately succeed and so
also skips the loop).
But if we are going to make this robust then there is another (pre-
existing) issue. We keep reading the data from the d_buf as a
GElf_Verneed and use verneed->vn_next to give us the next offset. We
need to check if that is sane. First we need to check verneed_data-
>d_size is at least sizeof (GElf_Verneed) and then vn_next_offset must
always be smaller than verneed_data->d_size - sizeof (GElf_Verneed).
>/* Update the verneed_data to include the vernaux. */
> - verneed_data = elf_getdata_rawchunk (
> - ebl->elf, offs[i_verneed],
> - (addrs[i_verneednum] + vernauxnum) * sizeof (GElf_Verneed),
> ELF_T_VNEED);
> + if (offs[i_verneed] != 0 && addrs[i_verneednum] != 0)
> +verneed_data = elf_getdata_rawchunk (
> + ebl->elf, offs[i_verneed],
> + (addrs[i_verneednum] + vernauxnum) * sizeof (GElf_Verneed),
> + ELF_T_VNEED);
>
>/* Get the verdef_data without verdaux. */
> - verdef_data = elf_getdata_rawchunk (
> - ebl->elf, offs[i_verdef], addrs[i_verdefnum] * sizeof (Elf64_Verdef),
> - ELF_T_VDEF);
> + if (offs[i_verdef] != 0 && addrs[i_verdefnum] != 0)
> +verdef_data = elf_getdata_rawchunk (
> + ebl->elf, offs[i_verdef], addrs[i_verdefnum] * sizeof (Elf64_Verdef),
> + ELF_T_VDEF);
> +
Looks good.
>size_t verdauxnum = 0;
>size_t vd_next_offset = 0;
>
> - for (size_t i = 0; i < addrs[i_verdefnum]; i++)
> -{
> - GElf_Verdef *verdef
> - = (GElf_Verdef *)(verdef_data->d_buf + vd_next_offset);
> - verdauxnum += verdef->vd_cnt;
> - vd_next_offset += verdef->vd_next;
> -}
> + if (verdef_data != NULL && verdef_data->d_buf != NULL
> + && addrs[i_verdefnum] != 0)
> +for (size_t i = 0; i < addrs[i_verdefnum]; i++)
> + {
> + GElf_Verdef *verdef
> + = (GElf_Verdef *)(verdef_data->d_buf + vd_next_off
Re: [PATCH] PR32930 backends/: guard asm/perf_regs.h include
Hi Serhei, On Mon, May 5, 2025 at 12:26 PM Serhei Makarov wrote: > > asm/perf_regs.h is an arch-specific linux include, not present on > architectures like hppa and m68k that lack perf_events support. > > Only one place we need to fix; others already guard the include by > architecture, or use architecture-independent headers (e.g. > linux/perf_events.h). > > * backends/libebl_PERF_FLAGS.h: Only include asm/perf_regs.h on > architectures where we use it. > --- > backends/libebl_PERF_FLAGS.h | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/backends/libebl_PERF_FLAGS.h b/backends/libebl_PERF_FLAGS.h > index 2ed45f0f..51c20ea6 100644 > --- a/backends/libebl_PERF_FLAGS.h > +++ b/backends/libebl_PERF_FLAGS.h > @@ -32,8 +32,12 @@ > #define _LIBEBL_PERF_FLAGS_H 1 > > #if defined(__linux__) > +/* XXX Need to exclude __linux__ arches without perf_regs.h. */ > +#if defined(__x86_64__) || defined(__i386__) > +/* || defined(other_architecture)... */ > # include > #endif > +#endif > > #if defined(_ASM_X86_PERF_REGS_H) > /* See the code in x86_initreg_sample.c for list of required regs and > @@ -49,8 +53,8 @@ > see the code in tools/perf/util/intel-pt.c intel_pt_add_gp_regs() > and note how regs are added in the same order as the perf_regs.h enum. */ > #else > -/* Since asm/perf_regs.h gives the register layout for a different arch, > - we can't unwind x86_64 frames. */ > +/* Since asm/perf_regs.h is absent, or gives the register layout for a > + different arch, we can't unwind i386 and x86_64 frames. */ > #define PERF_FRAME_REGISTERS_I386 0 > #define PERF_FRAME_REGISTERS_X86_64 0 > #endif > -- > 2.47.0 > So if the arch is not x86_64 or i386, then PERF_FRAME_REGISTERS_* will be 0 and this signals to the backend to skip any unwinding. LGTM. Aaron
☺ Buildbot (Sourceware): elfutils - build successful (main)
A restored build has been detected on builder elfutils-debian-armhf while building elfutils. Full details are available at: https://builder.sourceware.org/buildbot/#/builders/6/builds/412 Build state: build successful Revision: d05241ce690018548857199bd34d7734c7164679 Worker: debian-armhf Build Reason: (unknown) Blamelist: Mark Wielaard , Serhei Makarov Steps: - 0: worker_preparation ( success ) - 1: set package name ( success ) - 2: git checkout ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/6/builds/412/steps/2/logs/stdio - 3: autoreconf ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/6/builds/412/steps/3/logs/stdio - 4: configure ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/6/builds/412/steps/4/logs/stdio - config.log: https://builder.sourceware.org/buildbot/#/builders/6/builds/412/steps/4/logs/config_log - 5: get version ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/6/builds/412/steps/5/logs/stdio - property changes: https://builder.sourceware.org/buildbot/#/builders/6/builds/412/steps/5/logs/property_changes - 6: make ( warnings ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/6/builds/412/steps/6/logs/stdio - warnings (3): https://builder.sourceware.org/buildbot/#/builders/6/builds/412/steps/6/logs/warnings__3_ - 7: make check ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/6/builds/412/steps/7/logs/stdio - test-suite.log: https://builder.sourceware.org/buildbot/#/builders/6/builds/412/steps/7/logs/test-suite_log - 8: prep ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/6/builds/412/steps/8/logs/stdio - 9: build bunsen.cpio.gz ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/6/builds/412/steps/9/logs/stdio - 10: fetch bunsen.cpio.gz ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/6/builds/412/steps/10/logs/stdio - 11: unpack bunsen.cpio.gz ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/6/builds/412/steps/11/logs/stdio - 12: pass .bunsen.source.* ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/6/builds/412/steps/12/logs/stdio - 13: upload to bunsen ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/6/builds/412/steps/13/logs/stdio - 14: clean up ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/6/builds/412/steps/14/logs/stdio - 15: make distclean ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/6/builds/412/steps/15/logs/stdio
Re: [PATCH] PR32930 backends/: guard asm/perf_regs.h include
On Tue, May 6, 2025, at 11:24 AM, Aaron Merey wrote: > So if the arch is not x86_64 or i386, then PERF_FRAME_REGISTERS_* will > be 0 and this signals to the backend to skip any unwinding. LGTM. Yep, will expand the #ifdef as more architectures are added. Committed. -- All the best, Serhei http://serhei.io
