Re: [PATCH] Make sure to always build with build-ids.
Hi, On Fri, Dec 06, 2019 at 05:22:41PM +0100, Mark Wielaard wrote: > We really need build-ids for various things. If the system compiler > doesn't generate build-ids warn and generate them anyway for both the > binaries and the tests. I pushed this to master with one change: > +# We really want build-ids. Warn and force generating them if gcc was > +# configure without --enable-linker-build-id > +AC_CACHE_CHECK([whether the compiler generates build-ids], ac_cv_buildid, > [dnl > +AC_LINK_IFELSE([AC_LANG_PROGRAM()],[ac_cv_buildid=yes; readelf -n > conftest$EXEEXT | grep -q NT_GNU_BUILD_ID || > ac_cv_buildid=no],AC_MSG_FAILURE([unexpected compile failure]))]) > +if test "$ac_cv_buildid" = "no"; then > + AC_MSG_WARN([compiler doesn't generate build-id by default]) > + CFLAGS="$CFLAGS -Wl,--build-id" > +fi This should have been LDFLAGS="$LDFLAGS -Wl,--build-id" With this the testsuite now also passes when everything was build with a gcc that wasn't configured with --enable-linker-build-id (sadly the default is off). Cheers, Mark
Re: RFCv2: debuginfod debian archive support
Hi,
On Fri, 2019-12-06 at 16:17 -0500, Frank Ch. Eigler wrote:
> Presenting testing for the debuginfod .deb/.ddeb support patch, after
> finding a good debian-packaging tutorial, and generating a workable
> basic set of test deb's on a Ubuntu box.
According to Matthias on irc there should be no difference between
Ubuntu and Debian doing this, so this should cover both systems.
> debuginfod: deb support, tests
>
> Using a synthetic .deb/.ddeb from a Ubuntu 18 machine,
> extend the debuginfod testsuite with some .deb processing,
> if the dpkg-deb binary is installed.
Looks good. Thanks.
All buildbot workers should already have the dpkg-deb binary installed.
Two nitpicks below, not very important.
> diff --git a/config/ChangeLog b/config/ChangeLog
> index d71fb39..9b2a408 100644
> --- a/config/ChangeLog
> +++ b/config/ChangeLog
> @@ -1,3 +1,8 @@
> +2019-12-06 Frank Ch. Eigler
> +
> + * elfutils.spec.in (debuginfod): Add BuildRequire dpkg
> + for deb testing. (Available on Fedora & EPEL, not base RHEL.)
> +
> 2019-11-28 Mark Wielaard
>
> * elfutils.spec.in (debuginfod): Add an explicit Requires
> diff --git a/config/elfutils.spec.in b/config/elfutils.spec.in
> index 1cdca21..faeb7f8 100644
> --- a/config/elfutils.spec.in
> +++ b/config/elfutils.spec.in
> @@ -35,6 +35,9 @@ BuildRequires: pkgconfig(libarchive) >= 3.1.2
> BuildRequires: bzip2
> # For the run-debuginfod-find.sh test case in %check for
> /usr/sbin/ss
> BuildRequires: iproute
> +%if 0%{?fedora} >= 20
> +BuildRequires: dpkg
> +%endif
> BuildRequires: curl
I believe all public builders (koji, copr, etc) do have epel enabled,
so I would not conditionalize the BuildRequires.
> @@ -205,10 +209,12 @@ rpm_test() {
Maybe rename this archive_test?
Cheers,
Mark
Re: RFCv2: debuginfod debian archive support
Hi Frank,
On Fri, 2019-12-06 at 22:03 -0500, Frank Ch. Eigler wrote:
> @@ -851,7 +867,11 @@ handle_buildid_r_match (int64_t b_mtime,
> > >return 0;
> > > }
> > >
> > > - string popen_cmd = string("rpm2cpio " + shell_escape(b_source0));
> > > + string archive_decoder = "/dev/null";
> > > + for (auto&& arch : scan_archives)
> > > +if (string_endswith(b_source0, arch.first))
> > > + archive_decoder = arch.second;
> > > + string popen_cmd = archive_decoder + " " + shell_escape(b_source0);
> > >FILE* fp = popen (popen_cmd.c_str(), "r"); // "e" O_CLOEXEC?
> > >if (fp == NULL)
> > > throw libc_exception (errno, string("popen ") + popen_cmd);
> >
> > This seems a lot of work for dealing with non-archives.
>
> We don't do this for non-archives. This is in the path where an
> archive record already matched in the database.
>
> > > +archive_classify (const string& rps, sqlite_ps& ps_upsert_buildids,
> > > sqlite_ps& ps_upsert_files,
> > >sqlite_ps& ps_upsert_de, sqlite_ps& ps_upsert_sref,
> > > sqlite_ps& ps_upsert_sdef,
> > >time_t mtime,
> > >unsigned& fts_executable, unsigned& fts_debuginfo,
> > > unsigned& fts_sref, unsigned& fts_sdef,
> > >bool& fts_sref_complete_p)
> > > {
> > > - string popen_cmd = string("rpm2cpio " + shell_escape(rps));
> > > + string archive_decoder = "/dev/null";
> > > + for (auto&& arch : scan_archives)
> > > +if (string_endswith(rps, arch.first))
> > > + archive_decoder = arch.second;
> > > + string popen_cmd = archive_decoder + " " + shell_escape(rps);
> > >FILE* fp = popen (popen_cmd.c_str(), "r"); // "e" O_CLOEXEC?
> > >if (fp == NULL)
> > > throw libc_exception (errno, string("popen ") + popen_cmd);
> >
> > Likewise as above. Can we skip the whole popen dance if nothing
> > matches?
>
> If you check out the caller, this part is not even called if
> the extension does not match.
I see, I missed that both functions are only called after first
checking the archive type. I think it might be helpful/clearer if both
methods would be called with the intended archive type then, also
because that might make it simpler to...
> > But it does feel like the errors, logs and metrics are a little
> > generic (e.g. "cannot select all format").
>
> The way in which specializing the format errors could help if
> debuginfod were run against rpms that had a non-cpio payload, or debs
> that had a non-tar payload. This means some sort of corruption, which
> contravenes our "trustworthy data" assumption -- or upstream policy
> change, which is nothing to worry about.
>
> If you think separate metrics for .deb vs .rpm archives might be
> useful, can do.
If it isn't too much work then I do think it would be useful/clearer if
the logs/metrics reported debs and rpms separately.
Thanks,
Mark
Re: [PATCH] debuginfod: Check the DEBUGINFOD_URLS environment variable early in client.
On Mon, 2019-12-09 at 19:49 +0100, Mark Wielaard wrote: > If the debuginfod-client isn't configured we should do as little > as possible. Simply return early with ENOSYS if no servers are > configured. This means we won't check > > This does change the behavior of the debuginfod_find calls slightly. > Previously we would setup and check the cache if the given build-id > was valid. Which might have provided a result if an earlier client > had run with the same cache and valid server URLs which knew about > that particular build-id. Now we don't return any cached results > unless at least one server is configured. > > This prevents selinux errors when the library is used in a confined > setup. This was tested in Fedora and Frank independently came up with almost the same fix. Pushed to master. Cheers, Mark
[PATCH 0/2] elfutils: minor build system fixes
From: Omar Sandoval Hello, This small series fixes a couple of issues I encountered when building elfutils. Patch 1 fixes an issue when compiling with CFLAGS='-Wno-error -O0'. Patch 2 gets rid of a warning. Thanks! Omar Sandoval (2): configure: Fix -D_FORTIFY_SOURCE=2 check when CFLAGS contains -Wno-error libcpu: Compile i386_lex.c with -Wno-implicit-fallthrough ChangeLog | 5 + configure.ac | 2 +- libcpu/ChangeLog | 4 libcpu/Makefile.am | 3 ++- 4 files changed, 12 insertions(+), 2 deletions(-) -- 2.24.0
[PATCH 2/2] libcpu: Compile i386_lex.c with -Wno-implicit-fallthrough
From: Omar Sandoval elfutils is compiled with -Wimplicit-fallthrough=5, so the fallthrough comment in i386_lex.c (generated by flex) doesn't prevent the implicit fallthrough warning. Add -Wno-implicit-fallthrough to i386_lex_CFLAGS. Signed-off-by: Omar Sandoval --- libcpu/ChangeLog | 4 libcpu/Makefile.am | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/libcpu/ChangeLog b/libcpu/ChangeLog index 52567be8..70796514 100644 --- a/libcpu/ChangeLog +++ b/libcpu/ChangeLog @@ -1,3 +1,7 @@ +2019-12-11 Omar Sandoval + + * Makefile.am (i386_lex_CFLAGS): Add -Wno-implicit-fallthrough. + 2019-10-17 Mark Wielaard * i386_data.h (FCT_sel): Check for param_start + 2 >= end instead diff --git a/libcpu/Makefile.am b/libcpu/Makefile.am index 03c71ea3..59def7d1 100644 --- a/libcpu/Makefile.am +++ b/libcpu/Makefile.am @@ -81,7 +81,8 @@ i386_lex_no_Werror = yes libeu = ../lib/libeu.a -i386_lex_CFLAGS = -Wno-unused-label -Wno-unused-function -Wno-sign-compare +i386_lex_CFLAGS = -Wno-unused-label -Wno-unused-function -Wno-sign-compare \ + -Wno-implicit-fallthrough i386_parse.o: i386_parse.c i386.mnemonics i386_parse_CFLAGS = -DNMNES="`wc -l < i386.mnemonics`" i386_lex.o: i386_parse.h -- 2.24.0
[PATCH 1/2] configure: Fix -D_FORTIFY_SOURCE=2 check when CFLAGS contains -Wno-error
From: Omar Sandoval
If CFLAGS contains -Wno-error, then the check for -D_FORTIFY_SOURCE=2
won't fail when appropriate. E.g., compiling with:
./configure CFLAGS='-Wno-error -O0' &&
Results in a flood of "_FORTIFY_SOURCE requires compiling with
optimization (-O)" warnings.
Make sure we add -Werror after the user-defined CFLAGS.
Signed-off-by: Omar Sandoval
---
ChangeLog| 5 +
configure.ac | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index ed5f5866..ff012e3c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2019-12-11 Omar Sandoval
+
+ * configure.ac: Apply -Werror after user-defined CFLAGS in
+ -D_FORTIFY_SOURCE=2 check.
+
2019-12-06 Mark Wielaard
* configure.ac: Add ac_cv_buildid check.
diff --git a/configure.ac b/configure.ac
index 36a6b6c2..2d0b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -273,7 +273,7 @@ case "$CFLAGS" in
;;
*)
save_CFLAGS="$CFLAGS"
-CFLAGS="-D_FORTIFY_SOURCE=2 -Werror $CFLAGS"
+CFLAGS="-D_FORTIFY_SOURCE=2 $CFLAGS -Werror"
AC_COMPILE_IFELSE([AC_LANG_SOURCE([[
#include
int main() { return 0; }
--
2.24.0
Re: [PATCH] libdwfl: remove broken coalescing logic in dwfl_report_segment()
On Tue, Dec 10, 2019 at 05:28:19PM -0800, Omar Sandoval wrote: > From: Omar Sandoval > > dwfl_report_segment() has some logic that detects when a segment is > contiguous with the previously reported segment, in which case it's > supposed to coalesce them. However, in this case, it actually returns > without updating the segment array at all. As far as I can tell, this > has always been broken. It appears that no one uses the coalescing logic > anyways, as they pass IDENT as NULL. Let's just get rid of the logic and > add a test case. > > Signed-off-by: Omar Sandoval Disregard this patch, I'm going to send it as part of a larger series shortly.
[PATCH 0/4] libdwfl: make dwfl_addrmodule work for Linux kernel modules
From: Omar Sandoval Hello, I recently encountered a bug that dwfl_addrmodule doesn't work correctly for Linux kernel modules. This is because each section of a kernel module is allocated independently, so sections from different kernel modules may be intermixed. For example: # cd /sys/modules # cat ext4/sections/.init.text 0xc0f0f000 # cat ext4/sections/.bss 0xc1303e80 # cat kvm/sections/.init.text 0xc0f06000 # cat kvm/sections/.bss 0xc10d2340 This confuses dwfl_addrmodule/dwfl_addrsegment, which builds a lookup table based on mod->low_addr and mod->high_addr. For relocatable files, we should be using the addresses of each section, instead. Patch 4 makes this change, but it needs some preparation. Patch 1 allows us to distinguish between unloaded sections and sections loaded at zero. This is necessary so that dwfl_addrmodule doesn't map, e.g., 0x123 to a module with an unloaded section of size 0x200. Because indexing every section creates many more lookup entries than we previously had, patch 3 separates the module lookup table from the dwfl_report_segment lookup table. Finally, patch 2 is the patch I sent yesterday, included in this series because it would conflict with the later patches. If these patches are the wrong way to go about this, please consider this a bug report. I'd be happy to test alternative fixes. Thanks! Omar Sandoval (4): libdwfl: return error from __libdwfl_relocate_value for unloaded sections libdwfl: remove broken coalescing logic in dwfl_report_segment libdwfl: store module lookup table separately from segments libdwfl: use sections of relocatable files for dwfl_addrmodule .gitignore | 1 + libdwfl/ChangeLog | 30 + libdwfl/derelocate.c | 24 +--- libdwfl/dwfl_addrmodule.c | 106 ++- libdwfl/dwfl_getmodules.c | 14 +- libdwfl/dwfl_module.c | 11 +- libdwfl/dwfl_module_getsym.c | 3 +- libdwfl/libdwfl.h | 20 +-- libdwfl/libdwflP.h | 42 -- libdwfl/link_map.c | 7 +- libdwfl/relocate.c | 9 +- libdwfl/segment.c | 178 +++-- tests/ChangeLog| 5 + tests/Makefile.am | 6 +- tests/dwfl-report-segment-contiguous.c | 82 15 files changed, 305 insertions(+), 233 deletions(-) create mode 100644 tests/dwfl-report-segment-contiguous.c -- 2.24.0
[PATCH 3/4] libdwfl: store module lookup table separately from segments
From: Omar Sandoval
Currently, the address ranges for segments reported with
dwfl_report_segment and modules are stored in the same sorted array.
This requires complicated logic in reify_segments for splitting up
existing segments and inserting into the table, which can have quadratic
runtime in the worst case. Simplify it by adding a separate table for
the module list, which we can manage much more simply. This will be
especially important for the next change.
Signed-off-by: Omar Sandoval
---
libdwfl/ChangeLog | 14 +++-
libdwfl/dwfl_addrmodule.c | 89 +++-
libdwfl/dwfl_getmodules.c | 14 ++--
libdwfl/dwfl_module.c | 11 +--
libdwfl/libdwflP.h| 17 -
libdwfl/link_map.c| 7 +-
libdwfl/segment.c | 143 +-
7 files changed, 128 insertions(+), 167 deletions(-)
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 09a4a473..8dc20961 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -3,12 +3,24 @@
* libdwflP.h: Add new NOT_LOADED DWFL_ERROR.
(Dwfl_Module): Remove coalescing state.
Rename lookup_tail_ndx to next_segndx.
+ Rename segment member to lookup.
+ (Dwfl): Replace module lookup table with new definition.
* relocate.c (__libdwfl_relocate_value): Return
DWFL_E_NOT_LOADED if section is not loaded.
(relocate): Handle DWFL_E_NOT_LOADED.
* dwfl_module_getsym: Ignore DWFL_E_NOT_LOADED.
- * segment.c (dwfl_report_segment): Remove coalescing logic.
+ * segment.c: Remove old module lookup table implementation.
+ (dwfl_report_segment): Remove coalescing logic.
+ (dwfl_addrsegment): Use dwfl_addrmodule to get module.
* libdwfl.h (dwfl_report_segment): Document that IDENT is now ignored.
+ * dwfl_addrmodule.c: Add new module lookup table implementation.
+ (dwfl_addrmodule): Use new module lookup table instead of
+ dwfl_addrsegment.
+ * dwfl_getmodules.c (dwfl_getmodules): Use new module lookup table.
+ * dwfl_module.c (dwfl_report_begin): Clear new module lookup table.
+ (use): Likewise.
+ * link_map.c: Use dwfl_addrmodule instead of dwfl_addrsegment when
+ segment index is not needed.
2019-12-05 Mark Wielaard
diff --git a/libdwfl/dwfl_addrmodule.c b/libdwfl/dwfl_addrmodule.c
index abf1ff48..21db4883 100644
--- a/libdwfl/dwfl_addrmodule.c
+++ b/libdwfl/dwfl_addrmodule.c
@@ -32,11 +32,94 @@
#include "libdwflP.h"
+static bool append_lookup_module (Dwfl *dwfl, Dwfl_Module *mod, GElf_Addr
start,
+ GElf_Addr end)
+{
+ if (dwfl->lookup_module_elts >= dwfl->lookup_module_alloc)
+{
+ size_t n = dwfl->lookup_module_alloc;
+ n = n == 0 ? 16 : n * 2;
+ struct dwfl_module_segment *tmp;
+ tmp = realloc (dwfl->lookup_module, n * sizeof (tmp[0]));
+ if (tmp == NULL)
+ {
+ __libdwfl_seterrno (DWFL_E_NOMEM);
+ return true;
+ }
+ dwfl->lookup_module = tmp;
+ dwfl->lookup_module_alloc = n;
+}
+ size_t i = dwfl->lookup_module_elts++;
+ dwfl->lookup_module[i].start = start;
+ dwfl->lookup_module[i].end = end;
+ dwfl->lookup_module[i].mod = mod;
+ return false;
+}
+
+static int compare_dwfl_module_segment (const void *a, const void *b)
+{
+ const struct dwfl_module_segment *seg1 = a;
+ const struct dwfl_module_segment *seg2 = b;
+ if (seg1->start < seg2->start)
+return -1;
+ else if (seg1->start > seg2->start)
+return 1;
+ else
+return 0;
+}
+
+static bool
+create_lookup_module (Dwfl *dwfl)
+{
+ for (Dwfl_Module *mod = dwfl->modulelist; mod != NULL; mod = mod->next)
+if (! mod->gc)
+ {
+ GElf_Addr start = __libdwfl_segment_start(dwfl, mod->low_addr);
+ GElf_Addr end = __libdwfl_segment_end(dwfl, mod->high_addr);
+ if (append_lookup_module(dwfl, mod, start, end))
+ return true;
+ }
+
+ qsort (dwfl->lookup_module, dwfl->lookup_module_elts,
+sizeof (dwfl->lookup_module[0]), compare_dwfl_module_segment);
+ for (size_t i = 0; i < dwfl->lookup_module_elts; i++)
+{
+ dwfl->lookup_module[i].mod->lookup = i;
+ /* If the upper boundary of the segment isn't part of the next segment,
+treat it as part of the segment. */
+ if (i == dwfl->lookup_module_elts - 1
+ || dwfl->lookup_module[i].end < dwfl->lookup_module[i + 1].start)
+ dwfl->lookup_module[i].end++;
+}
+ return false;
+}
+
+static int search_dwfl_module_segment (const void *key, const void *elt)
+{
+ Dwarf_Addr address = *(Dwarf_Addr *)key;
+ const struct dwfl_module_segment *seg = elt;
+ if (address < seg->start)
+return -1;
+ else if (address >= seg->end)
+return 1;
+ else
+return 0;
+}
+
Dwfl_Module *
dwfl_addrmodule (Dwfl *dwfl, Dwarf_Addr address)
{
- Dwfl_Module *mod;
- (void) INTUSE(dwfl_addrsegment) (dwfl, address, &mod);
- return mod;
+ if (unlikely (dwfl == NULL)
[PATCH 4/4] libdwfl: use sections of relocatable files for dwfl_addrmodule
From: Omar Sandoval
dwfl_addrmodule matches a module if the address lies within low_addr and
high_addr. This is incorrect for relocatable files, particularly kernel
modules: sections of different modules can be intermingled within the
same range of addresses. Instead, we should index each section
independently.
Signed-off-by: Omar Sandoval
---
libdwfl/ChangeLog | 6 ++
libdwfl/derelocate.c | 24 ++--
libdwfl/dwfl_addrmodule.c | 25 +
libdwfl/libdwflP.h| 17 +
4 files changed, 50 insertions(+), 22 deletions(-)
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 8dc20961..d55aeb47 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -4,6 +4,8 @@
(Dwfl_Module): Remove coalescing state.
Rename lookup_tail_ndx to next_segndx.
Rename segment member to lookup.
+ (struct dwfl_relocation): Move from derelocate.c
+ (__libdwfl_cache_sections): Add.
(Dwfl): Replace module lookup table with new definition.
* relocate.c (__libdwfl_relocate_value): Return
DWFL_E_NOT_LOADED if section is not loaded.
@@ -14,6 +16,7 @@
(dwfl_addrsegment): Use dwfl_addrmodule to get module.
* libdwfl.h (dwfl_report_segment): Document that IDENT is now ignored.
* dwfl_addrmodule.c: Add new module lookup table implementation.
+ Index sections for relocatable files instead of full address range.
(dwfl_addrmodule): Use new module lookup table instead of
dwfl_addrsegment.
* dwfl_getmodules.c (dwfl_getmodules): Use new module lookup table.
@@ -21,6 +24,9 @@
(use): Likewise.
* link_map.c: Use dwfl_addrmodule instead of dwfl_addrsegment when
segment index is not needed.
+ * derelocate.c (struct dwfl_relocation): Move to libdwflP.h.
+ (cache_sections): Rename to __libdwfl_cache_sections and make
+ non-static.
2019-12-05 Mark Wielaard
diff --git a/libdwfl/derelocate.c b/libdwfl/derelocate.c
index 2f80b20f..238aa6e2 100644
--- a/libdwfl/derelocate.c
+++ b/libdwfl/derelocate.c
@@ -32,19 +32,6 @@
#include "libdwflP.h"
-struct dwfl_relocation
-{
- size_t count;
- struct
- {
-Elf_Scn *scn;
-Elf_Scn *relocs;
-const char *name;
-GElf_Addr start, end;
- } refs[0];
-};
-
-
struct secref
{
struct secref *next;
@@ -76,8 +63,9 @@ compare_secrefs (const void *a, const void *b)
return elf_ndxscn ((*p1)->scn) - elf_ndxscn ((*p2)->scn);
}
-static int
-cache_sections (Dwfl_Module *mod)
+int
+internal_function
+__libdwfl_cache_sections (Dwfl_Module *mod)
{
if (likely (mod->reloc_info != NULL))
return mod->reloc_info->count;
@@ -242,7 +230,7 @@ dwfl_module_relocations (Dwfl_Module *mod)
switch (mod->e_type)
{
case ET_REL:
- return cache_sections (mod);
+ return __libdwfl_cache_sections (mod);
case ET_DYN:
return 1;
@@ -278,7 +266,7 @@ dwfl_module_relocation_info (Dwfl_Module *mod, unsigned int
idx,
return NULL;
}
- if (cache_sections (mod) < 0)
+ if (__libdwfl_cache_sections (mod) < 0)
return NULL;
struct dwfl_relocation *sections = mod->reloc_info;
@@ -330,7 +318,7 @@ check_module (Dwfl_Module *mod)
static int
find_section (Dwfl_Module *mod, Dwarf_Addr *addr)
{
- if (cache_sections (mod) < 0)
+ if (__libdwfl_cache_sections (mod) < 0)
return -1;
struct dwfl_relocation *sections = mod->reloc_info;
diff --git a/libdwfl/dwfl_addrmodule.c b/libdwfl/dwfl_addrmodule.c
index 21db4883..1eb45317 100644
--- a/libdwfl/dwfl_addrmodule.c
+++ b/libdwfl/dwfl_addrmodule.c
@@ -74,10 +74,27 @@ create_lookup_module (Dwfl *dwfl)
for (Dwfl_Module *mod = dwfl->modulelist; mod != NULL; mod = mod->next)
if (! mod->gc)
{
- GElf_Addr start = __libdwfl_segment_start(dwfl, mod->low_addr);
- GElf_Addr end = __libdwfl_segment_end(dwfl, mod->high_addr);
- if (append_lookup_module(dwfl, mod, start, end))
- return true;
+ Dwarf_Addr bias;
+ if (mod->dwfl->callbacks->find_elf
+ && dwfl_module_getdwarf(mod, &bias)
+ && mod->e_type == ET_REL)
+ {
+ if (__libdwfl_cache_sections (mod) < 0)
+ return true;
+
+ struct dwfl_relocation *sections = mod->reloc_info;
+ for (size_t i = 0; i < sections->count; i++)
+ if (append_lookup_module(dwfl, mod, sections->refs[i].start,
+ sections->refs[i].end))
+ return true;
+ }
+ else
+ {
+ GElf_Addr start = __libdwfl_segment_start(dwfl, mod->low_addr);
+ GElf_Addr end = __libdwfl_segment_end(dwfl, mod->high_addr);
+ if (append_lookup_module(dwfl, mod, start, end))
+ return true;
+ }
}
qsort (dwfl->lookup_module, dwfl->lookup_module_elts,
diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
index 6661cc4c..2589eb66 100644
---
[PATCH 1/4] libdwfl: return error from __libdwfl_relocate_value for unloaded sections
From: Omar Sandoval
Currently, __libdwfl_relocate_value doesn't distinguish between unloaded
sections and sections loaded at address zero. This has a few
consequences:
* relocate.c attempts relocation on unloaded sections when we don't have
anything meaningful to relocate against.
* derelocate.c matches addresses which happen to be less than the
sh_size of an unloaded section, which can lead to confusing results
from dwfl_module_relocate_address and __libdwfl_find_section_ndx.
* find_elf_build_id returns an invalid non-zero address if the build ID
note is not loaded.
Let's return a new error, DWFL_E_NOT_LOADED, from
__libdwfl_relocate_value if the section is not loaded that callers can
handle appropriately.
Signed-off-by: Omar Sandoval
---
libdwfl/ChangeLog| 8
libdwfl/dwfl_module_getsym.c | 3 ++-
libdwfl/libdwflP.h | 3 ++-
libdwfl/relocate.c | 9 ++---
4 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index b6b427d4..b00ac8d6 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,11 @@
+2019-12-11 Omar Sandoval
+
+ * libdwflP.h: Add new NOT_LOADED DWFL_ERROR.
+ * relocate.c (__libdwfl_relocate_value): Return
+ DWFL_E_NOT_LOADED if section is not loaded.
+ (relocate): Handle DWFL_E_NOT_LOADED.
+ * dwfl_module_getsym: Ignore DWFL_E_NOT_LOADED.
+
2019-12-05 Mark Wielaard
* linux-kernel-modules.c (find_kernel_elf): Also try to find
diff --git a/libdwfl/dwfl_module_getsym.c b/libdwfl/dwfl_module_getsym.c
index 8de9a3eb..d75588b2 100644
--- a/libdwfl/dwfl_module_getsym.c
+++ b/libdwfl/dwfl_module_getsym.c
@@ -165,7 +165,8 @@ __libdwfl_getsym (Dwfl_Module *mod, int ndx, GElf_Sym *sym,
GElf_Addr *addr,
Dwfl_Error result = __libdwfl_relocate_value (mod, elf,
&symshstrndx,
shndx, &st_value);
- if (unlikely (result != DWFL_E_NOERROR))
+ if (unlikely (result != DWFL_E_NOERROR
+ && result != DWFL_E_NOT_LOADED))
{
__libdwfl_seterrno (result);
return NULL;
diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
index f631f946..6c10eddc 100644
--- a/libdwfl/libdwflP.h
+++ b/libdwfl/libdwflP.h
@@ -90,7 +90,8 @@ typedef struct Dwfl_Process Dwfl_Process;
DWFL_ERROR (NO_ATTACH_STATE, N_("Dwfl has no attached state")) \
DWFL_ERROR (NO_UNWIND, N_("Unwinding not supported for this architecture")) \
DWFL_ERROR (INVALID_ARGUMENT, N_("Invalid argument"))
\
- DWFL_ERROR (NO_CORE_FILE, N_("Not an ET_CORE ELF file"))
+ DWFL_ERROR (NO_CORE_FILE, N_("Not an ET_CORE ELF file")) \
+ DWFL_ERROR (NOT_LOADED, N_("Not loaded"))
#define DWFL_ERROR(name, text) DWFL_E_##name,
typedef enum { DWFL_ERRORS DWFL_E_NUM } Dwfl_Error;
diff --git a/libdwfl/relocate.c b/libdwfl/relocate.c
index 88b5211d..5c9c08f3 100644
--- a/libdwfl/relocate.c
+++ b/libdwfl/relocate.c
@@ -73,9 +73,8 @@ __libdwfl_relocate_value (Dwfl_Module *mod, Elf *elf, size_t
*shstrndx,
return CBFAIL;
if (refshdr->sh_addr == (Dwarf_Addr) -1l)
- /* The callback indicated this section wasn't really loaded but we
- don't really care. */
- refshdr->sh_addr = 0; /* Make no adjustment below. */
+ /* The callback indicated this section wasn't loaded. */
+ return DWFL_E_NOT_LOADED;
/* Update the in-core file's section header to show the final
load address (or unloadedness). This serves as a cache,
@@ -361,6 +360,8 @@ relocate (Dwfl_Module * const mod,
GElf_Word shndx;
Dwfl_Error error = relocate_getsym (mod, relocated, reloc_symtab,
symndx, &sym, &shndx);
+ if (error == DWFL_E_NOT_LOADED)
+ return DWFL_E_NOERROR;
if (unlikely (error != DWFL_E_NOERROR))
return error;
@@ -368,6 +369,8 @@ relocate (Dwfl_Module * const mod,
{
/* Maybe we can figure it out anyway. */
error = resolve_symbol (mod, reloc_symtab, &sym, shndx);
+ if (error == DWFL_E_NOT_LOADED)
+ return DWFL_E_NOERROR;
if (error != DWFL_E_NOERROR
&& !(error == DWFL_E_RELUNDEF && shndx == SHN_COMMON))
return error;
--
2.24.0
[PATCH 2/4] libdwfl: remove broken coalescing logic in dwfl_report_segment
From: Omar Sandoval
dwfl_report_segment has some logic that detects when a segment is
contiguous with the previously reported segment, in which case it's
supposed to coalesce them. However, in this case, it actually returns
without updating the segment array at all. As far as I can tell, this
has always been broken. It appears that no one uses the coalescing logic
anyways, as they pass IDENT as NULL. Let's just get rid of the logic and
add a test case.
Signed-off-by: Omar Sandoval
---
.gitignore | 1 +
libdwfl/ChangeLog | 4 ++
libdwfl/libdwfl.h | 20 ++-
libdwfl/libdwflP.h | 7 +--
libdwfl/segment.c | 35 +--
tests/ChangeLog| 5 ++
tests/Makefile.am | 6 +-
tests/dwfl-report-segment-contiguous.c | 82 ++
8 files changed, 115 insertions(+), 45 deletions(-)
create mode 100644 tests/dwfl-report-segment-contiguous.c
diff --git a/.gitignore b/.gitignore
index 72f22855..43fb7275 100644
--- a/.gitignore
+++ b/.gitignore
@@ -112,6 +112,7 @@ Makefile.in
/tests/dwfl-bug-report
/tests/dwfl-proc-attach
/tests/dwfl-report-elf-align
+/tests/dwfl-report-segment-contiguous
/tests/dwfllines
/tests/dwflmodtest
/tests/dwflsyms
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index b00ac8d6..09a4a473 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,10 +1,14 @@
2019-12-11 Omar Sandoval
* libdwflP.h: Add new NOT_LOADED DWFL_ERROR.
+ (Dwfl_Module): Remove coalescing state.
+ Rename lookup_tail_ndx to next_segndx.
* relocate.c (__libdwfl_relocate_value): Return
DWFL_E_NOT_LOADED if section is not loaded.
(relocate): Handle DWFL_E_NOT_LOADED.
* dwfl_module_getsym: Ignore DWFL_E_NOT_LOADED.
+ * segment.c (dwfl_report_segment): Remove coalescing logic.
+ * libdwfl.h (dwfl_report_segment): Document that IDENT is now ignored.
2019-12-05 Mark Wielaard
diff --git a/libdwfl/libdwfl.h b/libdwfl/libdwfl.h
index a0c1d357..d5fa06d4 100644
--- a/libdwfl/libdwfl.h
+++ b/libdwfl/libdwfl.h
@@ -111,7 +111,7 @@ extern void dwfl_report_begin (Dwfl *dwfl);
/* Report that segment NDX begins at PHDR->p_vaddr + BIAS.
If NDX is < 0, the value succeeding the last call's NDX
- is used instead (zero on the first call).
+ is used instead (zero on the first call). IDENT is ignored.
If nonzero, the smallest PHDR->p_align value seen sets the
effective page size for the address space DWFL describes.
@@ -120,21 +120,9 @@ extern void dwfl_report_begin (Dwfl *dwfl);
Returns -1 for errors, or NDX (or its assigned replacement) on success.
- When NDX is the value succeeding the last call's NDX (or is implicitly
- so as above), IDENT is nonnull and matches the value in the last call,
- and the PHDR and BIAS values reflect a segment that would be contiguous,
- in both memory and file, with the last segment reported, then this
- segment may be coalesced internally with preceding segments. When given
- an address inside this segment, dwfl_addrsegment may return the NDX of a
- preceding contiguous segment. To prevent coalesced segments, always
- pass a null pointer for IDENT.
-
- The values passed are not stored (except to track coalescence).
- The only information that can be extracted from DWFL later is the
- mapping of an address to a segment index that starts at or below
- it. Reporting segments at all is optional. Its only benefit to
- the caller is to offer this quick lookup via dwfl_addrsegment,
- or use other segment-based calls. */
+ Reporting segments at all is optional. Its only benefit to the caller is to
+ offer this quick lookup via dwfl_addrsegment, or use other segment-based
+ calls. */
extern int dwfl_report_segment (Dwfl *dwfl, int ndx,
const GElf_Phdr *phdr, GElf_Addr bias,
const void *ident);
diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
index 6c10eddc..d21471b1 100644
--- a/libdwfl/libdwflP.h
+++ b/libdwfl/libdwflP.h
@@ -133,12 +133,7 @@ struct Dwfl
GElf_Addr *lookup_addr; /* Start address of segment. */
Dwfl_Module **lookup_module; /* Module associated with segment, or null. */
int *lookup_segndx; /* User segment index, or -1. */
-
- /* Cache from last dwfl_report_segment call. */
- const void *lookup_tail_ident;
- GElf_Off lookup_tail_vaddr;
- GElf_Off lookup_tail_offset;
- int lookup_tail_ndx;
+ int next_segndx;
struct Dwfl_User_Core *user_core;
};
diff --git a/libdwfl/segment.c b/libdwfl/segment.c
index d9599a7f..f6a3e84e 100644
--- a/libdwfl/segment.c
+++ b/libdwfl/segment.c
@@ -287,11 +287,15 @@ int
dwfl_report_segment (Dwfl *dwfl, int ndx, const GElf_Phdr *phdr, GElf_Addr
bias,
const void *ident)
{
+ /* This was previously used for coalescing s
