Support DW_AT_high_pc DWARF4 constant form

Support DW_AT_high_pc DWARF4 constant form

[Mark Wielaard]

Hi,

The DWARF spec says (since version 4) that DW_AT_high_pc can be
represented by a constant form.

        If the value of the DW_AT_high_pc is of class address, it is the
        relocated address of the first location past the last
        instruction associated with the entity; if it is of class
        constant, the value is an unsigned integer offset which when
        added to the low PC gives the address of the first location past
        the last instruction associated with the entity.

I have a patch for gcc to encode DW_AT_high_pc this way (which saves a
lot of relocations) and jakub has a patch for dwz to encode
DW_AT_high_pc in the smallest possible constant form, which can save ~1%
on the size of debuginfo.

Here are three patches to make binutils handle this.
The first is for bfd dwarf2.c dwarf reader. Without this some testcases
fail when using a gcc that outputs this new form.

The second is for gas dwarf2dbg.c to output the new form for DWARF
version 4+. Though this isn't actually used in practice, since almost
everything still uses DWARF version 2, except for VMS in tc-ia64.h.
The patch does include two fixes for the ranges and line table output
which should not use the wrong version code. I did test that all
testcases do pass when explicitly setting DWARF2_VERSION to 4 now.

The last is for gold --gdb-index support. It includes a new testcase
that will fail when using a gcc that outputs the new form.

Patches in separate emails:

 [PATCH 1/3] bfd: DW_AT_high_pc can be relative to DW_AT_low_pc.
 [PATCH 2/3] gas: Make dwarf2dbg.c versions specific and add
 [PATCH 3/3] gold: Handle DW_AT_high_pc as offset from DW_AT_low_pc

Thanks,

Mark

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

[Mark Wielaard]

        * 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.
---
 gold/ChangeLog                     |   12 ++++++++
 gold/dwarf_reader.cc               |   11 +++++++
 gold/dwarf_reader.h                |    5 +++
 gold/gdb-index.cc                  |   10 +++++-
 gold/testsuite/Makefile.am         |   12 ++++++++
 gold/testsuite/Makefile.in         |   52 ++++++++++++++++++++++-------------
 gold/testsuite/gdb_index_test_3.c  |   39 +++++++++++++++++++++++++++
 gold/testsuite/gdb_index_test_3.sh |   49 +++++++++++++++++++++++++++++++++
 8 files changed, 169 insertions(+), 21 deletions(-)
 create mode 100644 gold/testsuite/gdb_index_test_3.c
 create mode 100755 gold/testsuite/gdb_index_test_3.sh

