Re: PATCH: dwarf_highpc: Handle DW_AT_high_pc being a constant offset from DW_AT_low_pc.

[Mark Wielaard <mjw at redhat dot com>]

On Fri, 2012-04-27 at 14:44 -0700, Roland McGrath wrote:
> The functionality change is fine.  Needs a NEWS item.

OK, added.

> Don't make the same dwarf_attr call twice.  If it fails,
> return immediately.

Done.

> I'm not sure you should use dwarf_formaddr for a case that might not work,
> since we get an extra __libdw_seterrno call when the user's call succeeds.
> I don't know off hand if we have clear precedent about that issue.  But
> here it's easy enough just to check attr->form for DW_FORM_addr.

Done.

> If dwarf_formudata fails I don't think we should return with
> DWARF_E_NO_CONSTANT, since the user asked for an address, not a constant.
> Probably simplest just to add a
> 	__libdw_seterrno (DWARF_E_NO_ADDR);
> in that error case.

Also done.

Rewritten patch attached.

> Also, please look at the relocate branch for how this can be handled there,
> so the branch (including new interfaces) doesn't regress from trunk in
> subtle ways.  I think the only change needed is dwarf_ranges_relocatable,
> but that seems like it could be nontrivial.

I'll take a look. This actually came from studying the relocation branch
and realizing a lot of relocations came in low_pc/high_pc pairs.

Thanks,

Mark

commit 6ce3eae100cdd01f0a0fa17b95d051fa81b84abf
Author: Mark Wielaard <mjw@redhat.com>
Date:   Fri Apr 27 13:00:50 2012 +0200

    dwarf_highpc: Handle DW_AT_high_pc being a constant offset from DW_AT_low_pc.

diff --git a/NEWS b/NEWS
index aa660a8..cb39785 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,7 @@
+Version 0.154
+
+libdw: dwarf_highpc function now handles DAWRF 4 DW_AT_high_pc constant form.
+
 Version 0.153
 
 libdw: Support reading .zdebug_* DWARF sections compressed via zlib.
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index f96c0d1..3ff83e4 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,8 @@
+2012-04-27  Mark Wielaard  <mjw@redhat.com>
+
+	* libdw/dwarf_highpc.c (dwarf_highpc): Handle DW_AT_high_pc being
+	a constant offset from DW_AT_low_pc.
+
 2012-03-19  Tom Tromey  <tromey@redhat.com>
 
 	* libdw_findcu.c (findcu_cb): Move earlier.
diff --git a/libdw/dwarf_highpc.c b/libdw/dwarf_highpc.c
index c88e072..1ca8e31 100644
--- a/libdw/dwarf_highpc.c
+++ b/libdw/dwarf_highpc.c
@@ -1,5 +1,5 @@
 /* Return high PC attribute of DIE.
-   Copyright (C) 2003, 2005 Red Hat, Inc.
+   Copyright (C) 2003, 2005, 2012 Red Hat, Inc.
    This file is part of Red Hat elfutils.
    Written by Ulrich Drepper <drepper@redhat.com>, 2003.
 
@@ -61,10 +61,32 @@ dwarf_highpc (die, return_addr)
      Dwarf_Die *die;
      Dwarf_Addr *return_addr;
 {
-  Dwarf_Attribute attr_mem;
+  Dwarf_Attribute attr_high_mem;
+  Dwarf_Attribute *attr_high = INTUSE(dwarf_attr) (die, DW_AT_high_pc,
+						   &attr_high_mem);
+  if (! attr_high)
+    return -1;
 
-  return INTUSE(dwarf_formaddr) (INTUSE(dwarf_attr) (die, DW_AT_high_pc,
-						     &attr_mem),
-				 return_addr);
+  if (attr_high->form == DW_FORM_addr)
+    return INTUSE(dwarf_formaddr) (attr_high, return_addr);
+  else
+    {
+      /* DWARF 4 allows high_pc to be a constant offset from low_pc. */
+      Dwarf_Attribute attr_low_mem;
+
+      if (INTUSE(dwarf_formaddr) (INTUSE(dwarf_attr) (die, DW_AT_low_pc,
+						      &attr_low_mem),
+				  return_addr) == 0)
+	{
+	  Dwarf_Word uval;
+	  if (INTUSE(dwarf_formudata) (attr_high, &uval) == 0)
+	    {
+	      *return_addr += uval;
+	      return 0;
+	    }
+	  __libdw_seterrno (DWARF_E_NO_ADDR);
+	}
+      return -1;
+    }
 }
 INTDEF(dwarf_highpc)
