[PATCH 2/4] libdwfl: remove broken coalescing logic in dwfl_report_segment
Omar Sandoval
osandov@osandov.com
Thu Dec 12 01:30:00 GMT 2019
From: Omar Sandoval <osandov@fb.com>
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 <osandov@fb.com>
---
.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 <osandov@fb.com>
* 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 <mark@klomp.org>
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 segments, but it was buggy since
+ day one. We don't use it anymore. */
+ (void)ident;
+
if (dwfl == NULL)
return -1;
if (ndx < 0)
- ndx = dwfl->lookup_tail_ndx;
+ ndx = dwfl->next_segndx;
if (phdr->p_align > 1 && (dwfl->segment_align <= 1 ||
phdr->p_align < dwfl->segment_align))
@@ -307,30 +311,19 @@ dwfl_report_segment (Dwfl *dwfl, int ndx, const GElf_Phdr *phdr, GElf_Addr bias,
GElf_Addr end = __libdwfl_segment_end (dwfl,
bias + phdr->p_vaddr + phdr->p_memsz);
- /* Coalesce into the last one if contiguous and matching. */
- if (ndx != dwfl->lookup_tail_ndx
- || ident == NULL
- || ident != dwfl->lookup_tail_ident
- || start != dwfl->lookup_tail_vaddr
- || phdr->p_offset != dwfl->lookup_tail_offset)
- {
- /* Normally just appending keeps us sorted. */
+ /* Normally just appending keeps us sorted. */
- size_t i = dwfl->lookup_elts;
- while (i > 0 && unlikely (start < dwfl->lookup_addr[i - 1]))
- --i;
+ size_t i = dwfl->lookup_elts;
+ while (i > 0 && unlikely (start < dwfl->lookup_addr[i - 1]))
+ --i;
- if (unlikely (insert (dwfl, i, start, end, ndx)))
- {
- __libdwfl_seterrno (DWFL_E_NOMEM);
- return -1;
- }
+ if (unlikely (insert (dwfl, i, start, end, ndx)))
+ {
+ __libdwfl_seterrno (DWFL_E_NOMEM);
+ return -1;
}
- dwfl->lookup_tail_ident = ident;
- dwfl->lookup_tail_vaddr = end;
- dwfl->lookup_tail_offset = end - bias - phdr->p_vaddr + phdr->p_offset;
- dwfl->lookup_tail_ndx = ndx + 1;
+ dwfl->next_segndx = ndx + 1;
return ndx;
}
diff --git a/tests/ChangeLog b/tests/ChangeLog
index ce67ce55..30bd8256 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,8 @@
+2019-12-11 Omar Sandoval <osandov@fb.com>
+
+ * dwfl-report-segment-coalesce.c: New test.
+ * Makefile.am: Add dwfl-report-segment-coalesce
+
2019-12-06 Mark Wielaard <mark@klomp.org>
* run-debuginfod-find.sh: Force -Wl,--build-id.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index eab4ae6f..2c3ac299 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -50,7 +50,8 @@ check_PROGRAMS = arextract arsymtest newfile saridx scnnames sectiondump \
test-flag-nobits dwarf-getstring rerequest_tag \
alldts typeiter typeiter2 low_high_pc \
test-elf_cntl_gelf_getshdr dwflsyms dwfllines \
- dwfl-report-elf-align varlocs backtrace backtrace-child \
+ dwfl-report-elf-align dwfl-report-segment-contiguous \
+ varlocs backtrace backtrace-child \
backtrace-data backtrace-dwarf debuglink debugaltlink \
buildid deleted deleted-lib.so aggregate_size peel_type \
vdsosyms \
@@ -113,7 +114,7 @@ TESTS = run-arextract.sh run-arsymtest.sh run-ar.sh newfile test-nlist \
run-native-test.sh run-bug1-test.sh \
run-debuglink.sh run-debugaltlink.sh run-buildid.sh \
dwfl-bug-addr-overflow run-addrname-test.sh \
- dwfl-bug-fd-leak dwfl-bug-report \
+ dwfl-bug-fd-leak dwfl-bug-report dwfl-report-segment-contiguous \
run-dwfl-bug-offline-rel.sh run-dwfl-addr-sect.sh \
run-disasm-x86.sh run-disasm-x86-64.sh \
run-early-offscn.sh run-dwarf-getmacros.sh run-dwarf-ranges.sh \
@@ -589,6 +590,7 @@ test_elf_cntl_gelf_getshdr_LDADD = $(libelf)
dwflsyms_LDADD = $(libdw) $(libelf) $(argp_LDADD)
dwfllines_LDADD = $(libdw) $(libelf) $(argp_LDADD)
dwfl_report_elf_align_LDADD = $(libdw)
+dwfl_report_segment_contiguous_LDADD = $(libdw) $(libebl) $(libelf)
varlocs_LDADD = $(libdw) $(libelf) $(argp_LDADD)
backtrace_LDADD = $(libdw) $(libelf) $(argp_LDADD)
# backtrace-child-biarch also uses those *_CFLAGS and *_LDLAGS variables:
diff --git a/tests/dwfl-report-segment-contiguous.c b/tests/dwfl-report-segment-contiguous.c
new file mode 100644
index 00000000..61e6bfee
--- /dev/null
+++ b/tests/dwfl-report-segment-contiguous.c
@@ -0,0 +1,82 @@
+/* Test bug in dwfl_report_segment() coalescing.
+ Copyright (C) 2019 Facebook
+ This file is part of elfutils.
+
+ This file is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ elfutils is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include <config.h>
+#include <assert.h>
+#include <inttypes.h>
+#include <stdio.h>
+#include <stdio_ext.h>
+#include <locale.h>
+#include ELFUTILS_HEADER(dwfl)
+
+
+static const Dwfl_Callbacks offline_callbacks =
+ {
+ .find_debuginfo = INTUSE(dwfl_standard_find_debuginfo),
+ .section_address = INTUSE(dwfl_offline_section_address),
+ };
+
+
+int
+main (void)
+{
+ /* We use no threads here which can interfere with handling a stream. */
+ (void) __fsetlocking (stdout, FSETLOCKING_BYCALLER);
+
+ /* Set locale. */
+ (void) setlocale (LC_ALL, "");
+
+ Dwfl *dwfl = dwfl_begin (&offline_callbacks);
+ assert (dwfl != NULL);
+
+ GElf_Phdr phdr1 =
+ {
+ .p_type = PT_LOAD,
+ .p_flags = PF_R,
+ .p_offset = 0xf00,
+ .p_vaddr = 0xf00,
+ .p_filesz = 0x100,
+ .p_memsz = 0x100,
+ .p_align = 4,
+ };
+
+ int ndx = dwfl_report_segment (dwfl, 1, &phdr1, 0, dwfl);
+ assert(ndx == 1);
+
+ ndx = dwfl_addrsegment (dwfl, 0xf00, NULL);
+ assert(ndx == 1);
+
+ GElf_Phdr phdr2 =
+ {
+ .p_type = PT_LOAD,
+ .p_flags = PF_R | PF_W,
+ .p_offset = 0x1000,
+ .p_vaddr = 0x1000,
+ .p_filesz = 0x100,
+ .p_memsz = 0x100,
+ .p_align = 4,
+ };
+ ndx = dwfl_report_segment (dwfl, 2, &phdr2, 0, dwfl);
+ assert(ndx == 2);
+
+ ndx = dwfl_addrsegment (dwfl, 0x1000, NULL);
+ assert(ndx == 1 || ndx == 2);
+
+ dwfl_end (dwfl);
+
+ return 0;
+}
--
2.24.0
More information about the Elfutils-devel
mailing list