diff --git a/gold/ChangeLog b/gold/ChangeLog
index 990247c..562e22a 100644
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,15 @@
+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.
+
 2012-04-25  Ian Lance Taylor  <iant@google.com>
 
 	* arm.cc (Target_arm::do_is_defined_by_abi): Make sym a const
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..b4ccd8c 100644
--- a/gold/gdb-index.cc
+++ b/gold/gdb-index.cc
@@ -824,8 +824,14 @@ Gdb_index_info_reader::record_cu_ranges(Dwarf_die* die)
     }
 
   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)
         {
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/Makefile.in b/gold/testsuite/Makefile.in
index 86c0ea3..d67c1c6 100644
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@@ -487,9 +487,14 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @GCC_TRUE@@HAVE_ZLIB_TRUE@@NATIVE_LINKER_TRUE@am__append_55 = gdb_index_test_2.stdout
 @GCC_TRUE@@HAVE_ZLIB_TRUE@@NATIVE_LINKER_TRUE@am__append_56 = gdb_index_test_2.stdout gdb_index_test_2
 
+# Another simple C test (DW_AT_high_pc encoding) for --gdb-index
+@GCC_TRUE@@NATIVE_LINKER_TRUE@am__append_57 = gdb_index_test_3.sh
+@GCC_TRUE@@NATIVE_LINKER_TRUE@am__append_58 = gdb_index_test_3.stdout
+@GCC_TRUE@@NATIVE_LINKER_TRUE@am__append_59 = gdb_index_test_3.stdout gdb_index_test_3
+
 # Test the --incremental-unchanged flag with an archive library.
 # The second link should not update the library.
-@DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@am__append_57 = incremental_test_2 \
+@DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@am__append_60 = incremental_test_2 \
 @DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	incremental_test_3 \
 @DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	incremental_test_4 \
 @DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	incremental_test_5 \
@@ -497,7 +502,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	incremental_copy_test \
 @DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	incremental_common_test_1 \
 @DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	incremental_comdat_test_1
-@DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@am__append_58 = two_file_test_tmp_2.o \
+@DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@am__append_61 = two_file_test_tmp_2.o \
 @DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_test_tmp_3.o \
 @DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	incremental_test_4.base \
 @DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_test_tmp_4.o \
@@ -507,22 +512,22 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 # These tests work with native and cross linkers.
 
 # Test script section order.
-@NATIVE_OR_CROSS_LINKER_TRUE@am__append_59 = script_test_10.sh
-@NATIVE_OR_CROSS_LINKER_TRUE@am__append_60 = script_test_10.stdout
+@NATIVE_OR_CROSS_LINKER_TRUE@am__append_62 = script_test_10.sh
+@NATIVE_OR_CROSS_LINKER_TRUE@am__append_63 = script_test_10.stdout
 
 # These tests work with cross linkers only.
-@DEFAULT_TARGET_I386_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_61 = split_i386.sh
-@DEFAULT_TARGET_I386_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_62 = split_i386_1.stdout split_i386_2.stdout \
+@DEFAULT_TARGET_I386_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_64 = split_i386.sh
+@DEFAULT_TARGET_I386_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_65 = split_i386_1.stdout split_i386_2.stdout \
 @DEFAULT_TARGET_I386_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	split_i386_3.stdout split_i386_4.stdout split_i386_r.stdout
 
-@DEFAULT_TARGET_I386_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_63 = split_i386_1 split_i386_2 split_i386_3 \
+@DEFAULT_TARGET_I386_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_66 = split_i386_1 split_i386_2 split_i386_3 \
 @DEFAULT_TARGET_I386_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	split_i386_4 split_i386_r
 
-@DEFAULT_TARGET_X86_64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_64 = split_x86_64.sh
-@DEFAULT_TARGET_X86_64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_65 = split_x86_64_1.stdout split_x86_64_2.stdout \
+@DEFAULT_TARGET_X86_64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_67 = split_x86_64.sh
+@DEFAULT_TARGET_X86_64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_68 = split_x86_64_1.stdout split_x86_64_2.stdout \
 @DEFAULT_TARGET_X86_64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	split_x86_64_3.stdout split_x86_64_4.stdout split_x86_64_r.stdout
 
-@DEFAULT_TARGET_X86_64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_66 = split_x86_64_1 split_x86_64_2 split_x86_64_3 \
+@DEFAULT_TARGET_X86_64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_69 = split_x86_64_1 split_x86_64_2 split_x86_64_3 \
 @DEFAULT_TARGET_X86_64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	split_x86_64_4 split_x86_64_r
 
 
@@ -537,7 +542,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 # Check Thumb to Thumb farcall veneers
 
 # Check Thumb to ARM farcall veneers
-@DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_67 = arm_abs_global.sh \
+@DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_70 = arm_abs_global.sh \
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	arm_branch_in_range.sh \
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	arm_branch_out_of_range.sh \
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	arm_fix_v4bx.sh \
@@ -551,7 +556,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	arm_farcall_arm_thumb.sh \
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	arm_farcall_thumb_thumb.sh \
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	arm_farcall_thumb_arm.sh
-@DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_68 = arm_abs_global.stdout \
+@DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_71 = arm_abs_global.stdout \
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	arm_bl_in_range.stdout \
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	arm_bl_out_of_range.stdout \
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	thumb_bl_in_range.stdout \
@@ -596,7 +601,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	arm_farcall_thumb_thumb_6m.stdout \
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	arm_farcall_thumb_arm.stdout \
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	arm_farcall_thumb_arm_5t.stdout
-@DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_69 = arm_abs_global \
+@DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_72 = arm_abs_global \
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	arm_bl_in_range \
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	arm_bl_out_of_range \
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	thumb_bl_in_range \
@@ -644,6 +649,7 @@ DIST_COMMON = $(srcdir)/Makefile.in $(srcdir)/Makefile.am
 ACLOCAL_M4 = $(top_srcdir)/aclocal.m4
 am__aclocal_m4_deps = $(top_srcdir)/../config/depstand.m4 \
 	$(top_srcdir)/../config/gettext-sister.m4 \
+	$(top_srcdir)/../config/lcmessage.m4 \
 	$(top_srcdir)/../config/lead-dot.m4 \
 	$(top_srcdir)/../config/nls.m4 \
 	$(top_srcdir)/../config/override.m4 \
@@ -2006,19 +2012,19 @@ TEST_AS = $(top_builddir)/../gas/as-new
 MOSTLYCLEANFILES = *.so *.syms *.stdout $(am__append_4) \
 	$(am__append_17) $(am__append_26) $(am__append_28) \
 	$(am__append_30) $(am__append_36) $(am__append_40) \
-	$(am__append_56) $(am__append_58) $(am__append_63) \
-	$(am__append_66) $(am__append_69)
+	$(am__append_56) $(am__append_59) $(am__append_61) \
+	$(am__append_66) $(am__append_69) $(am__append_72)
 
 # We will add to these later, for each individual test.  Note
 # that we add each test under check_SCRIPTS or check_PROGRAMS;
 # the TESTS variable is automatically populated from these.
 check_SCRIPTS = $(am__append_2) $(am__append_34) $(am__append_38) \
-	$(am__append_54) $(am__append_59) $(am__append_61) \
-	$(am__append_64) $(am__append_67)
+	$(am__append_54) $(am__append_57) $(am__append_62) \
+	$(am__append_64) $(am__append_67) $(am__append_70)
 check_DATA = $(am__append_3) $(am__append_27) $(am__append_29) \
 	$(am__append_35) $(am__append_39) $(am__append_55) \
-	$(am__append_60) $(am__append_62) $(am__append_65) \
-	$(am__append_68)
+	$(am__append_58) $(am__append_63) $(am__append_65) \
+	$(am__append_68) $(am__append_71)
 BUILT_SOURCES = $(am__append_25)
 TESTS = $(check_SCRIPTS) $(check_PROGRAMS)
 
@@ -3735,6 +3741,8 @@ gdb_index_test_1.sh.log: gdb_index_test_1.sh
 	@p='gdb_index_test_1.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 gdb_index_test_2.sh.log: gdb_index_test_2.sh
 	@p='gdb_index_test_2.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
+gdb_index_test_3.sh.log: gdb_index_test_3.sh
+	@p='gdb_index_test_3.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 script_test_10.sh.log: script_test_10.sh
 	@p='script_test_10.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 split_i386.sh.log: split_i386.sh
@@ -5068,6 +5076,12 @@ uninstall-am:
 @GCC_TRUE@@HAVE_ZLIB_TRUE@@NATIVE_LINKER_TRUE@	$(CXXLINK) -Bgcctestdir/ -Wl,--gdb-index $<
 @GCC_TRUE@@HAVE_ZLIB_TRUE@@NATIVE_LINKER_TRUE@gdb_index_test_2.stdout: gdb_index_test_2
 @GCC_TRUE@@HAVE_ZLIB_TRUE@@NATIVE_LINKER_TRUE@	$(TEST_READELF) --debug-dump=gdb_index $< > $@
+@GCC_TRUE@@NATIVE_LINKER_TRUE@gdb_index_test_3.o: gdb_index_test_3.c
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(COMPILE) -O0 -g -c -o $@ $<
+@GCC_TRUE@@NATIVE_LINKER_TRUE@gdb_index_test_3: gdb_index_test_3.o gcctestdir/ld
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(LINK) -Bgcctestdir/ -Wl,--gdb-index,--fatal-warnings $<
+@GCC_TRUE@@NATIVE_LINKER_TRUE@gdb_index_test_3.stdout: gdb_index_test_3
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(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
-- 
1.7.7.6

[PATCH 2/3] gas: Make dwarf2dbg.c versions specific and add DW_AT_high_pc case for DWARF 4+.

[Mark Wielaard]

       * dwarf2dbg.c (DWARF2_ARANGES_VERSION): New define to 2.
       (DWARF2_LINE_VERSION): Likewise.
       (out_debug_line): Use DWARF2_LINE_VERSION not DWARF2_VERSION.
       (out_debug_aranges): Use DWARF2_ARANGES_VERSION not DWARF2_VERSION.
       (out_debug_abbrev): Use DW_FORM_data for DW_AT_high_pc when
       DWARF2_VERSION >= 4.
       (out_debug_info): Use difference between start and end as data
       value for DW_AT_high_pc when DWARF2_VERSION >= 4.
---
 gas/ChangeLog   |   11 +++++++++++
 gas/dwarf2dbg.c |   24 ++++++++++++++++++++----
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/gas/ChangeLog b/gas/ChangeLog
index cfc3f8d..df3de43 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,14 @@
+2012-04-26  Mark Wielaard  <mjw@redhat.com>
+
+	* dwarf2dbg.c (DWARF2_ARANGES_VERSION): New define to 2.
+	(DWARF2_LINE_VERSION): Likewise.
+	(out_debug_line): Use DWARF2_LINE_VERSION not DWARF2_VERSION.
+	(out_debug_aranges): Use DWARF2_ARANGES_VERSION not DWARF2_VERSION.
+	(out_debug_abbrev): Use DW_FORM_data for DW_AT_high_pc when
+	DWARF2_VERSION >= 4.
+	(out_debug_info): Use difference between start and end as data
+	value for DW_AT_high_pc when DWARF2_VERSION >= 4.
+
 2012-04-20  Tristan Gingold  <gingold@adacore.com>
 
 	* config/tc-ia64.c (obj_elf_vms_common): New function.
diff --git a/gas/dwarf2dbg.c b/gas/dwarf2dbg.c
index 76f5e84..3e6c41e 100644
--- a/gas/dwarf2dbg.c
+++ b/gas/dwarf2dbg.c
@@ -93,6 +93,12 @@
 #define DWARF2_VERSION 2
 #endif
 
+/* The .debug_aranges version has been 2 in DWARF version 2, 3 and 4. */
+#define DWARF2_ARANGES_VERSION 2
+
+/* This implementation output version 2 .debug_line information. */
+#define DWARF2_LINE_VERSION 2
+
 #include "subsegs.h"
 
 #include "dwarf2.h"
@@ -1457,7 +1463,7 @@ out_debug_line (segT line_seg)
   line_end = exp.X_add_symbol;
 
   /* Version.  */
-  out_two (DWARF2_VERSION);
+  out_two (DWARF2_LINE_VERSION);
 
   /* Length of the prologue following this length.  */
   prologue_end = symbol_temp_make ();
@@ -1565,7 +1571,7 @@ out_debug_aranges (segT aranges_seg, segT info_seg)
   aranges_end = exp.X_add_symbol;
 
   /* Version.  */
-  out_two (DWARF2_VERSION);
+  out_two (DWARF2_ARANGES_VERSION);
 
   /* Offset to .debug_info.  */
   TC_DWARF2_EMIT_OFFSET (section_symbol (info_seg), sizeof_offset);
@@ -1631,7 +1637,11 @@ out_debug_abbrev (segT abbrev_seg,
   if (all_segs->next == NULL)
     {
       out_abbrev (DW_AT_low_pc, DW_FORM_addr);
-      out_abbrev (DW_AT_high_pc, DW_FORM_addr);
+      if (DWARF2_VERSION < 4)
+	out_abbrev (DW_AT_high_pc, DW_FORM_addr);
+      else
+	out_abbrev (DW_AT_high_pc, (sizeof_address == 4
+				    ? DW_FORM_data4 : DW_FORM_data8));
     }
   else
     {
@@ -1694,7 +1704,13 @@ out_debug_info (segT info_seg, segT abbrev_seg, segT line_seg, segT ranges_seg)
       emit_expr (&exp, sizeof_address);
 
       /* DW_AT_high_pc */
-      exp.X_op = O_symbol;
+      if (DWARF2_VERSION < 4)
+	exp.X_op = O_symbol;
+      else
+	{
+	  exp.X_op = O_subtract;
+	  exp.X_op_symbol = all_segs->text_start;
+	}
       exp.X_add_symbol = all_segs->text_end;
       exp.X_add_number = 0;
       emit_expr (&exp, sizeof_address);
-- 
1.7.7.6

[PATCH 1/3] bfd: DW_AT_high_pc can be relative to DW_AT_low_pc.

[Mark Wielaard]

       * dwarf2.c (scan_unit_for_symbols): Account for DW_AT_high_pc
       possibly being relative to DW_AT_low_pc.
       (parse_comp_unit): Likewise.
---
 bfd/ChangeLog |    6 ++++++
 bfd/dwarf2.c  |   11 +++++++++++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index b46eb9c..0978890 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,9 @@
+2012-04-26  Mark Wielaard  <mjw@redhat.com>
+
+	* dwarf2.c (scan_unit_for_symbols): Account for DW_AT_high_pc
+	possibly being relative to DW_AT_low_pc.
+	(parse_comp_unit): Likewise.
+
 2012-04-24  Hans-Peter Nilsson  <hp@axis.com>
 
 	PR ld/13990
diff --git a/bfd/dwarf2.c b/bfd/dwarf2.c
index 66fd16f..4098d70 100644
--- a/bfd/dwarf2.c
+++ b/bfd/dwarf2.c
@@ -2092,6 +2092,7 @@ scan_unit_for_symbols (struct comp_unit *unit)
       struct varinfo *var;
       bfd_vma low_pc = 0;
       bfd_vma high_pc = 0;
+      int high_pc_relative = 0;
 
       abbrev_number = read_unsigned_leb128 (abfd, info_ptr, &bytes_read);
       info_ptr += bytes_read;
@@ -2197,6 +2198,8 @@ scan_unit_for_symbols (struct comp_unit *unit)
 
 		case DW_AT_high_pc:
 		  high_pc = attr.u.val;
+		  if (attr.form != DW_FORM_addr)
+		    high_pc_relative = 1;
 		  break;
 
 		case DW_AT_ranges:
@@ -2275,6 +2278,9 @@ scan_unit_for_symbols (struct comp_unit *unit)
 	    }
 	}
 
+      if (high_pc_relative == 1)
+	high_pc += low_pc;
+
       if (func && high_pc != 0)
 	{
 	  if (!arange_add (unit->abfd, &func->arange, low_pc, high_pc))
@@ -2338,6 +2344,7 @@ parse_comp_unit (struct dwarf2_debug *stash,
   bfd_vma low_pc = 0;
   bfd_vma high_pc = 0;
   bfd *abfd = stash->bfd_ptr;
+  int high_pc_relative = 0;
 
   version = read_2_bytes (abfd, info_ptr);
   info_ptr += 2;
@@ -2440,6 +2447,8 @@ parse_comp_unit (struct dwarf2_debug *stash,
 
 	case DW_AT_high_pc:
 	  high_pc = attr.u.val;
+	  if (attr.form != DW_FORM_addr)
+	    high_pc_relative = 1;
 	  break;
 
 	case DW_AT_ranges:
@@ -2467,6 +2476,8 @@ parse_comp_unit (struct dwarf2_debug *stash,
 	  break;
 	}
     }
+  if (high_pc_relative == 1)
+    high_pc += low_pc;
   if (high_pc != 0)
     {
       if (!arange_add (unit->abfd, &unit->arange, low_pc, high_pc))
-- 
1.7.7.6

Re: [PATCH 1/3] bfd: DW_AT_high_pc can be relative to DW_AT_low_pc.

[Jakub Jelinek]

On Fri, Apr 27, 2012 at 02:15:19PM +0200, Mark Wielaard wrote:
> --- a/bfd/dwarf2.c
> +++ b/bfd/dwarf2.c
> @@ -2092,6 +2092,7 @@ scan_unit_for_symbols (struct comp_unit *unit)
>        struct varinfo *var;
>        bfd_vma low_pc = 0;
>        bfd_vma high_pc = 0;
> +      int high_pc_relative = 0;

Please use bfd_boolean high_pc_relative = FALSE; instead.
>  
>        abbrev_number = read_unsigned_leb128 (abfd, info_ptr, &bytes_read);
>        info_ptr += bytes_read;
> @@ -2197,6 +2198,8 @@ scan_unit_for_symbols (struct comp_unit *unit)
>  
>  		case DW_AT_high_pc:
>  		  high_pc = attr.u.val;
> +		  if (attr.form != DW_FORM_addr)
> +		    high_pc_relative = 1;

And high_pc_relative = attr.form != DW_FORM_addr; here.

> @@ -2275,6 +2278,9 @@ scan_unit_for_symbols (struct comp_unit *unit)
>  	    }
>  	}
>  
> +      if (high_pc_relative == 1)
> +	high_pc += low_pc;

And just if (high_pc_relative) here.
Similarly elsewhere.

Ok for trunk with those changes, thanks.

	Jakub

Re: [PATCH 2/3] gas: Make dwarf2dbg.c versions specific and add DW_AT_high_pc case for DWARF 4+.

[Tristan Gingold]

On Apr 27, 2012, at 2:15 PM, Mark Wielaard wrote:

>       * dwarf2dbg.c (DWARF2_ARANGES_VERSION): New define to 2.
>       (DWARF2_LINE_VERSION): Likewise.
>       (out_debug_line): Use DWARF2_LINE_VERSION not DWARF2_VERSION.
>       (out_debug_aranges): Use DWARF2_ARANGES_VERSION not DWARF2_VERSION.
>       (out_debug_abbrev): Use DW_FORM_data for DW_AT_high_pc when
>       DWARF2_VERSION >= 4.
>       (out_debug_info): Use difference between start and end as data
>       value for DW_AT_high_pc when DWARF2_VERSION >= 4.
> ---
> gas/ChangeLog   |   11 +++++++++++
> gas/dwarf2dbg.c |   24 ++++++++++++++++++++----
> 2 files changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/gas/ChangeLog b/gas/ChangeLog
> index cfc3f8d..df3de43 100644
> --- a/gas/ChangeLog
> +++ b/gas/ChangeLog
> @@ -1,3 +1,14 @@
> +2012-04-26  Mark Wielaard  <mjw@redhat.com>
> +
> +	* dwarf2dbg.c (DWARF2_ARANGES_VERSION): New define to 2.
> +	(DWARF2_LINE_VERSION): Likewise.
> +	(out_debug_line): Use DWARF2_LINE_VERSION not DWARF2_VERSION.
> +	(out_debug_aranges): Use DWARF2_ARANGES_VERSION not DWARF2_VERSION.
> +	(out_debug_abbrev): Use DW_FORM_data for DW_AT_high_pc when
> +	DWARF2_VERSION >= 4.
> +	(out_debug_info): Use difference between start and end as data
> +	value for DW_AT_high_pc when DWARF2_VERSION >= 4.
> +
> 2012-04-20  Tristan Gingold  <gingold@adacore.com>

[ Usually the ChangeLog chunk is omitted ]

> 	* config/tc-ia64.c (obj_elf_vms_common): New function.
> diff --git a/gas/dwarf2dbg.c b/gas/dwarf2dbg.c
> index 76f5e84..3e6c41e 100644
> --- a/gas/dwarf2dbg.c
> +++ b/gas/dwarf2dbg.c
> @@ -93,6 +93,12 @@
> #define DWARF2_VERSION 2
> #endif
> 
> +/* The .debug_aranges version has been 2 in DWARF version 2, 3 and 4. */
> +#define DWARF2_ARANGES_VERSION 2
> +
> +/* This implementation output version 2 .debug_line information. */
> +#define DWARF2_LINE_VERSION 2

Please, wrap DWARF2_LINE_VERSION within #ifndef/#endif, so that a target can override it.  In particular on ia64/VMS, the backtrace symbolizer requires version 3.

> +
> #include "subsegs.h"
> 
> #include "dwarf2.h"
> @@ -1457,7 +1463,7 @@ out_debug_line (segT line_seg)
>   line_end = exp.X_add_symbol;
> 
>   /* Version.  */
> -  out_two (DWARF2_VERSION);
> +  out_two (DWARF2_LINE_VERSION);
> 
>   /* Length of the prologue following this length.  */
>   prologue_end = symbol_temp_make ();
> @@ -1565,7 +1571,7 @@ out_debug_aranges (segT aranges_seg, segT info_seg)
>   aranges_end = exp.X_add_symbol;
> 
>   /* Version.  */
> -  out_two (DWARF2_VERSION);
> +  out_two (DWARF2_ARANGES_VERSION);
> 
>   /* Offset to .debug_info.  */
>   TC_DWARF2_EMIT_OFFSET (section_symbol (info_seg), sizeof_offset);
> @@ -1631,7 +1637,11 @@ out_debug_abbrev (segT abbrev_seg,
>   if (all_segs->next == NULL)
>     {
>       out_abbrev (DW_AT_low_pc, DW_FORM_addr);
> -      out_abbrev (DW_AT_high_pc, DW_FORM_addr);
> +      if (DWARF2_VERSION < 4)
> +	out_abbrev (DW_AT_high_pc, DW_FORM_addr);
> +      else
> +	out_abbrev (DW_AT_high_pc, (sizeof_address == 4
> +				    ? DW_FORM_data4 : DW_FORM_data8));
>     }
>   else
>     {
> @@ -1694,7 +1704,13 @@ out_debug_info (segT info_seg, segT abbrev_seg, segT line_seg, segT ranges_seg)
>       emit_expr (&exp, sizeof_address);
> 
>       /* DW_AT_high_pc */
> -      exp.X_op = O_symbol;
> +      if (DWARF2_VERSION < 4)
> +	exp.X_op = O_symbol;
> +      else
> +	{
> +	  exp.X_op = O_subtract;
> +	  exp.X_op_symbol = all_segs->text_start;
> +	}
>       exp.X_add_symbol = all_segs->text_end;
>       exp.X_add_number = 0;
>       emit_expr (&exp, sizeof_address);

Otherwise, looks good to me (but I cannot approve it).

Tristan.

Re: [PATCH 2/3] gas: Make dwarf2dbg.c versions specific and add DW_AT_high_pc case for DWARF 4+.

[Jakub Jelinek]

On Fri, Apr 27, 2012 at 02:15:20PM +0200, Mark Wielaard wrote:
>        * dwarf2dbg.c (DWARF2_ARANGES_VERSION): New define to 2.
>        (DWARF2_LINE_VERSION): Likewise.
>        (out_debug_line): Use DWARF2_LINE_VERSION not DWARF2_VERSION.
>        (out_debug_aranges): Use DWARF2_ARANGES_VERSION not DWARF2_VERSION.
>        (out_debug_abbrev): Use DW_FORM_data for DW_AT_high_pc when
>        DWARF2_VERSION >= 4.
>        (out_debug_info): Use difference between start and end as data
>        value for DW_AT_high_pc when DWARF2_VERSION >= 4.

Ok, thanks.

	Jakub

Re: [PATCH 2/3] gas: Make dwarf2dbg.c versions specific and add DW_AT_high_pc case for DWARF 4+.

[Mark Wielaard]

On Fri, 2012-04-27 at 14:28 +0200, Tristan Gingold wrote:
> > +/* The .debug_aranges version has been 2 in DWARF version 2, 3 and 4. */
> > +#define DWARF2_ARANGES_VERSION 2
> > +
> > +/* This implementation output version 2 .debug_line information. */
> > +#define DWARF2_LINE_VERSION 2
> 
> Please, wrap DWARF2_LINE_VERSION within #ifndef/#endif, so that a
> target can override it.  In particular on ia64/VMS, the backtrace
> symbolizer requires version 3.

OK can do. Do you know which version it wants set to 3?
info, aranges or line?

The trouble with the current code is that info, line and aranges all use
the same version number. But for aranges only 2 is a valid version and
the code does output only version 2 line info. Though it might be valid
to claim version 3 since the header length shows there is not really
more info than with version 2. But version 4 line info is really
different from earlier versions (readers will not be able to read it if
we set the version to 4).

Thanks,

Mark

Re: [PATCH 2/3] gas: Make dwarf2dbg.c versions specific and add DW_AT_high_pc case for DWARF 4+.

[Tristan Gingold]

On Apr 27, 2012, at 4:32 PM, Mark Wielaard wrote:

> On Fri, 2012-04-27 at 14:28 +0200, Tristan Gingold wrote:
>>> +/* The .debug_aranges version has been 2 in DWARF version 2, 3 and 4. */
>>> +#define DWARF2_ARANGES_VERSION 2
>>> +
>>> +/* This implementation output version 2 .debug_line information. */
>>> +#define DWARF2_LINE_VERSION 2
>> 
>> Please, wrap DWARF2_LINE_VERSION within #ifndef/#endif, so that a
>> target can override it.  In particular on ia64/VMS, the backtrace
>> symbolizer requires version 3.
> 
> OK can do. Do you know which version it wants set to 3?
> info, aranges or line?

Just line.

> The trouble with the current code is that info, line and aranges all use
> the same version number. But for aranges only 2 is a valid version and
> the code does output only version 2 line info. Though it might be valid
> to claim version 3 since the header length shows there is not really
> more info than with version 2. But version 4 line info is really
> different from earlier versions (readers will not be able to read it if
> we set the version to 4).

Agreed.  Looks like there is a tolerance for aranges.

Thanks,
Tristan.

Re: [PATCH 1/3] bfd: DW_AT_high_pc can be relative to DW_AT_low_pc.

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 196 bytes --]

On Fri, 2012-04-27 at 14:30 +0200, Jakub Jelinek wrote:
> And just if (high_pc_relative) here.
> Similarly elsewhere.
> 
> Ok for trunk with those changes, thanks.

Updated patch attached.

[-- Attachment #2: binutils-01.patch --]
[-- Type: text/x-patch, Size: 2358 bytes --]

commit 506b7f75204e9e19200087dddc84612964852706
Author: Mark Wielaard <mjw@redhat.com>
Date:   Thu Apr 26 15:11:52 2012 +0200

    bfd: DW_AT_high_pc can be relative to DW_AT_low_pc.
    
           * dwarf2.c (scan_unit_for_symbols): Account for DW_AT_high_pc
           possibly being relative to DW_AT_low_pc.
           (parse_comp_unit): Likewise.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index b46eb9c..0978890 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,9 @@
+2012-04-26  Mark Wielaard  <mjw@redhat.com>
+
+	* dwarf2.c (scan_unit_for_symbols): Account for DW_AT_high_pc
+	possibly being relative to DW_AT_low_pc.
+	(parse_comp_unit): Likewise.
+
 2012-04-24  Hans-Peter Nilsson  <hp@axis.com>
 
 	PR ld/13990
diff --git a/bfd/dwarf2.c b/bfd/dwarf2.c
index 66fd16f..51e27b4 100644
--- a/bfd/dwarf2.c
+++ b/bfd/dwarf2.c
@@ -2092,6 +2092,7 @@ scan_unit_for_symbols (struct comp_unit *unit)
       struct varinfo *var;
       bfd_vma low_pc = 0;
       bfd_vma high_pc = 0;
+      bfd_boolean high_pc_relative = FALSE;
 
       abbrev_number = read_unsigned_leb128 (abfd, info_ptr, &bytes_read);
       info_ptr += bytes_read;
@@ -2197,6 +2198,7 @@ scan_unit_for_symbols (struct comp_unit *unit)
 
 		case DW_AT_high_pc:
 		  high_pc = attr.u.val;
+		  high_pc_relative = attr.form != DW_FORM_addr;
 		  break;
 
 		case DW_AT_ranges:
@@ -2275,6 +2277,9 @@ scan_unit_for_symbols (struct comp_unit *unit)
 	    }
 	}
 
+      if (high_pc_relative)
+	high_pc += low_pc;
+
       if (func && high_pc != 0)
 	{
 	  if (!arange_add (unit->abfd, &func->arange, low_pc, high_pc))
@@ -2338,6 +2343,7 @@ parse_comp_unit (struct dwarf2_debug *stash,
   bfd_vma low_pc = 0;
   bfd_vma high_pc = 0;
   bfd *abfd = stash->bfd_ptr;
+  bfd_boolean high_pc_relative = FALSE;
 
   version = read_2_bytes (abfd, info_ptr);
   info_ptr += 2;
@@ -2440,6 +2446,7 @@ parse_comp_unit (struct dwarf2_debug *stash,
 
 	case DW_AT_high_pc:
 	  high_pc = attr.u.val;
+	  high_pc_relative = attr.form != DW_FORM_addr;
 	  break;
 
 	case DW_AT_ranges:
@@ -2467,6 +2474,8 @@ parse_comp_unit (struct dwarf2_debug *stash,
 	  break;
 	}
     }
+  if (high_pc_relative)
+    high_pc += low_pc;
   if (high_pc != 0)
     {
       if (!arange_add (unit->abfd, &unit->arange, low_pc, high_pc))

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

[Cary Coutant]

>   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.

The rest looks good to me. (I can't approve, though.)

Thanks!

-cary

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

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 1459 bytes --]

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

[-- Attachment #2: binutils-gold.patch --]
[-- Type: text/x-patch, Size: 5946 bytes --]

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

Re: [PATCH 2/3] gas: Make dwarf2dbg.c versions specific and add DW_AT_high_pc case for DWARF 4+.

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 1487 bytes --]

On Fri, 2012-04-27 at 16:39 +0200, Tristan Gingold wrote:
> On Apr 27, 2012, at 4:32 PM, Mark Wielaard wrote:
> 
> > On Fri, 2012-04-27 at 14:28 +0200, Tristan Gingold wrote:
> >>> +/* The .debug_aranges version has been 2 in DWARF version 2, 3 and 4. */
> >>> +#define DWARF2_ARANGES_VERSION 2
> >>> +
> >>> +/* This implementation output version 2 .debug_line information. */
> >>> +#define DWARF2_LINE_VERSION 2
> >> 
> >> Please, wrap DWARF2_LINE_VERSION within #ifndef/#endif, so that a
> >> target can override it.  In particular on ia64/VMS, the backtrace
> >> symbolizer requires version 3.
> > 
> > OK can do. Do you know which version it wants set to 3?
> > info, aranges or line?
> 
> Just line.

OK, would the following version of the patch work for you?
The only change to the variant that jakub approved is the addition of
#ifndef and the override in config/tc-ia64.h.

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

    * dwarf2dbg.c (DWARF2_ARANGES_VERSION): New define to 2.
    (DWARF2_LINE_VERSION): Likewise.
    (out_debug_line): Use DWARF2_LINE_VERSION not DWARF2_VERSION.
    (out_debug_aranges): Use DWARF2_ARANGES_VERSION not DWARF2_VERSION.
    (out_debug_abbrev): Use DW_FORM_data for DW_AT_high_pc when
    DWARF2_VERSION >= 4.
    (out_debug_info): Use difference between start and end as data
    value for DW_AT_high_pc when DWARF2_VERSION >= 4.
    * config/tc-ia64.h (DWARF2_LINE_VERSION): Override it.

Thanks,

Mark

[-- Attachment #2: binutils-gas.patch --]
[-- Type: text/x-patch, Size: 2328 bytes --]

diff --git a/gas/config/tc-ia64.h b/gas/config/tc-ia64.h
index 96618e9..7cade49 100644
--- a/gas/config/tc-ia64.h
+++ b/gas/config/tc-ia64.h
@@ -327,4 +327,5 @@ typedef struct unwind_record
 /* VMS backtraces expect dwarf version 3.  */
 #ifdef TE_VMS
 #define DWARF2_VERSION 3
+#define DWARF2_LINE_VERSION 3
 #endif
diff --git a/gas/dwarf2dbg.c b/gas/dwarf2dbg.c
index 76f5e84..f428f0d 100644
--- a/gas/dwarf2dbg.c
+++ b/gas/dwarf2dbg.c
@@ -93,6 +93,16 @@
 #define DWARF2_VERSION 2
 #endif
 
+/* The .debug_aranges version has been 2 in DWARF version 2, 3 and 4. */
+#ifndef DWARF2_ARANGES_VERSION
+#define DWARF2_ARANGES_VERSION 2
+#endif
+
+/* This implementation output version 2 .debug_line information. */
+#ifndef DWARF2_LINE_VERSION
+#define DWARF2_LINE_VERSION 2
+#endif
+
 #include "subsegs.h"
 
 #include "dwarf2.h"
@@ -1457,7 +1467,7 @@ out_debug_line (segT line_seg)
   line_end = exp.X_add_symbol;
 
   /* Version.  */
-  out_two (DWARF2_VERSION);
+  out_two (DWARF2_LINE_VERSION);
 
   /* Length of the prologue following this length.  */
   prologue_end = symbol_temp_make ();
@@ -1565,7 +1575,7 @@ out_debug_aranges (segT aranges_seg, segT info_seg)
   aranges_end = exp.X_add_symbol;
 
   /* Version.  */
-  out_two (DWARF2_VERSION);
+  out_two (DWARF2_ARANGES_VERSION);
 
   /* Offset to .debug_info.  */
   TC_DWARF2_EMIT_OFFSET (section_symbol (info_seg), sizeof_offset);
@@ -1631,7 +1641,11 @@ out_debug_abbrev (segT abbrev_seg,
   if (all_segs->next == NULL)
     {
       out_abbrev (DW_AT_low_pc, DW_FORM_addr);
-      out_abbrev (DW_AT_high_pc, DW_FORM_addr);
+      if (DWARF2_VERSION < 4)
+	out_abbrev (DW_AT_high_pc, DW_FORM_addr);
+      else
+	out_abbrev (DW_AT_high_pc, (sizeof_address == 4
+				    ? DW_FORM_data4 : DW_FORM_data8));
     }
   else
     {
@@ -1694,7 +1708,13 @@ out_debug_info (segT info_seg, segT abbrev_seg, segT line_seg, segT ranges_seg)
       emit_expr (&exp, sizeof_address);
 
       /* DW_AT_high_pc */
-      exp.X_op = O_symbol;
+      if (DWARF2_VERSION < 4)
+	exp.X_op = O_symbol;
+      else
+	{
+	  exp.X_op = O_subtract;
+	  exp.X_op_symbol = all_segs->text_start;
+	}
       exp.X_add_symbol = all_segs->text_end;
       exp.X_add_number = 0;
       emit_expr (&exp, sizeof_address);

Re: [PATCH 2/3] gas: Make dwarf2dbg.c versions specific and add DW_AT_high_pc case for DWARF 4+.

[Tristan Gingold]

On Apr 28, 2012, at 12:21 AM, Mark Wielaard wrote:

> On Fri, 2012-04-27 at 16:39 +0200, Tristan Gingold wrote:
>> On Apr 27, 2012, at 4:32 PM, Mark Wielaard wrote:
>> 
>>> On Fri, 2012-04-27 at 14:28 +0200, Tristan Gingold wrote:
>>>>> +/* The .debug_aranges version has been 2 in DWARF version 2, 3 and 4. */
>>>>> +#define DWARF2_ARANGES_VERSION 2
>>>>> +
>>>>> +/* This implementation output version 2 .debug_line information. */
>>>>> +#define DWARF2_LINE_VERSION 2
>>>> 
>>>> Please, wrap DWARF2_LINE_VERSION within #ifndef/#endif, so that a
>>>> target can override it.  In particular on ia64/VMS, the backtrace
>>>> symbolizer requires version 3.
>>> 
>>> OK can do. Do you know which version it wants set to 3?
>>> info, aranges or line?
>> 
>> Just line.
> 
> OK, would the following version of the patch work for you?
> The only change to the variant that jakub approved is the addition of
> #ifndef and the override in config/tc-ia64.h.

This looks good to me.  I haven't tested it but I will adjust it in case of issues.

Thanks,
Tristan.

> 
> 2012-04-26  Mark Wielaard  <mjw@redhat.com>
> 
>    * dwarf2dbg.c (DWARF2_ARANGES_VERSION): New define to 2.
>    (DWARF2_LINE_VERSION): Likewise.
>    (out_debug_line): Use DWARF2_LINE_VERSION not DWARF2_VERSION.
>    (out_debug_aranges): Use DWARF2_ARANGES_VERSION not DWARF2_VERSION.
>    (out_debug_abbrev): Use DW_FORM_data for DW_AT_high_pc when
>    DWARF2_VERSION >= 4.
>    (out_debug_info): Use difference between start and end as data
>    value for DW_AT_high_pc when DWARF2_VERSION >= 4.
>    * config/tc-ia64.h (DWARF2_LINE_VERSION): Override it.
> 
> Thanks,
> 
> Mark
> <binutils-gas.patch>

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

[Ian Lance Taylor]

Mark Wielaard <mjw@redhat.com> writes:

> 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.


> 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);

Looks like you can bring shndx up to the previous line here.


This is OK with that change.

Thanks.

Ian

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

[Cary Coutant]

>    * testsuite/gdb_index_test_3.c: New test source file.
>    * testsuite/gdb_index_test_3.sh: New test source file.

These two new files were accidentally omitted from the commit. I've
checked them in.

-cary

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

[Mark Wielaard]

On Tue, May 01, 2012 at 03:45:24PM -0700, Cary Coutant wrote:
> >    * testsuite/gdb_index_test_3.c: New test source file.
> >    * testsuite/gdb_index_test_3.sh: New test source file.
> 
> These two new files were accidentally omitted from the commit. I've
> checked them in.

Thanks and apologies for forgetting to cvs add.