Re: [PATCH 9/9] doc/Makefile.am: Add man pages
On Tue, Oct 15, 2024 at 6:01 PM Mark Wielaard wrote: > > On Wed, Oct 02, 2024 at 10:26:10PM -0400, Aaron Merey wrote: > > Add the following man pages to notrans_dist_man3_MANS: > > > > elf32_xlatetom.3 elf64_xlatetom.3 elf32_xlatetof.3 elf64_xlatetof.3 > > elf32_newphdr.3 elf64_newphdr.3 elf32_newehdr.3 elf64_newehdr.3 > > elf32_getshdr.3 elf64_getshdr.3 elf32_getphdr.3 elf64_getphdr.3 > > elf32_getchdr.3 elf64_getchdr.3 elf32_fsize.3 elf64_fsize.3 > > elf32_checksum.3 elf64_checksum.3 > > Looks good. > Maybe could have been added when the individual files were added. > But it should be fine to add them all at the end. Thanks Mark. I fixed the typos you pointed out and pushed this series. Aaron
Re: [PATCH] PR32218: debuginfod-client: support very long source file
Hi Frank, On Tue, 2024-10-15 at 18:12 -0400, Frank Ch. Eigler wrote: > > Is this 4-byte hash enough to prevent accidental collisions? > > Yeah, it only needs to be uniqueish among source files of the same > binary, and with the same #-escaped tail string. Aha, it is "under" an unique build-id already. > > > This is a transparent change to clients, who are not to make any > > > assumptions about cache file naming structure. > > > > How does it interact with existing source file cache files? > > Existing files will be ignored. All files will be periodically > garbage collected. OK, so for source files an update will kind of invalidate the cache. That is no major disaster, just a minor inconvenience. > > [...] > > Are you sure you want to use elf_hash? It returns an (arch dependent) > > unsigned long int but the actual implementation only produces an > > unsigned int. To simplify things and not need extra build dependencies > > you might want to just include a hand-coded string hash function like > > djb2? Which is really just 4 lines of code and probably a better hash > > for strings than elf_hash. > > Good idea, done. Nice. It looks even simpler than I thought. > > [...] > > What do these "Reduce" comments mean? > > So we hope to get at least the original input path length plus 10 (8 > > char hash plus dash plus zero), but we are fine with less. > > New comments elaborate on this. > > > > [...] > > OK, so this always adds (or overwrites) a 8 char hash plus dash '-' in > > front. Have you considered to only add it if the src doesn't fit the > > dest_len? > > Yes, new comments explain why. Thanks, these new comments are clear. And also explain why we always want the hash-prefix to guard against "escape collisions". > > [...] > > sizeof(suffix) == PATH_MAX > > What is the relation to NAME_MAX? > > Nothing, just PATH_MAX >> NAME_MAX. Aha. But since we now never use more than NAME_MAX and suffix is stack allocated would it be good to make it NAME_MAX lenght? > patch v2: > > From 4023ad1a81f31ae404c2959bad752d05ad2bb3b9 Mon Sep 17 00:00:00 2001 > From: "Frank Ch. Eigler" > Date: Thu, 10 Oct 2024 16:30:19 -0400 > Subject: [PATCH] PR32218: debuginfod-client: support very long source file > names > > debuginfod clients & servers may sometimes encounter very long source > file names. Previously, the client would synthesize a path name like >$CACHEDIR/$BUILDID/source-$PATHNAME > where $PATHNAME was a funky ##-encoded version of the entire source > path name. See https://sourceware.org/PR32218 for a horror case. > This can get too long to store as a single component of a file system > pathname (e.g. linux/limits.h NAME_MAX), resulting on client-side > errors even after a successful download. > > New code switches encoding of the $PATHNAME part to use less escaping, > and a merciless truncation to the tail part of the filename. (We keep > the tail rather than the head, so that the extension is preserved, > which makes some consumers happier.) To limit collision damage from > truncation, we add also insert a goofy hash (4-byte DJBX33A) of the > name into the path name. The result is a relatively short name: > >$CACHEDIR/$BUILDID/source-$HASH-$NAMETAIL > > This is a transparent change to clients, who are not to make any > assumptions about cache file naming structure. However, one existing > test did make such assumptions, so is fixed with some globness. A new > test is also added, using a pre-baked tarball with a very long srcfile > name. Looks good with or without the suggested s/PATH_MAX/NAME_MAX/ tweak suggested above. Thanks, Mark
☠ Buildbot (Sourceware): elfutils - failed test (failure) (main)
A new failure has been detected on builder elfutils-debian-ppc64 while building elfutils. Full details are available at: https://builder.sourceware.org/buildbot/#/builders/63/builds/398 Build state: failed test (failure) Revision: 088ac7726a935aa048f231bbb793c0fc2134cb51 Worker: debian-ppc64 Build Reason: (unknown) Blamelist: Aaron Merey Steps: - 0: worker_preparation ( success ) - 1: set package name ( success ) - 2: git checkout ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/2/logs/stdio - 3: autoreconf ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/3/logs/stdio - 4: configure ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/4/logs/stdio - config.log: https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/4/logs/config_log - 5: get version ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/5/logs/stdio - property changes: https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/5/logs/property_changes - 6: make ( warnings ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/6/logs/stdio - warnings (4): https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/6/logs/warnings__4_ - 7: make check ( failure ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/7/logs/stdio - test-suite.log: https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/7/logs/test-suite_log - 8: prep ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/8/logs/stdio - 9: build bunsen.cpio.gz ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/9/logs/stdio - 10: fetch bunsen.cpio.gz ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/10/logs/stdio - 11: unpack bunsen.cpio.gz ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/11/logs/stdio - 12: pass .bunsen.source.* ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/12/logs/stdio - 13: upload to bunsen ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/13/logs/stdio - 14: clean up ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/14/logs/stdio - 15: make distclean ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/15/logs/stdio
Re: ☠ Buildbot (Sourceware): elfutils - failed test (failure) (main)
On Wed, Oct 16, 2024 at 2:07 PM wrote: > > A new failure has been detected on builder elfutils-debian-ppc64 while > building elfutils. > > Full details are available at: > https://builder.sourceware.org/buildbot/#/builders/63/builds/398 > > Build state: failed test (failure) > Revision: 088ac7726a935aa048f231bbb793c0fc2134cb51 > Worker: debian-ppc64 > Build Reason: (unknown) > Blamelist: Aaron Merey > [...] > - 7: make check ( failure ) > Logs: > - stdio: > https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/7/logs/stdio > - test-suite.log: > https://builder.sourceware.org/buildbot/#/builders/63/builds/398/steps/7/logs/test-suite_log This failure is unrelated to the commit being tested. The test failed due to an assert failure that occurs randomly. It appears to be the same failure seen here: https://sourceware.org/pipermail/elfutils-devel/2024q3/007415.html Aaron
☺ Buildbot (Sourceware): elfutils - build successful (main)
A restored build has been detected on builder elfutils-debian-ppc64 while building elfutils. Full details are available at: https://builder.sourceware.org/buildbot/#/builders/63/builds/399 Build state: build successful Revision: 1fabae9c499359932fd5db5f0014ed0ab5257b65 Worker: debian-ppc64 Build Reason: (unknown) Blamelist: Aaron Merey Steps: - 0: worker_preparation ( success ) - 1: set package name ( success ) - 2: git checkout ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/399/steps/2/logs/stdio - 3: autoreconf ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/399/steps/3/logs/stdio - 4: configure ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/399/steps/4/logs/stdio - config.log: https://builder.sourceware.org/buildbot/#/builders/63/builds/399/steps/4/logs/config_log - 5: get version ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/399/steps/5/logs/stdio - property changes: https://builder.sourceware.org/buildbot/#/builders/63/builds/399/steps/5/logs/property_changes - 6: make ( warnings ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/399/steps/6/logs/stdio - warnings (4): https://builder.sourceware.org/buildbot/#/builders/63/builds/399/steps/6/logs/warnings__4_ - 7: make check ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/399/steps/7/logs/stdio - test-suite.log: https://builder.sourceware.org/buildbot/#/builders/63/builds/399/steps/7/logs/test-suite_log - 8: prep ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/399/steps/8/logs/stdio - 9: build bunsen.cpio.gz ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/399/steps/9/logs/stdio - 10: fetch bunsen.cpio.gz ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/399/steps/10/logs/stdio - 11: unpack bunsen.cpio.gz ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/399/steps/11/logs/stdio - 12: pass .bunsen.source.* ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/399/steps/12/logs/stdio - 13: upload to bunsen ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/399/steps/13/logs/stdio - 14: clean up ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/399/steps/14/logs/stdio - 15: make distclean ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#/builders/63/builds/399/steps/15/logs/stdio
[PATCH v2 5/6] Remove usage of "unlocked" variant of stdio print functions
These "unlocked" Linux Standard Base variants of standard functions are not available on some systems that are still capable of building Linux and ELFs. The difference is negligible for simple printing to stdout. POSIX also states for the similar putc_unlocked(): These functions can safely be used in a multi-threaded program if and only if they are called while the invoking thread owns the (FILE *) object, as is the case after a successful call to the flockfile() or ftrylockfile() functions. ... These unlocked versions can be safely used only within explicitly locked program regions, using exported locking primitives. and these precautions were never done. Use the standard forms of these print functions. There is inconsistent use of fputc_unlocked() with putc_unlocked(), so consistently use the safer fputc() instead. Signed-off-by: Michael Pratt --- libasm/asm_align.c | 4 +- libcpu/i386_parse.y | 4 +- libebl/eblobjnote.c | 4 +- src/nm.c| 20 +- src/objdump.c | 24 ++-- src/readelf.c | 90 ++--- src/size.c | 8 ++-- src/strings.c | 20 +- tests/showptable.c | 8 ++-- 9 files changed, 91 insertions(+), 91 deletions(-) diff --git a/libasm/asm_align.c b/libasm/asm_align.c index 3a976756..19ec9a13 100644 --- a/libasm/asm_align.c +++ b/libasm/asm_align.c @@ -60,13 +60,13 @@ asm_align (AsmScn_t *asmscn, GElf_Word value) fprintf (asmscn->ctx->out.file, "%02hhx\n", asmscn->pattern->bytes[0]); else { - fputc_unlocked ('"', asmscn->ctx->out.file); + fputc ('"', asmscn->ctx->out.file); for (size_t cnt = 0; cnt < asmscn->pattern->len; ++cnt) fprintf (asmscn->ctx->out.file, "\\x%02hhx", asmscn->pattern->bytes[cnt]); - fputs_unlocked ("\"\n", asmscn->ctx->out.file); + fputs ("\"\n", asmscn->ctx->out.file); } return 0; } diff --git a/libcpu/i386_parse.y b/libcpu/i386_parse.y index 459684c6..5c91e520 100644 --- a/libcpu/i386_parse.y +++ b/libcpu/i386_parse.y @@ -1158,7 +1158,7 @@ instrtable_out (void) EMIT_SUFFIX (w1); EMIT_SUFFIX (W1); - fputc_unlocked ('\n', outfile); + fputc ('\n', outfile); for (int i = 0; i < 3; ++i) { @@ -1333,7 +1333,7 @@ instrtable_out (void) b = b->next; } - fputc_unlocked ('\n', outfile); + fputc ('\n', outfile); } fputs ("};\n", outfile); } diff --git a/libebl/eblobjnote.c b/libebl/eblobjnote.c index ad3f49de..da69e99c 100644 --- a/libebl/eblobjnote.c +++ b/libebl/eblobjnote.c @@ -648,10 +648,10 @@ ebl_object_note (Ebl *ebl, uint32_t namesz, const char *name, uint32_t type, for (size_t cnt = 1; cnt < descsz / 4; ++cnt) { if (cnt > 1) - putchar_unlocked ('.'); + putchar ('.'); printf ("%" PRIu32, buf[cnt]); } - putchar_unlocked ('\n'); + putchar ('\n'); } if (descsz / 4 > FIXED_TAG_BYTES) free (buf); diff --git a/src/nm.c b/src/nm.c index 3675f59b..06f1a072 100644 --- a/src/nm.c +++ b/src/nm.c @@ -439,7 +439,7 @@ handle_ar (int fd, Elf *elf, const char *prefix, const char *fname, Elf_Arhdr *arhdr = NULL; size_t arhdr_off = 0; /* Note: 0 is no valid offset. */ - fputs_unlocked (_("\nArchive index:\n"), stdout); + fputs (_("\nArchive index:\n"), stdout); while (arsym->as_off != 0) { @@ -825,8 +825,8 @@ show_symbols_sysv (Ebl *ebl, GElf_Word strndx, const char *fullname, /* If we have to precede the line with the file name. */ if (print_file_name) { - fputs_unlocked (fullname, stdout); - putchar_unlocked (':'); + fputs (fullname, stdout); + putchar (':'); } /* Convert the address. */ @@ -972,8 +972,8 @@ show_symbols_bsd (Elf *elf, const GElf_Ehdr *ehdr, GElf_Word strndx, /* If we have to precede the line with the file name. */ if (print_file_name) { - fputs_unlocked (fullname, stdout); - putchar_unlocked (':'); + fputs (fullname, stdout); + putchar (':'); } bool is_tls = GELF_ST_TYPE (syms[cnt].sym.st_info) == STT_TLS; @@ -1046,8 +1046,8 @@ show_symbols_bsd (Elf *elf, const GElf_Ehdr *ehdr, GElf_Word strndx, } if (color_mode) - fputs_unlocked (color_off, stdout); - putchar_unlocked ('\n'); + fputs (color_off, stdout); + putchar ('\n'); } #ifdef USE_DEMANGLE @@ -1104,9 +1104,9 @@ show_symbols_posix (Elf *elf, const GElf_Ehdr *ehdr, GElf_Word strndx, /* If we have to precede the line with the file name. */ if (print_file_name) { - fputs_unlocked (fullname, stdout); - putcha
[PATCH 1/4] eu-stacktrace: add eu-stacktrace tool
Hi Serhei, On Tue, Oct 15, 2024 at 11:27 AM Serhei Makarov wrote > eu-stacktrace: add eu-stacktrace tool > > eu-stacktrace is a utility to process a stream of raw stack > samples (such as those obtained from the Linux kernel's > PERF_SAMPLE_STACK facility) into a stream of stack traces (such as > those obtained from PERF_SAMPLE_CALLCHAIN), freeing other profiling > utilities from having to implement their own backtracing logic. Please add an eu-stacktrace entry to NEWS. > > eu-stacktrace accepts data from a profiling tool via a pipe or > fifo. The initial version of the tool works on x86 architectures and > accepts data from Sysprof [1]. For future work, it will make sense > to expand support to other profilers, in particular perf tool. > > Further patches in this series provide configury, docs, and improved > diagnostics for tracking the method used to unwind each frame in the > stack trace. > > [1]: The following patched version of Sysprof (upstream submission ETA > ~very_soon) can produce data with stack samples: > > https://git.sr.ht/~serhei/sysprof-experiments/log/serhei/samples-via-fifo > > Invoking the patched sysprof with eu-stacktrace: > > $ sudo sysprof-cli --use-stacktrace > $ sudo sysprof-cli --use-stacktrace --stacktrace-path=/path/to/eu-stacktrace I tried to run this but I kept getting: Failed to complete recording: GDBus.Error[...]: Action org.gnome.sysprof3.profile is not registered On the eu-stacktrace side I got: TOTAL -- received 0 samples, lost 0 samples, loaded 0 processes > > Invoking the patched sysprof and eu-stacktrace manually through a fifo: > > $ mkfifo /tmp/test.fifo > $ sudo eu-stacktrace --input /tmp/test.fifo --output test.syscap & > $ sysprof-cli --sample-method=stack --use-fifo=/tmp/test.fifo test.syscap I ran eu-stacktrace under `valgrind --leak-check=full`. Memory allocated at stacktrace.c:1542 and 1543 is leaked. > > Note that sysprof polkit actions must be installed systemwide > (e.g. installing the system sysprof package will provide these). > > * src/stacktrace.c: Add new tool. > > [...] > > +/* > + * Includes: sysprof data structures * > + */ > + > +#if HAVE_SYSPROF_6_HEADERS > +#include > +#define HAVE_SYSPROF_HEADERS 1 > +#elif HAVE_SYSPROF_4_HEADERS > +#include > +#define HAVE_SYSPROF_HEADERS 1 > +#else > +#define HAVE_SYSPROF_HEADERS 0 > +#endif > + > +#if HAVE_SYSPROF_HEADERS > + > +/* XXX: To be added to new versions of sysprof. */ > +#ifndef SYSPROF_CAPTURE_FRAME_STACK_USER > + > +#undef SYSPROF_CAPTURE_FRAME_LAST > +#define SYSPROF_CAPTURE_FRAME_STACK_USER 18 > +#define SYSPROF_CAPTURE_FRAME_LAST 19 > + > +SYSPROF_ALIGNED_BEGIN(1) > +typedef struct > +{ > + SysprofCaptureFrame frame; > + uint64_t size; > + int32_t tid; > + uint32_t padding; > + uint8_t data[0]; > +} SysprofCaptureStackUser > +SYSPROF_ALIGNED_END(1); > + > +/* Does not appear standalone; instead, appended to the end of a > SysprofCaptureStackUser frame. */ > +SYSPROF_ALIGNED_BEGIN(1) > +typedef struct > +{ > + uint32_t n_regs; > + uint32_t abi; > + uint64_t regs[0]; > +} SysprofCaptureUserRegs > +SYSPROF_ALIGNED_END(1); I got a "conflicting types" error when trying to compile this due to SysprofCaptureStackUser and SysprofCaptureUserRegs being defined both here and in sysprof-capture-types.h. Should these structs instead be defined when HAVE_SYSPROF_HEADERS is 0? I was able to build eu-stacktrace by making that change. I'm not familiar with the sysprof APIs so I'm not able to do a thorough review of the rest of this particular patch before the next elfutils release on Oct 18. Normally I'd say lets hold off on merging this so close to the release. However you do indicate in the eu-stacktrace usage text that this is a WIP feature: > + const struct argp argp = > +{ > + .options = options, > + .parser = parse_opt, > + .doc = N_("Process a stream of stack samples into stack traces.\n\ > +\n\ > +Utility is a work-in-progress, see README.eu-stacktrace in the source > branch.") > +}; Also the patches that modify other parts of elfutils aren't risky IMO. I'm comfortable with this being merged for the release as long as any build errors are sorted out and we emphasize in the usage text and NEWS that eu-stacktrace is an experimental feature with a possibly unstable ABI. Aaron
[PATCH 2/4] eu-stacktrace: configury for initial version (x86/sysprof only)
Hi Serhei, On Tue, Oct 15, 2024 at 11:28 AM Serhei Makarov wrote: > > Due to the x86-specific code in the initial version the configury has > significant restrictions. If --enable-stacktrace is not explicitly > provided, then eu-stacktrace will be disabled by default when > x86 architecture or sysprof headers are absent. > > The way we test for x86 is a bit unusual. What we actually care about > is that the register file provided by perf_events on the system is an > x86 register file; this is done by checking that is > Linux kernel arch/x86/include/uapi/asm/perf_regs.h. > > Once eu-stacktrace is properly portable across architectures, > these grody checks can be simplified. > > * configure.ac: Add configure checks and conditionals for stacktrace tool. > * src/Makefile.am: Add stacktrace tool conditional on ENABLE_STACKTRACE. > > Signed-off-by: Serhei Makarov > --- > configure.ac| 66 - > src/Makefile.am | 9 ++- > 2 files changed, 73 insertions(+), 2 deletions(-) Using less wide indentation throughout would help make the configure.ac changes more readable. Otherwise this patch LGTM. Aaron
[PATCH 3/4] eu-stacktrace: add unwind origin diagnostics
Hi Serhei, On Tue, Oct 15, 2024 at 11:28 AM Serhei Makarov wrote: > > Track the method used to unwind each Dwfl_Frame using an enum field > unwound_source; provide access to the field. Then use that in > eu-stacktrace to display the unwind methods used for each process. > > This is an important diagnostic to verify how many processes are > adequately covered by the eh_frame unwinder. > > * libdw/libdw.map (ELFUTILS_0.192): Add dwfl_frame_unwound_source, > dwfl_unwound_source_str. > * libdwfl/libdwfl.h (Dwfl_Unwound_Source): New enum. > (dwfl_frame_unwound_source) > (dwfl_unwound_source_str): New functions. > * libdwfl/dwfl_frame.c (dwfl_frame_unwound_source) > (dwfl_unwound_source_str): New functions. > * libdwfl/dwflP.h: Add INTDECL for dwfl_frame_unwound_source, Should be libdwflP.h. > dwfl_unwound_source_str. > (struct Dwfl_Frame): Add unwound_source field. > * libdwfl/frame_unwind.c (__libdwfl_frame_unwind): Set > state->unwound_source depending on the unwind method used. > * src/stacktrace.c (struct sysprof_unwind_info): Add last_pid > field to provide access to the current sample's dwfltab entry. > (sysprof_unwind_frame_cb): Add unwound_source to the data displayed > with the show_frames option. > (sysprof_unwind_cb): Set last_pid when processing a sample. > (main): Add unwind_source to the data displayed in the final summary > table. > > Signed-off-by: Serhei Makarov > --- > libdw/libdw.map| 2 ++ > libdwfl/dwfl_frame.c | 31 ++- > libdwfl/frame_unwind.c | 14 +++--- > libdwfl/libdwfl.h | 20 +++- > libdwfl/libdwflP.h | 5 - > src/stacktrace.c | 31 ++- > 6 files changed, 92 insertions(+), 11 deletions(-) > > diff --git a/libdw/libdw.map b/libdw/libdw.map > index 552588a9..bc53385f 100644 > --- a/libdw/libdw.map > +++ b/libdw/libdw.map > @@ -382,4 +382,6 @@ ELFUTILS_0.191 { > ELFUTILS_0.192 { >global: > dwfl_set_sysroot; > +dwfl_frame_unwound_source; > +dwfl_unwound_source_str; > } ELFUTILS_0.191; > diff --git a/libdwfl/dwfl_frame.c b/libdwfl/dwfl_frame.c > index 8af8843f..2e6c6de8 100644 > --- a/libdwfl/dwfl_frame.c > +++ b/libdwfl/dwfl_frame.c > @@ -1,5 +1,5 @@ > /* Get Dwarf Frame state for target PID or core file. > - Copyright (C) 2013, 2014 Red Hat, Inc. > + Copyright (C) 2013, 2014, 2024 Red Hat, Inc. > This file is part of elfutils. > > This file is free software; you can redistribute it and/or modify > @@ -98,6 +98,7 @@ state_alloc (Dwfl_Thread *thread) >state->signal_frame = false; >state->initial_frame = true; >state->pc_state = DWFL_FRAME_STATE_ERROR; > + state->unwound_source = DWFL_UNWOUND_INITIAL_FRAME; >memset (state->regs_set, 0, sizeof (state->regs_set)); >thread->unwound = state; >state->unwound = NULL; > @@ -248,6 +249,34 @@ dwfl_frame_thread (Dwfl_Frame *state) > } > INTDEF(dwfl_frame_thread) > > +Dwfl_Unwound_Source > +dwfl_frame_unwound_source (Dwfl_Frame *state) > +{ > + return state->unwound_source; > +} > +INTDEF(dwfl_frame_unwound_source) > + > +const char * > +dwfl_unwound_source_str (Dwfl_Unwound_Source unwound_source) > +{ > + switch (unwound_source) > +{ > +case DWFL_UNWOUND_NONE: > + return "none"; > +case DWFL_UNWOUND_INITIAL_FRAME: > + return "initial"; > +case DWFL_UNWOUND_EH_CFI: > + return "eh_frame"; > +case DWFL_UNWOUND_DWARF_CFI: > + return "dwarf"; > +case DWFL_UNWOUND_EBL: > + return "ebl"; > +default: > + return "unknown"; > +} > +} > +INTDEF(dwfl_unwound_source_str) > + > int > dwfl_getthreads (Dwfl *dwfl, int (*callback) (Dwfl_Thread *thread, void > *arg), >void *arg) > diff --git a/libdwfl/frame_unwind.c b/libdwfl/frame_unwind.c > index ab444d25..de65e09c 100644 > --- a/libdwfl/frame_unwind.c > +++ b/libdwfl/frame_unwind.c > @@ -1,5 +1,5 @@ > /* Get previous frame state for an existing frame state. > - Copyright (C) 2013, 2014, 2016 Red Hat, Inc. > + Copyright (C) 2013, 2014, 2016, 2024 Red Hat, Inc. > This file is part of elfutils. > > This file is free software; you can redistribute it and/or modify > @@ -515,6 +515,7 @@ new_unwound (Dwfl_Frame *state) >unwound->signal_frame = false; >unwound->initial_frame = false; >unwound->pc_state = DWFL_FRAME_STATE_ERROR; > + unwound->unwound_source = DWFL_UNWOUND_NONE; >memset (unwound->regs_set, 0, sizeof (unwound->regs_set)); >return unwound; > } > @@ -742,14 +743,20 @@ __libdwfl_frame_unwind (Dwfl_Frame *state) > { > handle_cfi (state, pc - bias, cfi_eh, bias); > if (state->unwound) > - return; > + { > + state->unwound->unwound_source = DWFL_UNWOUND_EH_CFI; > + return; > + } > } >Dwarf_CFI *cfi_dwarf = INTUSE(dwfl_module_dwarf_cfi) (mod, &bias); >if (cfi_dwarf) > { > handle_cfi (state,
[PATCH 4/4] eu-stacktrace: add unwind origin diagnostics to eu-stack
Hi Serhei, On Tue, Oct 15, 2024 at 11:29 AM Serhei Makarov wrote: > > Since we obtain diagnostics about unwind method, another logical place > to show them is in eu-stack. > > This requires an additional pseudo-unwind type DWFL_UNWOUND_INLINE to > clarify what happens with entries for inlined functions (which > eu-stacktrace does not display) based on the same Dwfl_Frame. > > * libdwfl/libdwfl.h (Dwfl_Unwound_Source): Add DWFL_UNWOUND_INLINE. > * libdwfl/dwfl_frame.c (dwfl_unwound_source_str): > Handle DWFL_UNWOUND_INLINE. > * src/stack.c (show_unwound_source): New global variable. > (struct frame): Add unwound_source field. > (frame_callback): Copy over unwound_source from Dwfl_Frame. > (print_frame): Take unwound_source argument and print it. > (print_inline_frames): Take unwound_source argument and pass it on, > except for subsequent frames where DWFL_UNWOUND_INLINE is assumed. > (print_frames): Take unwound_source field from struct frame and pass > it on. > (parse_opt): Add --cfi-type,-c option to set show_unwound_source. > (main): Ditto. > * tests/run-stack-i-test.sh: Add testcase for --cfi-type. > > Signed-off-by: Serhei Makarov This patch LGTM. Aaron