diff --git a/tests/ChangeLog b/tests/ChangeLog
index ffe61d5..abc7340 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,11 @@
+2012-04-27  Mark Wielaard  <mjw@redhat.com>
+
+	* Makefile.am (TESTS): Add run-low_high_pc.sh
+	(EXTRA_DIST): Add run-low_high_pc.sh and testfile_low_high_pc.bz2
+	(noinst_PROGRAMS): Add low_high_pc.
+	(low_high_pc_LDADD): New variable.
+	* low_high_pc.c: New test.
+
 2012-04-26  Mark Wielaard  <mjw@redhat.com>
 
 	* Makefile.am (EXTRA_DIST): Remove run-show-ciefde.sh.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 835cc7a..61247e4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -58,7 +58,7 @@ noinst_PROGRAMS = arextract arsymtest newfile saridx scnnames sectiondump \
 		  dwfl-addr-sect dwfl-bug-report early-offscn \
 		  dwfl-bug-getmodules dwarf-getmacros addrcfi \
 		  test-flag-nobits dwarf-getstring rerequest_tag \
-		  alldts md5-sha1-test typeiter
+		  alldts md5-sha1-test typeiter low_high_pc
 asm_TESTS = asm-tst1 asm-tst2 asm-tst3 asm-tst4 asm-tst5 \
 	    asm-tst6 asm-tst7 asm-tst8 asm-tst9
 
@@ -87,7 +87,7 @@ TESTS = run-arextract.sh run-arsymtest.sh newfile test-nlist \
 	run-early-offscn.sh run-dwarf-getmacros.sh \
 	run-test-flag-nobits.sh run-prelink-addr-test.sh \
 	run-dwarf-getstring.sh run-rerequest_tag.sh run-typeiter.sh \
-	run-readelf-d.sh run-unstrip-n.sh
+	run-readelf-d.sh run-unstrip-n.sh run-low_high_pc.sh
 
 if !STANDALONE
 noinst_PROGRAMS += msg_tst md5-sha1-test
@@ -164,7 +164,8 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh \
 	     testfile56.bz2 testfile57.bz2 testfile58.bz2 \
 	     run-typeiter.sh testfile59.bz2 \
 	     run-readelf-d.sh testlib_dynseg.so.bz2 \
-	     run-unstrip-n.sh testcore-rtlib.bz2
+	     run-unstrip-n.sh testcore-rtlib.bz2 \
+	     run-low_high_pc.sh testfile_low_high_pc.bz2
 
 installed_TESTS_ENVIRONMENT = libdir=$(DESTDIR)$(libdir) \
 			      bindir=$(DESTDIR)$(bindir) \
@@ -262,6 +263,7 @@ rerequest_tag_LDADD = $(libdw) $(libmudflap)
 alldts_LDADD = $(libebl) $(libelf) $(libmudflap)
 md5_sha1_test_LDADD = $(libeu)
 typeiter_LDADD = $(libdw) $(libelf) $(libmudflap)
+low_high_pc_LDADD = $(libdw) $(libelf) $(libmudflap)
 
 if GCOV
 check: check-am coverage
