[PATCH] readelf: Report error when decl_file or call_file attribute is invalid.
Report an error for why the DW_AT_decl_file or DW_AT_call_file cannot
be resolved to a file name. This is likely invalid DWARF, a missing
DW_AT_stmt_list attribute on the CU or a missing .debug_line section.
Signed-off-by: Mark Wielaard
---
src/ChangeLog | 5 +
src/readelf.c | 18 +++---
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index c879712..f2f99ed 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2018-03-20 Mark Wielaard
+
+ * readelf.c (attr_callback): Report error when DW_AT_decl_file or
+ DW_AT_call_file cannot be resolved.
+
2018-03-06 Mark Wielaard
* readelf.c (print_ops): Handle DW_OP_addrx, DW_OP_constx,
diff --git a/src/readelf.c b/src/readelf.c
index fcf5e2c..b1d63ef 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -6247,11 +6247,23 @@ attr_callback (Dwarf_Attribute *attrp, void *arg)
if (dwarf_getsrcfiles (&cudie, &files, &nfiles) == 0)
{
valuestr = dwarf_filesrc (files, num, NULL, NULL);
- char *filename = strrchr (valuestr, '/');
- if (filename != NULL)
- valuestr = filename + 1;
+ if (valuestr != NULL)
+ {
+ char *filename = strrchr (valuestr, '/');
+ if (filename != NULL)
+ valuestr = filename + 1;
+ }
+ else
+ error (0, 0, gettext ("invalid file (%" PRId64 "): %s"),
+num, dwarf_errmsg (-1));
}
+ else
+ error (0, 0, gettext ("no srcfiles for CU [%zx]"),
+dwarf_dieoffset (&cudie));
}
+ else
+error (0, 0, gettext ("couldn't get DWARF CU: %s"),
+ dwarf_errmsg (-1));
}
break;
default:
--
1.8.3.1
[Bug general/22976] global-buffer-overflow in ebl_dynamic_tag_name (libebl/ebldynamictagname.c)
https://sourceware.org/bugzilla/show_bug.cgi?id=22976 Mark Wielaard changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #2 from Mark Wielaard --- Pushed to master -- You are receiving this mail because: You are on the CC list for the bug.
Re: How to associate Elf with Dwfl_Module returned by dwfl_report_module
Hi Milian, On Sat, Mar 17, 2018 at 02:14:48PM +0100, Milian Wolff wrote: > a recurring issue in the libdwfl integration of perf and perfparser are > supposedly overlapping modules. The perf data file contains the exact > mappings > for all files corresponding to the actual mmap events that occurred during > runtime, e.g.: > > ``` > $ perf script --show-mmap-events | grep MMAP | grep stdc > heaptrack_print 13962 87163.483450: PERF_RECORD_MMAP2 13962/13962: > [0x7fd0aea84000(0x387000) @ 0 08:03 413039 3864781083]: r-xp /usr/lib/libstdc+ > +.so.6.0.24 > heaptrack_print 13962 87163.483454: PERF_RECORD_MMAP2 13962/13962: > [0x7fd0aebfc000(0x1ff000) @ 0x178000 08:03 413039 3864781083]: ---p /usr/lib/ > libstdc++.so.6.0.24 > heaptrack_print 13962 87163.483458: PERF_RECORD_MMAP2 13962/13962: > [0x7fd0aedfb000(0xd000) @ 0x177000 08:03 413039 3864781083]: rw-p /usr/lib/ > libstdc++.so.6.0.24 > heaptrack_print 13962 87163.484860: PERF_RECORD_MMAP2 13962/13962: > [0x7fd0aedfb000(0xc000) @ 0x177000 08:03 413039 3864781083]: r--p /usr/lib/ > libstdc++.so.6.0.24 > ``` > So far, both perf and perfparser are using dwfl_report_elf to report the > file. > But that API is deducing the mapping addresses internally, which may or may > not be what we saw at runtime. I suspect that this is the reason for some > issues we are seeing, such as supposedly overlapping modules. How exactly are you calling dwfl_report_elf? Specifically are you using false for the add_p_vaddr argument? And could you provide some example where the reported address is wrong/different from the start address of the Dwfl_Module? > Looking at the Dwfl API, I cannot figure out how to feed the mapping > directly. > There's dwfl_report_module, but how would I associate an Elf* and int fd with > it, as done by dwfl_report_elf? When using dwfl_report_module the find_elf callback will be called that you registered with dwfl_begin. That callback is called "lazily" the first time dwfl_module_getelf is called. The callback can then set the Elf*. But that does mean you have to keep track yourself (or immediately call dwfl_module_getelf). I would like to understand better what is really going wrong with dwfl_report_elf before diving into using dwfl_module_report. Cheers, Mark
Re: [PATCH] libdwfl: Use process_vm_readv when available.
On Sun, Mar 18, 2018 at 01:43:23AM +0100, Mark Wielaard wrote:
> If possible use process_vm_readv to read 4K blocks instead of fetching
> each word individually with ptrace. For unwinding this often means we
> only have to do one process_vm_readv of the stack instead of dozens of
> ptrace calls. There is one 4K cache per process, cleared whenever a
> thread is detached.
It seems to work well, but the GCC undefined sanitizer
(configure --enable-sanitize-undefined) found an issue in the
run-backtrace-native-biarch.sh testcase (from x86_64 to i686)
when reading unaligned data. To fix that don't assign to the
Dwarf_Word directly when unaligned, but use memcpy (which gcc
seems to inline).
diff --git a/libdwfl/linux-pid-attach.c b/libdwfl/linux-pid-attach.c
index ea65618f..6295e391 100644
--- a/libdwfl/linux-pid-attach.c
+++ b/libdwfl/linux-pid-attach.c
@@ -144,7 +144,10 @@ read_cached_memory (struct __libdwfl_pid_arg *pid_arg,
if (addr >= mem_cache->addr && addr - mem_cache->addr < mem_cache->len)
{
d = &mem_cache->buf[addr - mem_cache->addr];
- *result = *(unsigned long *) d;
+ if uintptr_t) d) & (sizeof (unsigned long) - 1)) == 0)
+ *result = *(unsigned long *) d;
+ else
+ memcpy (result, d, sizeof (unsigned long));
return true;
}
@@ -165,7 +168,10 @@ read_cached_memory (struct __libdwfl_pid_arg *pid_arg,
mem_cache->len = res;
d = &mem_cache->buf[addr - mem_cache->addr];
- *result = *((unsigned long *) d);
+ if uintptr_t) d) & (sizeof (unsigned long) - 1)) == 0)
+*result = *(unsigned long *) d;
+ else
+memcpy (result, d, sizeof (unsigned long));
return true;
}
#endif /* HAVE_PROCESS_VM_READV */
Re: [PATCH] Specify a custom git merge driver for ChangeLog files
I am not against this, but it could use a bit more documentation in at least the commit message, so people know what they should setup in their gitconfig. Also isn't it better to have something like the following in your local ~/.gitconfig: [core] attributesfile = ~/.gitattributes And then have the ChangeLog merge=merge-changelog line in ~/.gitattributes? Cheers, Mark
Re: [PATCH] Specify a custom git merge driver for ChangeLog files
On Wed, Mar 21, 2018 at 12:03:53AM +0100, Mark Wielaard wrote: > I am not against this, but it could use a bit more documentation > in at least the commit message, so people know what they should > setup in their gitconfig. The advantage of adding .gitattributes file to repository is that no gitconfig setup is needed at all, just git-merge-changelog has to be made available in $PATH. -- ldv signature.asc Description: PGP signature
Re: [PATCH] libdwfl: Use process_vm_readv when available.
On Sun, Mar 18, 2018 at 01:43:23AM +0100, Mark Wielaard wrote:
[...]
> @@ -115,12 +116,80 @@ __libdwfl_ptrace_attach (pid_t tid, bool
> *tid_was_stoppedp)
>return true;
> }
>
> +#ifdef HAVE_PROCESS_VM_READV
> +static bool
> +read_cached_memory (struct __libdwfl_pid_arg *pid_arg,
> + Dwarf_Addr addr, Dwarf_Word *result)
> +{
> + /* Let the ptrace fallback deal with the corner case of the address
> + possibly crossing a page boundery. */
> + if ((addr & ((Dwarf_Addr)__LIBDWFL_REMOTE_MEM_CACHE_SIZE - 1))
> + > (Dwarf_Addr)__LIBDWFL_REMOTE_MEM_CACHE_SIZE - sizeof (unsigned long))
It looks odd that the variable that is going to be assigned has type
Dwarf_Word, while the size being checked has type unsigned long.
Shouldn't it be sizeof(*result) instead?
> +return false;
> +
> + struct __libdwfl_remote_mem_cache *mem_cache = pid_arg->mem_cache;
> + if (mem_cache == NULL)
> +{
> + size_t mem_cache_size = sizeof (struct __libdwfl_remote_mem_cache);
> + mem_cache = (struct __libdwfl_remote_mem_cache *) malloc
> (mem_cache_size);
> + if (mem_cache == NULL)
> + return false;
> +
> + mem_cache->addr = 0;
> + mem_cache->len = 0;
> + pid_arg->mem_cache = mem_cache;
> +}
> +
> + unsigned char *d;
> + if (addr >= mem_cache->addr && addr - mem_cache->addr < mem_cache->len)
> +{
> + d = &mem_cache->buf[addr - mem_cache->addr];
> + *result = *(unsigned long *) d;
Likewise, shouldn't it be memcpy(result, d, sizeof(*result)) instead?
--
ldv
signature.asc
Description: PGP signature
Re: [PATCH] Specify a custom git merge driver for ChangeLog files
On Wed, Mar 21, 2018 at 02:20:51AM +0300, Dmitry V. Levin wrote: > On Wed, Mar 21, 2018 at 12:03:53AM +0100, Mark Wielaard wrote: > > I am not against this, but it could use a bit more documentation > > in at least the commit message, so people know what they should > > setup in their gitconfig. > > The advantage of adding .gitattributes file to repository is that > no gitconfig setup is needed at all, just git-merge-changelog > has to be made available in $PATH. My bad, gitconfig setup cannot be avoided this way. Being an early adopter of git-merge-changelog, I must have forgotten these details. -- ldv signature.asc Description: PGP signature
Re: [PATCH] Specify a custom git merge driver for ChangeLog files
On Wed, Mar 21, 2018 at 02:20:51AM +0300, Dmitry V. Levin wrote: > On Wed, Mar 21, 2018 at 12:03:53AM +0100, Mark Wielaard wrote: > > I am not against this, but it could use a bit more documentation > > in at least the commit message, so people know what they should > > setup in their gitconfig. > > The advantage of adding .gitattributes file to repository is that > no gitconfig setup is needed at all, just git-merge-changelog > has to be made available in $PATH. O, interesting. That doesn't seem to be documented anywhere. So, when git sees merge-changelog as custom merge command and there is no custom merge definition, then it will try to execute git merge-changelog, which will invoke git-merge-changelog from the PATH? Could you add this explanation to the commit message? And a brief comment about git-merge-changelog coming from gnulib? Thanks, Mark
Re: [PATCH] libdwfl: Use process_vm_readv when available.
On Wed, Mar 21, 2018 at 02:28:48AM +0300, Dmitry V. Levin wrote: > On Sun, Mar 18, 2018 at 01:43:23AM +0100, Mark Wielaard wrote: > > + /* Let the ptrace fallback deal with the corner case of the address > > + possibly crossing a page boundery. */ > > + if ((addr & ((Dwarf_Addr)__LIBDWFL_REMOTE_MEM_CACHE_SIZE - 1)) > > + > (Dwarf_Addr)__LIBDWFL_REMOTE_MEM_CACHE_SIZE - sizeof (unsigned > > long)) > > It looks odd that the variable that is going to be assigned has type > Dwarf_Word, while the size being checked has type unsigned long. > Shouldn't it be sizeof(*result) instead? > > > + d = &mem_cache->buf[addr - mem_cache->addr]; > > + *result = *(unsigned long *) d; > > Likewise, shouldn't it be memcpy(result, d, sizeof(*result)) instead? That is indeed not immediately clear. I'll add some documentation. Although the functions do use Dwarf_Word (which is always 64bits) they actually return the result of an unsigned long/address. This is true for both the pid based and core based memory read functions. I am not completely sure if this was originally deliberate, or if this was the accidental result of the ptrace interface returning a long (target word) for PTRACE_PEEKDATA. Thanks for reviewing. Cheers, Mark
