Re: [PATCH] readelf: Don't print average number of tests when no tests are done
Hi,
On Wed, 2023-11-15 at 17:41 +0100, Mark Wielaard wrote:
> If the symbol hash table only contains lenght zero chains, no lookup
> tests need to be done and eu-readelf -I would print out bogus numbers
> for the number of tests that were successful/unsuccessful.
>
> e.g. for an "empty" program like
> int main() {}
> eu-readelf -I would print:
>
> Histogram for bucket list length in section [ 5] '.gnu.hash' (total of 1
> bucket):
> Addr: 0x004003c0 Offset: 0x0003c0 Link to section: [ 6] '.dynsym'
> Symbol Bias: 1
> Bitmask Size: 8 bytes 0% bits set 2nd hash shift: 0
> Length Number % of total Coverage
> 0 1 100.0%
> Average number of tests: successful lookup: -nan
> unsuccessful lookup: 0.00
>
> Only print out the Average number of tests when there were actual
> tests to do.
Pushed,
Mark
Re: [PATCH 2/2] tests: Add test for duplicate entries in archive
Hello Mark, On Sat, Nov 18, 2023 at 10:50 PM Mark Wielaard wrote: > Do note that you also have to add the new test file to > EXTRA_DIST so it actually gets into the dist. Thanks, will do in [PATCH v2].
Re: [PATCH 1/2] libdwfl: handle duplicate ELFs when reporting archives
Hello Mark, On Sat, Nov 18, 2023 at 10:47 PM Mark Wielaard wrote: > If we goto overlap here don't we still have a problem? overlap will > set m->gc = true; and return NULL. So the caller will think they > still owns the elf handle and will probably close it. But then when > the module is GCed in dwfl_report_end it will close the elf handle > again. Thank you for noticing! Yes, this is oversight from my side. > Should we instead move the elf_end and reassignment of main.elf to > after this if statement? Will fix exactly like this in [PATCH v2].
[PATCH v2 1/2] libdwfl: handle duplicate ELFs when reporting archives
When archive is processed in process_archive (libdwfl/offline.c), it
creates an Elf object for each archive member. Then in
process_archive_member it calls process_file to create a Dwfl_Module
through __libdwfl_report_elf.
The ownership of the Elf object is expected to be:
* either transfered to the Dwfl_Module, if __libdwfl_report_elf returns
not NULL;
* or handled at the end of process_archive_member by calling elf_end.
Moreover, Elf object is expected to be alive, if __libdwfl_report_elf
returns not NULL, because at the end of process_archive_member it
advances to the next member through the elf_next call.
The problem happens when __libdwfl_report_elf encounters Elf with the
same name and content as it seen before. In that case dwfl_report_module
will reuse existing Dwfl_Module object. This leads to a codepath that
calls elf_end on the Elf object, while returning not NULL, breaking the
elf_next call to the next member.
The fix is to destroy m->main.elf instead and put the new Elf object in
the already existing Dwfl_Module.
* libdwfl/dwfl_report_elf.c (__libdwfl_report_elf): Replace Elf in
the Dwfl_Module in case of duplicate modules to prolong its
lifetime for subsequent processing.
Signed-off-by: Aleksei Vetrov
---
libdwfl/dwfl_report_elf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libdwfl/dwfl_report_elf.c b/libdwfl/dwfl_report_elf.c
index 581f4079..a76d3681 100644
--- a/libdwfl/dwfl_report_elf.c
+++ b/libdwfl/dwfl_report_elf.c
@@ -276,10 +276,11 @@ __libdwfl_report_elf (Dwfl *dwfl, const char *name, const
char *file_name,
}
else
{
- elf_end (elf);
if (m->main_bias != bias
|| m->main.vaddr != vaddr || m->main.address_sync != address_sync)
goto overlap;
+ elf_end (m->main.elf);
+ m->main.elf = elf;
}
}
return m;
--
2.43.0.rc1.413.gea7ed67945-goog
[PATCH v2 2/2] tests: Add test for duplicate entries in archive
Test dwfl-report-offline-memory against an archive that contains
non-relocatable ELFs with the same name and contents.
* tests/test-ar-duplicates.a.bz2: New test file.
* tests/run-dwfl-report-offline-memory.sh: Test new
test-ar-duplicates.a.bz2.
* tests/Makefile.am (EXTRA_DIST): Add test-ar-duplicates.a.bz2.
Signed-off-by: Aleksei Vetrov
---
tests/Makefile.am | 3 ++-
tests/run-dwfl-report-offline-memory.sh | 7 +++
tests/test-ar-duplicates.a.bz2 | Bin 0 -> 783 bytes
3 files changed, 9 insertions(+), 1 deletion(-)
create mode 100644 tests/test-ar-duplicates.a.bz2
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7fb8efb1..80f6cb59 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -632,7 +632,8 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
run-nvidia-extended-linemap-libdw.sh
run-nvidia-extended-linemap-readelf.sh \
testfile_nvidia_linemap.bz2 \
testfile-largealign.o.bz2 run-strip-largealign.sh \
-run-funcretval++11.sh
+run-funcretval++11.sh \
+test-ar-duplicates.a.bz2
if USE_VALGRIND
diff --git a/tests/run-dwfl-report-offline-memory.sh
b/tests/run-dwfl-report-offline-memory.sh
index 644a45dc..85f43f53 100755
--- a/tests/run-dwfl-report-offline-memory.sh
+++ b/tests/run-dwfl-report-offline-memory.sh
@@ -20,7 +20,14 @@
testfiles testfile-dwfl-report-elf-align-shlib.so
testfiles testarchive64.a
+# echo "int _start(void) { return 0; }" > test.c
+# gcc test.c -nostdlib -static -o test.o
+# ar -r test-ar-duplicates.a test.o test.o test.o
+# bzip2 -zf test-ar-duplicates.a
+testfiles test-ar-duplicates.a
+
testrun ${abs_builddir}/dwfl-report-offline-memory
./testfile-dwfl-report-elf-align-shlib.so 1
testrun ${abs_builddir}/dwfl-report-offline-memory ./testarchive64.a 3
+testrun ${abs_builddir}/dwfl-report-offline-memory ./test-ar-duplicates.a 1
exit 0
diff --git a/tests/test-ar-duplicates.a.bz2 b/tests/test-ar-duplicates.a.bz2
new file mode 100644
index
..e8168c6bb813bb05292372d6b856e2ca1163a835
GIT binary patch
literal 783
zcmV+q1MvJpT4*^jL0KkKS*O-t`~U>U|NsC0^v!1T{dI5Uazg*-o^**RKmrI*1V9J?
z0RUpbpefJ;J^%m!H~;_|001&HG5`Po00EJZ00u$8007Vc0g<7Q042*yPG7bO$
z27mwzjSPSQ0001FWB>t>Z~y=_003lYWB>pF00Sc+01Sb>Qb4DoG|(YN(+YV{O*J>F
zZ891m=`tHrY3VgR0F2UlO|*o0s(PNL3ovPT-FWk=B;Jux%~F)fXQs4SQOt9gyiE>j
zn6Ga(89!@GYHl-L-dxbs$}&@+(DyVdX=!IB!kuQ))6nI$Da(vh*7{bCREV6#KNqm!
zdu&Mb+M`10By`;OIdVe?-cr&jN>kKstt5_(4hBpYI!%S0RYq!zoTdw-r};8m5SuEI
zu&BS|S>97Ll_Ks%qWX~|eNFX2+)(wDko6K&h@AyU+*7EjB0KJbX(i2-NWr+XprTIu
zB%S!mUnj<+n4}YmKId7>S^5_w!Fy?EWo4tTvdK6U;gV9C5?V$|i8mD;#XsVDVQ%g0=kvP>JW-B*XEa@sbQG}Le!OvXnKQ+tOr1|<*S=e&p2DS+rYDV*_vs1e6
zu1U>irf%B#9wjbP_72;9ds3ZsFn8O_*f5OUXClse^_%iUp6rvfsXM*Jq^>8z@U|Q6
z*J1OxE^e`H$%>n8wk=MZHxzfH?{*p^+fvGr{$BN;1J#puO|@+L>>t^ZP1xPOJkJDd
zG&~LmIfeUCo0!>Vz3RBGUhvbk7`3dlX0%-$MeRv3e^oYHE+>J;`q?pM6W?@SQ#QEc
ze~0GQ%}g2{wdp?SsT`$F_Z(%Ds5sw)!*Wg-`sD8<;cAYoXHLwT6*G9*M;hmeoNOvD
zUXsh63|o(x!uDHDwwJQbnE~C
literal 0
HcmV?d1
--
2.43.0.rc1.413.gea7ed67945-goog
