Re: [PATCH 3/3] gold: Handle DW_AT_high_pc as offset from DW_AT_low_pc in gdb-index.cc.

[Mark Wielaard <mjw at redhat dot com>]

On Fri, 2012-04-27 at 11:21 -0700, Cary Coutant wrote:
> >   off_t low_pc = die->ref_attribute(elfcpp::DW_AT_low_pc, &shndx);
> > -  off_t high_pc = die->ref_attribute(elfcpp::DW_AT_high_pc, &shndx2);
> > -  if ((low_pc != 0 || high_pc != 0) && low_pc != -1 && high_pc != -1)
> > +  off_t high_pc = die->address_attribute(elfcpp::DW_AT_high_pc, &shndx2);
> > +  if (high_pc == -1)
> > +    {
> > +      high_pc = die->uint_attribute(elfcpp::DW_AT_high_pc);
> > +      high_pc += low_pc;
> > +      shndx2 = shndx;
> > +    }
> > +  if ((low_pc != 0 || high_pc != 0) && low_pc != -1)
> >     {
> >       if (shndx != shndx2)
> >         {
> 
> You should change the first call for low_pc to use
> die->address_attribute() as well.

Yes, of course, good point.
Adjusted patch attached (with that line changed and not including
ChangeLog file and regenerated Makefile.in file in the patch).

2012-04-26  Mark Wielaard  <mjw@redhat.com>

    * dwarf_reader.cc (Dwarf_die::address_attribute): New function.
    * dwarf_reader.h (Dwarf_die::address_attribute): Likewise.
    * gdb-index.cc (Gdb_index_info_reader::record_cu_ranges): Handle
    DW_AT_high_pc as offset from DW_AT_low_pc.

    * testsuite/Makefile.am (gdb_index_test_3.sh): New test case.
    * testsuite/Makefile.in: Regenerate.
    * testsuite/gdb_index_test_3.c: New test source file.
    * testsuite/gdb_index_test_3.sh: New test source file.

Thanks,

Mark

diff --git a/gold/dwarf_reader.cc b/gold/dwarf_reader.cc
index eaf35bf..6245dc8 100644
--- a/gold/dwarf_reader.cc
+++ b/gold/dwarf_reader.cc
@@ -1051,6 +1051,17 @@ Dwarf_die::ref_attribute(unsigned int attr, unsigned int* shndx)
     }
 }
 
+off_t
+Dwarf_die::address_attribute(unsigned int attr, unsigned int* shndx)
+{
+  const Attribute_value* attr_val = this->attribute(attr);
+  if (attr_val == NULL || attr_val->form != elfcpp::DW_FORM_addr)
+    return -1;
+
+  *shndx = attr_val->aux.shndx;
+  return attr_val->val.refval;
+}
+
 // Return the offset of this DIE's first child.
 
 off_t
diff --git a/gold/dwarf_reader.h b/gold/dwarf_reader.h
index 0c3dab6..6baef66 100644
--- a/gold/dwarf_reader.h
+++ b/gold/dwarf_reader.h
@@ -550,6 +550,11 @@ class Dwarf_die
   ref_attribute(unsigned int attr,
 		unsigned int* shndx);
 
+  // Return the value of attribute ATTR as a address.
+  off_t
+  address_attribute(unsigned int attr,
+		    unsigned int* shndx);
+
   // Return the value of attribute ATTR as a flag.
   bool
   flag_attribute(unsigned int attr)
diff --git a/gold/gdb-index.cc b/gold/gdb-index.cc
index a6db505..6666988 100644
--- a/gold/gdb-index.cc
+++ b/gold/gdb-index.cc
@@ -823,9 +823,15 @@ Gdb_index_info_reader::record_cu_ranges(Dwarf_die* die)
       return;
     }
 
