[patch] Fix readelf SEGV for >1 files
by Jan Kratochvil
Hi Roland,
jankratochvil/readelf-twofiles
it seems to be obvious.
Thanks,
Jan
e1c1ceeab0af2ce94e37448fc6ee78275e39862e Mon Sep 17 00:00:00 2001
From: Jan Kratochvil <jan.kratochvil(a)redhat.com>
Date: Sun, 27 Feb 2011 08:35:50 +0100
Subject: [PATCH] Invalid free() / delete / delete[]
at 0x4C284F2: realloc (vg_replace_malloc.c:525)
by 0x41CACB: xrealloc (xmalloc.c:77)
by 0x40DB4A: notice_listptr (readelf.c:4382)
by 0x411CF0: attr_callback (readelf.c:5544)
by 0x4E438B8: dwarf_getattrs (dwarf_getattrs.c:124)
by 0x412641: print_debug_units (readelf.c:5795)
by 0x41287F: print_debug_info_section (readelf.c:5842)
by 0x41651B: print_debug (readelf.c:7033)
by 0x404910: process_elf_file (readelf.c:704)
by 0x4041EE: process_dwflmod (readelf.c:532)
by 0x4E52B17: dwfl_getmodules (dwfl_getmodules.c:103)
by 0x404480: process_file (readelf.c:602)
Address 0x60a0f10 is 0 bytes inside a block of size 1,024 free'd
at 0x4C2756E: free (vg_replace_malloc.c:366)
by 0x40DA9B: reset_listptr (readelf.c:4365)
by 0x41656D: print_debug (readelf.c:7040)
by 0x404910: process_elf_file (readelf.c:704)
by 0x4041EE: process_dwflmod (readelf.c:532)
by 0x4E52B17: dwfl_getmodules (dwfl_getmodules.c:103)
by 0x404480: process_file (readelf.c:602)
by 0x403774: main (readelf.c:277)
src/
2011-02-27 Jan Kratochvil <jan.kratochvil(a)redhat.com>
* readelf.c (reset_listptr): Clear TABLE->TABLE.
tests/
2011-02-27 Jan Kratochvil <jan.kratochvil(a)redhat.com>
* Makefile.am (TESTS): Add run-readelf-twofiles.sh.
* run-readelf-twofiles.sh: New file.
---
src/readelf.c | 1 +
tests/Makefile.am | 2 +-
tests/run-readelf-twofiles.sh | 32 ++++++++++++++++++++++++++++++++
3 files changed, 34 insertions(+), 1 deletions(-)
create mode 100755 tests/run-readelf-twofiles.sh
diff --git a/src/readelf.c b/src/readelf.c
index b798b5a..956d8bd 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -4363,6 +4363,7 @@ static void
reset_listptr (struct listptr_table *table)
{
free (table->table);
+ table->table = NULL;
table->n = table->alloc = 0;
}
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 3c37cb3..d0e4246 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -76,7 +76,7 @@ TESTS = run-arextract.sh run-arsymtest.sh newfile test-nlist \
run-addrscopes.sh run-strings-test.sh run-funcscopes.sh \
run-find-prologues.sh run-allregs.sh \
run-readelf-test1.sh run-readelf-test2.sh run-readelf-test3.sh \
- run-readelf-test4.sh \
+ run-readelf-test4.sh run-readelf-twofiles.sh \
run-native-test.sh run-bug1-test.sh \
dwfl-bug-addr-overflow run-addrname-test.sh \
dwfl-bug-fd-leak dwfl-bug-report \
diff --git a/tests/run-readelf-twofiles.sh b/tests/run-readelf-twofiles.sh
new file mode 100755
index 0000000..4255587
--- /dev/null
+++ b/tests/run-readelf-twofiles.sh
@@ -0,0 +1,32 @@
+#! /bin/sh
+# Copyright (C) 2011 Red Hat, Inc.
+# This file is part of Red Hat elfutils.
+#
+# Red Hat elfutils 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; version 2 of the License.
+#
+# Red Hat 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 Red Hat elfutils; if not, write to the Free Software Foundation,
+# Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301 USA.
+#
+# Red Hat elfutils is an included package of the Open Invention Network.
+# An included package of the Open Invention Network is a package for which
+# Open Invention Network licensees cross-license their patents. No patent
+# license is granted, either expressly or impliedly, by designation as an
+# included package. Should you wish to participate in the Open Invention
+# Network licensing program, please visit www.openinventionnetwork.com
+# <http://www.openinventionnetwork.com>.
+
+. $srcdir/test-subr.sh
+
+testfiles testfile14
+
+testrun >/dev/null ../src/readelf -w testfile14 testfile14
+
+exit 0
--
1.7.4.1
12 years, 7 months
--enable-tests-rpath
by Mark Wielaard
Hi,
Just noticed I should probably have build with configure
--enable-tests-rpath. I only noticed today on the dwarf branch because
of the new ELFUTILS_PRIVATE_TMP.
Why isn't this the default?
Also the spelling of BUILD_RPATH/BUILT_RPATH isn't consistent in
tests/Makefile.am. I assume it should always be the first.
Is the attached patch OK?
Thanks,
Mark
12 years, 7 months
Using __libdw_formptr from C++
by Petr Machata
Hi there,
this is about https://bugzilla.redhat.com/show_bug.cgi?id=679815 that
Mark filed this week. I fixed on dwarf branch a couple bugs that
dwarflint was hitting, but there's one outstanding issue that I don't
want to commit to dwarf.
The thing is that in range_list::const_iterator we now use
dwarf_formudata. dwarf_formudata doesn't handle DW_FORM_sec_offset, and
I'm not sure that it should. The function that does handle them is
__libdw_formptr and indeed where the iterators use dwarf_formudata, one
finds a comment in the spirit of "xxx __libdw_formptr". So I rewrote
the code to use that instead, just to find at link time that the
function obviously is not public, and since we compile the C++ bits
separately through an .a archive, this bites us.
Well the logic is written now, and seems to work, so I've committed it,
just separately to pmachata/range_list_iterator_formptr. The hope is
that it will be easy enough to adjust the patch to whatever the final
solution might be. I don't suppose that bundling C++ bits with C bits
in the single DSO ever was in plan, so some sort of publishing useful,
not-quite-ABI bits from libdw.so will be necessary.
Thanks,
PM
12 years, 7 months
readelf vs dwarfstrings
by Mark Wielaard
Hi,
Some time ago on the dwarf branch the the dwarf string functions were
split out of readelf.c into dwarfstrings.c plus dwarfstrings.h so they
could easily be reused in other elfutils programs. Over time they
diverged a bit.
Attached is a patch to merge the changes from the master readelf.c
version to the dwarf dwarfstrings.c version. Another to merge the
changes from the dwarfstrings.c version to the readelf.c master version.
To prevent further divergence and make merging changes easier there is
also a patch to make the same readelf/dwarfstrings for master.
Would it be OK to apply these patches to the branch and/or master?
Thanks,
Mark
12 years, 7 months
tests/dwarf_edit_output vs dwarf_path_finder changes
by Mark Wielaard
Hi,
The new dwarf_path_finder changes don't like the way
tests/dwarf_edit_output uses the dwarf_ref_tracker to compare two test
dies. It generates some assertion failures.
$ tests/dwarf_edit_output
dwarf_edit_output: /home/mark/src/elfutils/libdw/c++/dwarf_tracker:306:
void elfutils::dwarf_path_finder<dw>::prime_path_to(const typename
dw::debug_info_entry&) [with dw = elfutils::dwarf_edit, typename
dw::debug_info_entry = elfutils::dwarf_edit::debug_info_entry]:
Assertion `_m_owner != __null' failed.
Aborted
This is most likely because it jumps to comparing the dies directly:
// Customized compare function. Takes two debug_info_entries and
// the dwarf structure they reside in. The first
debug_info_entry is
// assumed to reside in the first cu, the debug_info_entry in
the
// next cu.
bool
compare_dies (const dwarf_edit::debug_info_entry &a,
const dwarf_edit::debug_info_entry &b,
const dwarf_edit &dw)
{
dwarf_edit::compile_units_type::const_iterator cu1, cu2;
cu1 = dw.compile_units ().begin ();
cu2 = dw.compile_units ().begin ();
cu2++;
cmp_tracker::walk in (&this->_m_tracker, cu1, cu2);
in.jump (a, b);
return equals (a, b);
}
in.jump (a, b) triggers the asserts.
What is the correct way to do this?
Thanks,
Mark
12 years, 7 months
circular reference chains with DW_AT_containing_type
by Mark Wielaard
Hi,
I finally fully implemented the dwarf_output hashing algorithm, with a
local hash that depends only on non-reference attributes and the local
hash of the children of a die, and a reference hash that depends on the
local hashes of all reference attributes (recursively) of the die.
This does reduce the number of attr_set hash collisions a lot.
Unfortunately there are circular reference chains for some larger c++
programs. That was unexpected. Here is an example running dwarfcmp-test
on itself:
$ src/dwarfcmp-test -T src/dwarfcmp-test src/dwarfcmp-test
Reference chain cycle detected
offset=[0x2898c8] already on the reference chain stack
offset=[0x2889cb] subprogram
offset=[0x2959bd] reference_type
offset=[0x289b7c] structure_type
offset=[0x2898c8] structure_type
Aborted (core dumped)
And offset=[0x2898c8] is:
<structure_type offset=[0x2898c8]
specification=[0x28889b]
byte_size=0x8
decl_file="...libdw/c++/dwarf_data" decl_line=761
containing_type=[0x2898c8]>
Is this valid dwarf? Can a structure_type have a containing_type? A
strict reading of the dwarf4 spec seems to imply only a
DW_TAG_ptr_to_member_type may have a DW_AT_containing_type.
Not yet sure how to handle this case. The original algorithm depended on
the reference chain not being cyclic on its own (when ignoring
children). Obviously that isn't true in at least this case.
Code, plus cycle detection is on mjw/dwarf_output branch.
Cheers,
Mark
12 years, 7 months
dwarf_path_finder changes
by Roland McGrath
I've revamped the dwarf_path_finder code a bit (f8af969). I think the new
way it works is what Petr was suggesting earlier, when I wasn't following
what he was saying.
Now all searches on any derived object go to the "master" object. If
walk-ahead is required to find a DIE not yet seen, then it uses a single
walkahead object that spins ahead until it steps over the target DIE.
That walkahead object stays alive until the master object catches up to its
position in the walk, at which point it's deleted.
This should have the effect of only ever doing at most two complete walks
(not including proper side-walks in subcomparators, of course).
There's a simple optimization I didn't do, for a case that the old code
tried to optimize. That's for a case where a forward reference is to the
immediate next sibling. This is fairly common, e.g. <pointer_type type=/>
with the referent type being the immediate next sibling DIE in the CU. As
things now stand, this will create the walkahead clone that takes only one
step to win, then delete it on the next step of the main walk. When that
case comes up a lot, it could be a lot of churn on new/construct and
destroy/delete work for that clone. We could short-circuit that if the
new/delete overhead there seems significant.
I tested this on run-dwarfcmp-self and saw no regressions in the C cases.
I didn't wait for the C++ cases to finish, since it will still take a week.
I didn't actually measure the performance at all, though algorithmically it
should be better than before. (And the new code is actually a bit simpler
than the old, too.)
Petr, I'd appreciate it if you could do that performance comparison and
also re-do your instrumentation checks to ensure that the redundant walks
for searches really are bounded the way I think they should be now.
Whenever you do some instrumentation, please post (directly or via private
git branches) the code hacks you write, as well as describing completely
the test data and test scenarios you do, so that other people can repeat
the experiments easily. (If cases and results get hairy, it may be
worthwhile to put up actual binaries you used as test data, in case
everybody's build doesn't come out the same.)
Thanks,
Roland
12 years, 7 months
more dwarf_output stumbling
by Mark Wielaard
Hi,
A am a bit stumbling forward. Current progress pushed on the mjw/dwarf_output branch. Not stuck, just fighting and backtracking on some ideas. So best to give a little status update in case more course corrections are necessary.
The local hash for dies is now no longer (re)calculated in the dwarf_output::debug_info_entry hierarchy, but stored in the die_info. This is calculated when the pending_entry is finalized. This also puts all calculations in one place.
This is slightly too late (or too early depending on how you look at it). Although at that time all children of the die have already been finalized, not all (circular) references have. But we are putting the candidate into the collector anyway.
At first I thought that I just needed to get rid of the attributes_type cache, since that is where the reference chasing ends up. That would get rid of some complexity, but doesn't actually solve the actual algorithmic problem. Then the reference chasing ends up in the hash calculation of the debug_info_entry itself. So I abandoned that idea for now.
Instead what we need is to make sure that the local hash is calculated for every entry die before we try to put the attributes_type (or debug_info_entry) into the collector, since the full hash is based on the reference attributes, which can point to any die, and so need their local hash set.
So I am now looking at only calculating the local hash when we currently finalize, but don't put the items directly in the cache, but only after we scanned over the whole CU. That way when we do put the item in the cache, we can calculate the full hash because we can be sure that all dies that the reference attributes (recursively) points at have their local hash set.
Cheers,
Mark
12 years, 7 months
0.152 release
by Roland McGrath
I think we're ready for an 0.152 release now. Since I wrote before,
I made a libdwfl change for finding kernel files, and added two new
switches to elfcmp. Those new things have marginal testing, but little
risk of regression. I've freshly repeated test builds and all looks good.
Don't forget the important release checklist items:
1. autoreconf -f -i with new-enough autotools (Fedora 14 recommended)
2. make distcheck
3. git tag -s with signature
4. git push the tag
5. upload .tar.bz2 and .tar.bz2.sig files
Thanks,
Roland
12 years, 7 months
next release
by Roland McGrath
I'd like to have the 0.152 release soon, perhaps early next week at the
latest. I was hoping to get some feedback from Jakub about the prelink
test case coverage, but he hasn't gotten back to me. We have test cases
for all the regressions we've seen, plus the things I know to worry about,
anyway. (But that was true last time, and we had one more regression.)
There are few changes since 0.151, but they are needed soon.
The libdwfl vs prelink fixes address yet another regression (hopefully the
last already!).
The other changes are warning nits that broke the build with gcc-4.6, which
we now have for Fedora 15.
I also fiddled some things that got warnings from autoconf, though those
were non-fatal. Since I changed those, it may well be necessary to ensure
that the dist gets built with recent enough autoconf/automake. (And, as
always, be sure --enable-maintainer-mode was in your config, or else run
autoreconf -f -i by hand before make distcheck.) Fedora 14's versions are
what I've used and those are fine (2.66, 1.11.1).
I've just completed some test builds for Fedora/RHEL and those look fine.
I don't know of anything else we are waiting for (modulo hearing from Jakub
about more prelink test cases that would be wise to have).
Thanks,
Roland
12 years, 7 months