On 12/24/2014 05:27 AM, Mark Wielaard wrote: > In commit 7d9b5a dwfl_module_getsrc was changed so that it returns the last > line record <= addr, rather than returning immediately on a match. This > changes dwarf_getsrc_die to do the same. And it adds a new test that checks > this by comparing against the same results from eu-addr2line (which uses > dwfl_module_getsrc) using dwarf_addrdie and dwarf_getsrc_die instead.
Sounds like a good idea. > Signed-off-by: Mark Wielaard <[email protected]> > --- > libdw/ChangeLog | 6 ++++ > libdw/dwarf_getsrc_die.c | 33 +++++++++------------- > tests/Makefile.am | 10 +++++-- > tests/getsrc_die.c | 72 > ++++++++++++++++++++++++++++++++++++++++++++++++ > tests/run-getsrc-die.sh | 51 ++++++++++++++++++++++++++++++++++ > 5 files changed, 149 insertions(+), 23 deletions(-) > create mode 100644 tests/getsrc_die.c > create mode 100755 tests/run-getsrc-die.sh > > diff --git a/libdw/ChangeLog b/libdw/ChangeLog > index abc2d71..bf64c2e 100644 > --- a/libdw/ChangeLog > +++ b/libdw/ChangeLog > @@ -1,3 +1,9 @@ > +2014-12-24 Mark Wielaard <[email protected]> > + > + * dwarf_getsrc_die.c (dwarf_getsrc_die): Return the last line record > + smaller than or equal to addr, rather than returning immediately on > + a match. > + > 2014-12-18 Ulrich Drepper <[email protected]> > > * Makefile.am: Suppress output of textrel_check command. > diff --git a/libdw/dwarf_getsrc_die.c b/libdw/dwarf_getsrc_die.c > index 1914cdf..fe06ca4 100644 > --- a/libdw/dwarf_getsrc_die.c > +++ b/libdw/dwarf_getsrc_die.c > @@ -1,5 +1,5 @@ > /* Find line information for address. > - Copyright (C) 2004, 2005 Red Hat, Inc. > + Copyright (C) 2004, 2005, 2014 Red Hat, Inc. > This file is part of elfutils. > Written by Ulrich Drepper <[email protected]>, 2004. > > @@ -45,33 +45,26 @@ dwarf_getsrc_die (Dwarf_Die *cudie, Dwarf_Addr addr) > return NULL; > > /* The lines are sorted by address, so we can use binary search. */ > - size_t l = 0, u = nlines; > + size_t l = 0, u = nlines - 1; > while (l < u) What if nlines == 0? > { > - size_t idx = (l + u) / 2; > - if (addr < lines->info[idx].addr) > - u = idx; > - else if (addr > lines->info[idx].addr || lines->info[idx].end_sequence) > - l = idx + 1; > + size_t idx = u - (u -l) / 2; [nit] missing a space: "(u - l)" > + Dwarf_Line *line = &lines->info[idx]; > + if (addr < line->addr) > + u = idx - 1; > else > - return &lines->info[idx]; > + l = idx; > } > > + /* This is guaranteed for us by libdw read_srclines. */ > if (nlines > 0) > assert (lines->info[nlines - 1].end_sequence); In case your answer above was that nlines is never 0, then this "if" is redundant. > > - /* If none were equal, the closest one below is what we want. We > - never want the last one, because it's the end-sequence marker > - with an address at the high bound of the CU's code. If the debug > - information is faulty and no end-sequence marker is present, we > - still ignore it. */ > - if (u > 0 && u < nlines && addr > lines->info[u - 1].addr) > - { > - while (lines->info[u - 1].end_sequence && u > 0) > - --u; > - if (u > 0) > - return &lines->info[u - 1]; > - } > + /* The last line which is less than or equal to addr is what we want, > + except with an end_sequence which can only be strictly equal. */ > + Dwarf_Line *line = &lines->info[l]; > + if (line->addr == addr || (! line->end_sequence && line->addr < addr)) > + return &lines->info[l]; BTW, it seems like the end ought to be exclusive, i.e. [start,end). I only wrote my commit this way because some existing tests depended on end-inclusive behavior, and I forgot to argue it. :) Thoughts? > > __libdw_seterrno (DWARF_E_ADDR_OUTOFRANGE); > return NULL; > diff --git a/tests/Makefile.am b/tests/Makefile.am > index 303f783..c364cc9 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -50,7 +50,8 @@ check_PROGRAMS = arextract arsymtest newfile saridx > scnnames sectiondump \ > test-elf_cntl_gelf_getshdr dwflsyms dwfllines \ > dwfl-report-elf-align varlocs backtrace backtrace-child \ > backtrace-data backtrace-dwarf debuglink debugaltlink \ > - buildid deleted deleted-lib.so aggregate_size vdsosyms > + buildid deleted deleted-lib.so aggregate_size vdsosyms \ > + getsrc_die > > asm_TESTS = asm-tst1 asm-tst2 asm-tst3 asm-tst4 asm-tst5 \ > asm-tst6 asm-tst7 asm-tst8 asm-tst9 > @@ -111,7 +112,8 @@ TESTS = run-arextract.sh run-arsymtest.sh newfile > test-nlist \ > run-backtrace-core-aarch64.sh \ > run-backtrace-demangle.sh run-stack-d-test.sh run-stack-i-test.sh \ > run-readelf-dwz-multi.sh run-allfcts-multi.sh run-deleted.sh \ > - run-linkmap-cut.sh run-aggregate-size.sh vdsosyms run-readelf-A.sh > + run-linkmap-cut.sh run-aggregate-size.sh vdsosyms run-readelf-A.sh \ > + run-getsrc-die.sh > > if !BIARCH > export ELFUTILS_DISABLE_BIARCH = 1 > @@ -280,7 +282,8 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh \ > linkmap-cut.bz2 linkmap-cut.core.bz2 \ > run-aggregate-size.sh testfile-sizes1.o.bz2 testfile-sizes2.o.bz2 \ > testfile-sizes3.o.bz2 \ > - run-readelf-A.sh testfileppc32attrs.o.bz2 > + run-readelf-A.sh testfileppc32attrs.o.bz2 \ > + run-getsrc-die.sh > > if USE_VALGRIND > valgrind_cmd='valgrind -q --error-exitcode=1 --run-libc-freeres=no' > @@ -420,6 +423,7 @@ deleted_lib_so_LDFLAGS = -shared -rdynamic > deleted_lib_so_CFLAGS = -fPIC -fasynchronous-unwind-tables > aggregate_size_LDADD = $(libdw) $(libelf) > vdsosyms_LDADD = $(libdw) $(libelf) > +getsrc_die_LDADD = $(libdw) $(libelf) > > if GCOV > check: check-am coverage > diff --git a/tests/getsrc_die.c b/tests/getsrc_die.c > new file mode 100644 > index 0000000..055aede > --- /dev/null > +++ b/tests/getsrc_die.c > @@ -0,0 +1,72 @@ > +/* Copyright (C) 2014 Red Hat, Inc. > + 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/>. */ > + > +#ifdef HAVE_CONFIG_H > +# include <config.h> > +#endif > + > +#include <errno.h> > +#include <error.h> > +#include <fcntl.h> > +#include <inttypes.h> > +#include <libelf.h> > +#include ELFUTILS_HEADER(dw) > +#include <stdio.h> > +#include <stdlib.h> > +#include <unistd.h> > + > + > +int > +main (int argc, char *argv[]) > +{ > + /* file addr+ */ > + int fd = open (argv[1], O_RDONLY); > + Dwarf *dbg = dwarf_begin (fd, DWARF_C_READ); > + if (dbg == NULL) > + error (-1, 0, "dwarf_begin (%s): %s\n", argv[1], dwarf_errmsg (-1)); > + > + for (int i = 2; i < argc; i++) > + { > + Dwarf_Addr addr; > + char *endptr; > + Dwarf_Die cudie; > + Dwarf_Line *line; > + > + errno = 0; > + addr = strtoull (argv[i], &endptr, 16); > + if (errno != 0) > + error (-1, errno, "Cannot parrse '%s'", argv[1]); > + > + if (dwarf_addrdie (dbg, addr, &cudie) == NULL) > + error (-1, 0, "dwarf_addrdie (%s): %s", argv[i], dwarf_errmsg (-1)); > + > + line = dwarf_getsrc_die (&cudie, addr); > + if (line == NULL) > + error (-1, 0, "dwarf_getsrc_die (%s): %s", argv[i], dwarf_errmsg (-1)); > + > + const char *f = dwarf_linesrc (line, NULL, NULL); > + int l; > + if (dwarf_lineno (line, &l) != 0) > + l = 0; > + > + printf ("%s:%d\n", f ?: "???", l); > + } > + > + dwarf_end (dbg); > + close (fd); > + > + return 0; > +} > diff --git a/tests/run-getsrc-die.sh b/tests/run-getsrc-die.sh > new file mode 100755 > index 0000000..4da16e7 > --- /dev/null > +++ b/tests/run-getsrc-die.sh > @@ -0,0 +1,51 @@ > +#! /bin/sh > +# Copyright (C) 2014 Red Hat, Inc. > +# 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/>. > + > +. $srcdir/test-subr.sh > + > +# See run-addr2line-test.sh run-addr2line-i-test.sh > run-addr2line-i-lex-test.sh > +# Output/files/lines matched should equal what is done through addr2line > +# which uses dwfl_module_getsrc. This test uses dwarf_addrdie and > +# dwarf_getsrc_die > +testfiles testfile testfile-inlines testfile-lex-inlines > + > +testrun_compare ${abs_top_builddir}/tests/getsrc_die testfile 0x08048468 > 0x0804845c <<\EOF > +/home/drepper/gnu/new-bu/build/ttt/f.c:3 > +/home/drepper/gnu/new-bu/build/ttt/b.c:4 > +EOF > + > +testrun_compare ${abs_top_builddir}/tests/getsrc_die testfile-inlines > 0x00000000000005a0 0x00000000000005a1 0x00000000000005b0 0x00000000000005b1 > 0x00000000000005c0 0x00000000000005d0 0x00000000000005e0 0x00000000000005e1 > 0x00000000000005f1 0x00000000000005f2 <<\EOF > +/tmp/x.cpp:5 > +/tmp/x.cpp:6 > +/tmp/x.cpp:10 > +/tmp/x.cpp:11 > +/tmp/x.cpp:5 > +/tmp/x.cpp:10 > +/tmp/x.cpp:5 > +/tmp/x.cpp:10 > +/tmp/x.cpp:10 > +/tmp/x.cpp:5 > +EOF > + > +testrun_compare ${abs_top_builddir}/tests/getsrc_die testfile-lex-inlines > 0x0000000000000680 0x0000000000000681 0x0000000000000690 0x0000000000000691 > <<\EOF > +/tmp/x.cpp:5 > +/tmp/x.cpp:5 > +/tmp/x.cpp:5 > +/tmp/x.cpp:5 > +EOF > + > +exit 0 >