diff --git a/tests/low_high_pc.c b/tests/low_high_pc.c
new file mode 100644
index 0000000..3054046
--- /dev/null
+++ b/tests/low_high_pc.c
@@ -0,0 +1,114 @@
+/* Test program for dwarf_lowpc and dwarf_highpc
+   Copyright (C) 2012 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>.  */
+
+#include <config.h>
+#include <assert.h>
+#include <inttypes.h>
+#include ELFUTILS_HEADER(dwfl)
+#include <dwarf.h>
+#include <argp.h>
+#include <stdio.h>
+#include <stdio_ext.h>
+#include <locale.h>
+#include <stdlib.h>
+#include <error.h>
+#include <string.h>
+#include <fnmatch.h>
+
+struct args
+{
+  Dwfl *dwfl;
+  Dwarf_Die *cu;
+  Dwarf_Addr dwbias;
+  char **argv;
+  const char *file;
+};
+
+static struct args *args;
+
+static void
+fail(Dwarf_Off off, const char *name, const char *msg)
+{
+  printf("%s: [%lx] '%s' %s\n", args->file, off, name, msg);
+  exit(-1);
+}
+
+static int
+handle_die (Dwarf_Die *die, void *arg)
+{
+  args = arg;
+  Dwarf_Off off = dwarf_dieoffset (die);
+
+  const char *name = dwarf_diename (die);
+  if (name == NULL)
+    fail (off, "<no name>", "die without a name");
+
+  Dwarf_Addr lowpc = 0;
+  Dwarf_Addr highpc = 0;
+  if (dwarf_lowpc (die, &lowpc) != 0 && dwarf_hasattr (die, DW_AT_low_pc))
+    fail (off, name, "has DW_AT_low_pc but dwarf_lowpc fails");
+  if (dwarf_highpc (die, &highpc) != 0 && dwarf_hasattr (die, DW_AT_high_pc))
+    fail (off, name, "has DW_AT_high_pc but dwarf_highpc fails");
+
+  if (dwarf_hasattr (die, DW_AT_low_pc)
+      && dwarf_hasattr (die, DW_AT_high_pc)
+      && highpc <= lowpc)
+    {
+      printf("lowpc: %lx, highpc: %lx\n", lowpc, highpc);
+      fail (off, name, "highpc <= lowpc");
+    }
+
+  return 0;
+}
+
+
+int
+main (int argc, char *argv[])
+{
+  int remaining;
+
+  /* Set locale.  */
+  (void) setlocale (LC_ALL, "");
+
+  struct args a = { .dwfl = NULL, .cu = NULL };
+
+  (void) argp_parse (dwfl_standard_argp (), argc, argv, 0, &remaining,
+		     &a.dwfl);
+  assert (a.dwfl != NULL);
+  a.argv = &argv[remaining];
+
+  int result = 0;
+
+  while ((a.cu = dwfl_nextcu (a.dwfl, a.cu, &a.dwbias)) != NULL)
+    {
+      a.file = dwarf_diename (a.cu);
+      handle_die (a.cu, &a);
+      dwarf_getfuncs (a.cu, &handle_die, &a, 0);
+    }
+
+  dwfl_end (a.dwfl);
+
+  return result;
+}
diff --git a/tests/run-low_high_pc.sh b/tests/run-low_high_pc.sh
new file mode 100755
index 0000000..8d891f1
--- /dev/null
+++ b/tests/run-low_high_pc.sh
@@ -0,0 +1,42 @@
+#! /bin/sh
+# Copyright (C) 2005 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
+
+# int
+# main (int argc, char **argv)
+# {
+#   return 0;
+# }
+# gcc -g -o main main.c
+testfiles testfile_low_high_pc
+
+testrun ./low_high_pc -e ./testfile_low_high_pc
+testrun ./low_high_pc -e ./low_high_pc
+testrun ./low_high_pc -e ../src/strip
+testrun ./low_high_pc -e ../src/strip.o
+testrun ./low_high_pc -e ../libelf/libelf.so
+
+exit 0
diff --git a/tests/testfile_low_high_pc.bz2 b/tests/testfile_low_high_pc.bz2
new file mode 100755
index 0000000..f20814a
Binary files /dev/null and b/tests/testfile_low_high_pc.bz2 differ