-  off_t low_pc = die->ref_attribute(elfcpp::DW_AT_low_pc, &shndx);
-  off_t high_pc = die->ref_attribute(elfcpp::DW_AT_high_pc, &shndx2);
-  if ((low_pc != 0 || high_pc != 0) && low_pc != -1 && high_pc != -1)
+  off_t low_pc = die->address_attribute(elfcpp::DW_AT_low_pc, &shndx);
+  off_t high_pc = die->address_attribute(elfcpp::DW_AT_high_pc, &shndx2);
+  if (high_pc == -1)
+    {
+      high_pc = die->uint_attribute(elfcpp::DW_AT_high_pc);
+      high_pc += low_pc;
+      shndx2 = shndx;
+    }
+  if ((low_pc != 0 || high_pc != 0) && low_pc != -1)
     {
       if (shndx != shndx2)
         {
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index d3c02e0..1c970a3 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -1992,6 +1992,18 @@ gdb_index_test_2.stdout: gdb_index_test_2
 
 endif HAVE_ZLIB
 
+# Another simple C test (DW_AT_high_pc encoding) for --gdb-index
+check_SCRIPTS += gdb_index_test_3.sh
+check_DATA += gdb_index_test_3.stdout
+MOSTLYCLEANFILES += gdb_index_test_3.stdout gdb_index_test_3
+gdb_index_test_3.o: gdb_index_test_3.c
+	$(COMPILE) -O0 -g -c -o $@ $<
+gdb_index_test_3: gdb_index_test_3.o gcctestdir/ld
+	$(LINK) -Bgcctestdir/ -Wl,--gdb-index,--fatal-warnings $<
+gdb_index_test_3.stdout: gdb_index_test_3
+	$(TEST_READELF) --debug-dump=gdb_index $< > $@
+
+
 # End-to-end incremental linking tests.
 # Incremental linking is currently supported only on the x86_64 target.
 
diff --git a/gold/testsuite/gdb_index_test_3.c b/gold/testsuite/gdb_index_test_3.c
new file mode 100644
index 0000000..df49261
--- /dev/null
+++ b/gold/testsuite/gdb_index_test_3.c
@@ -0,0 +1,39 @@
+// gdb_index_test.c -- a test case for the --gdb-index option.
+
+// Copyright 2012 Free Software Foundation, Inc.
+
+// This file is part of gold.
+
+// This program 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.
+
+// This program 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, write to the Free Software
+// Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+// MA 02110-1301, USA.
+
+// This source file is just a simple C source file that is mainly to
+// test the CU DW_AT_high_pc FORM encoding is handled correctly by the
+// DWARF scanner in gold.
+
+int check_int (int);
+int main (void);
+
+int j = 0;
+
+int
+check_int (int i)
+{ return i > 0; }
+
+int
+main()
+{
+  return check_int (0);
+}
diff --git a/gold/testsuite/gdb_index_test_3.sh b/gold/testsuite/gdb_index_test_3.sh
new file mode 100755
index 0000000..dd6ce7e
--- /dev/null
+++ b/gold/testsuite/gdb_index_test_3.sh
@@ -0,0 +1,49 @@
+#!/bin/sh
+
+# gdb_index_test_3.sh -- a test case for the --gdb-index option.
+
+# Copyright 2012 Free Software Foundation, Inc.
+# Written by Cary Coutant <ccoutant@google.com>.
+
+# This file is part of gold.
+
+# This program 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.
+
+# This program 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, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+check()
+{
+    if ! grep -q "$2" "$1"
+    then
+	echo "Did not find expected output:"
+	echo "   $2"
+	echo ""
+	echo "Actual error output below:"
+	cat "$1"
+	exit 1
+    fi
+}
+
+STDOUT=gdb_index_test_3.stdout
+
+check $STDOUT "^Version [45]"
+
+# Look for the symbols we know should be in the symbol table.
+
+check $STDOUT "^\[ *[0-9]*\] main: "
+check $STDOUT "^\[ *[0-9]*\] check_int: "
+check $STDOUT "^\[ *[0-9]*\] j: "
+check $STDOUT "^\[ *[0-9]*\] int: "
+
+exit 0