[PATCH] libdw: Search for the last matching address with dwarf_getsrc_die.

Josh Stone jistone at redhat.com
Fri Dec 26 21:12:14 UTC 2014


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 <mjw at redhat.com>
> ---
>  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  <mjw at redhat.com>
> +
> +	* 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  <drepper at gmail.com>
>  
>  	* 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 <drepper at redhat.com>, 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
> 



More information about the elfutils-devel mailing list