Your changes make sense to me, as do your concerns about accessing memory past the end of the synthetic frame header.
The only change I can suggest is that you should take the opportunity to stop the explicit bytes offsets in the code and write something like
struct dwarf_eh_frame_hdr *synth_eh_frame_hdr; unsigned char synth_eh_frame_space[sizeof(*synth_eh_frame_hdr)+sizeof(Elf_W(Addr))]; synth_eh_frame_hdr = (struct_dwarf_eh_frame_hdr *) synth_eh_frame_space;
…
synth_eh_frame_hdr->version = DW_EH_VERSION; synth_eh_frame_hdr->eh_frame_ptr_enc = … etc.
because I don’t understand why the original code used the struct member names in comments and not in the actual code.
Doug
Thanks, that does appear to fix things in my case. I thought I would do alittle more investigating in case it is useful in justifying your proposedpatch.Whenever we use synth_eh_frame_hdr, then hdr is pointed at it, sohdr->table_enc can only be DW_EH_PR_omit, and the FDE length word pointerwhich is ultimately dereferenced by dwarf_extract_proc_info_from_fde() isthe one (returned from dwarf_find_eh_frame_section()) whichdwarf_callback() sticks into the word after synth_eh_frame_hdr. In my case,it seems that whenever eh_frame_start is read from the word aftersynth_eh_frame_hdr like this, the FDE length word read bydwarf_extract_proc_info_from_fde() is always 0, irrespective of objectfile. As a result, it always returns -UNW_ENOINFO, so linear_search() alsoalways fails. Does that make sense?In passing, I notice that at lines 547 and 584, we're referencing anaddress beyond the bounds of synth_eh_frame_hdr, which could cause stackcorruption, depending on architecture and compiler. Wouldn't something likethis be better?--- a/src/dwarf/Gfind_proc_info-lsb.c+++ b/src/dwarf/Gfind_proc_info-lsb.c@@ -482,7 +482,7 @@ dwarf_callback (struct dl_phdr_info *info, size_t size, void *ptr) unw_accessors_t *a; long n; int found = 0;- struct dwarf_eh_frame_hdr synth_eh_frame_hdr;+ unsigned char synth_eh_frame_hdr[sizeof (struct dwarf_eh_frame_hdr) + sizeof (Elf_W (Addr))];#ifdef CONFIG_DEBUG_FRAME unw_word_t start, end;#endif /* CONFIG_DEBUG_FRAME*/@@ -537,15 +537,14 @@ dwarf_callback (struct dl_phdr_info *info, size_t size, void *ptr) eh_frame = dwarf_find_eh_frame_section (info); if (eh_frame) {- unsigned char *p = (unsigned char *) &synth_eh_frame_hdr; Debug (1, "using synthetic .eh_frame_hdr section for %s\n", info->dlpi_name);- /* synth_eh_frame_hdr.version */ p[0] = DW_EH_VERSION;- /* synth_eh_frame_hdr.eh_frame_ptr_enc */ p[1] = DW_EH_PE_absptr | ((sizeof(Elf_W (Addr)) == 4) ? DW_EH_PE_udata4 : DW_EH_PE_udata8);- /* synth_eh_frame_hdr.fde_count_enc */ p[2] = DW_EH_PE_omit;- /* synth_eh_frame_hdr.table_enc */ p[3] = DW_EH_PE_omit;- *(Elf_W (Addr) *)(&p[4]) = eh_frame;- hdr = &synth_eh_frame_hdr;+ /* version */ synth_eh_frame_hdr[0] = DW_EH_VERSION;+ /* eh_frame_ptr_enc */ synth_eh_frame_hdr[1] = DW_EH_PE_absptr | ((sizeof(Elf_W (Addr)) == 4) ? DW_EH_PE_udata4 : DW_EH_PE_udata8);+ /* fde_count_enc */ synth_eh_frame_hdr[2] = DW_EH_PE_omit;+ /* table_enc */ synth_eh_frame_hdr[3] = DW_EH_PE_omit;+ hdr = (struct dwarf_eh_frame_hdr *)synth_eh_frame_hdr;+ *(Elf_W (Addr) *)(hdr + 1) = eh_frame; } }I'm not 100% sure whether that last word shouldn't be accessed via theunw_word_t type though, as that's how it's accessed when the value is readback again.BenOn Fri, 30 Jun 2017 06:08:18 +0100, Doug Moore <address@hidden> wrote:I didn't invent the single_fde field, but I wonder is it was intended to be set in a case where the linear search failed and no unwind info was obtained. So, I wonder if this patch
--- a/src/dwarf/Gfind_proc_info-lsb.c +++ b/src/dwarf/Gfind_proc_info-lsb.c @@ -618,12 +618,13 @@ dwarf_callback (struct dl_phdr_info *info, size_t size, void *ptr)
/* XXX we know how to build a local binary search table for .debug_frame, so we could do that here too. */ - cb_data->single_fde = 1; found = linear_search (unw_local_addr_space, ip, eh_frame_start, eh_frame_end, fde_count, pi, need_unwind_info, NULL); if (found != 1) found = 0; + else + cb_data->single_fde = 1; } else {
addresses the problem, while being consistent with the greater purpose of single_fde.
Doug Moore
Quoting address@hidden:
Send Libunwind-devel mailing list submissions to address@hidden
To subscribe or unsubscribe via the World Wide Web, visit https://lists.nongnu.org/mailman/listinfo/libunwind-devel or, via email, send a message with subject or body 'help' to address@hidden
You can reach the person managing the list at address@hidden
When replying, please edit your Subject line so it is more specific than "Re: Contents of Libunwind-devel digest..."
Today's Topics:
1. Help please with debugging DWARF unwinding on ARM (Ben Avison)
----------------------------------------------------------------------
Message: 1 Date: Thu, 29 Jun 2017 01:56:06 +0100 From: "Ben Avison" <address@hidden> To: address@hidden Subject: [Libunwind-devel] Help please with debugging DWARF unwinding on ARM Message-ID: <address@hidden> Content-Type: text/plain; charset=iso-8859-15; format=flowed; delsp=yes
Hi all,
First a quick typo to report: at src/dwarf/Gparser.c:281, the memcpy() statement is lacking a trailing semicolon - this doesn't become apparent until you enable debugging.
I have previously used libunwind successfully as the backend for gperftools, however I just get segfaults with the latest version.
Some background: I'm running on ARM, and I configure libunwind with --enable-debug-frame then run the executable with UNW_ARM_UNWIND_METHOD=1 to force DWARF unwinding.
The revision in git which introduced the bug is 25413c7 (dwarf: Improve support for binaries missing the GNU_EH_FRAME segment).
Ultimately, the segfault is triggered at the end of fetch_proc_info() where c->pi is uninitialised when it tried to dereference it. However, I'm having a hard time understanding the code well enough to know how to ensure it is initialised correctly.
Gperftools always starts skipping through the call stack from an initial cursor position set up by unw_init_local(). The very first call to unw_step() is the one that fails.
If I back out the changes from that git revision, the pi struct is initialised towards the end of dwarf_extract_proc_info_from_fde(), which is called from dwarf_search_unwind_table(), in the second such call from dwarf_find_proc_info().
But using the head of git, dwarf_find_proc_info() exits early because cb_data.single_fde is set, so it never calls dwarf_search_unwind_table(). single_fde is only set in one place, in dwarf_callback() - in a bit of code that was never previously called when no .eh_frame_hdr section is found (which appears to be true in my case). However, if single_fde is set then control flow has unavoidably also passed through linear_search(), which also calls dwarf_extract_proc_info_from_fde().
It appears to me that the problem is that sometimes, the first time dwarf_extract_proc_info_from_fde() is called for a given pi struct, it is called with need_unwind_info=0, but then dwarf_find_proc_info() is later called with need_unwind_info=1 but due to single_fde being set, we never get the pi struct initialised by anyone.
It's possible I've got the wrong end of the stick here, but I'm also far from sure what the best way to fix this is. Any advice gratefully received! Please bear in mind I have rather limited working knowledge of the DWARF spec.
Thanks in advance, Ben
------------------------------
Subject: Digest Footer
_______________________________________________ Libunwind-devel mailing list address@hidden https://lists.nongnu.org/mailman/listinfo/libunwind-devel
------------------------------
End of Libunwind-devel Digest, Vol 124, Issue 11 ************************************************
_______________________________________________ Libunwind-devel mailing list address@hidden https://lists.nongnu.org/mailman/listinfo/libunwind-devel
!DSPAM:10223,595678b5326342783720507!
|