[RFA/DWARF] constant class of DW_AT_high_pc is offset for version >=4 only.

[RFA/DWARF] constant class of DW_AT_high_pc is offset for version >=4 only.

[Joel Brobecker]

Starting with DWARF version 4, the description of the DW_AT_high_pc
attribute was amended to say:

   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.

A change was made in Apr 27th, 2012 to reflect that change:

  | commit 91da14142c0171e58a91ad58a32fd010b700e761
  | Author: Mark Wielaard <mjw@redhat.com>
  | Date:   Fri Apr 27 18:55:19 2012 +0000
  |
  |     * dwarf2read.c (dwarf2_get_pc_bounds): Check DW_AT_high_pc form to
  |     see whether it is an address or a constant offset from DW_AT_low_pc.
  |     (dwarf2_record_block_ranges): Likewise.
  |     (read_partial_die): Likewise.

Unfortunately, this new interpretation is now used regardless of
the CU's DWARF version. It turns out that one of WindRiver's compilers
(FTR: Diabdata 4.4) is generating DWARF version 2 info with
DW_AT_high_pc attributes using the data4 form. Because of that,
we miscompute all high PCs incorrectly. This leads to a lot of
symtabs having overlapping ranges, which in turn causes havoc
in pc-to-symtab-and-line translations.

One visible effect is when inserting a breakpoint on a given function:

    (gdb) b world
    Breakpoint 1 at 0x4005c4

The source location of the breakpoint is missing. The output should be:

    (gdb) b world
    Breakpoint 1 at 0x4005c8: file dw2-rel-hi-pc-world.c, line 24.

What happens in this case is that the pc-to-SAL translation first
starts be trying to find the symtab associated to our PC using
each symtab's ranges. Because of the high_pc miscomputation,
many symtabs end up matching, and the heuristic trying to select
the most probable one unfortunately returns one that is unrelated
(it really had no change in this case to do any better). Once we
have the wrong symtab, the start searching the associated linetable,
where the addresses are correct, thus finding no match, and therefore
no SAL.

This patch fixes it by ammending a bit the changes from the commit
mentioned above to use the new interpretation of a relative address
only when the CU's DWARF version is 4 or greater.

gdb/ChangeLog:

        * dwarf2read.c (dwarf2_get_pc): Treat the DW_AT_high_pc attribute
        as an offset only when the CU's DWARF version is 4 or greater.
        (dwarf2_record_block_ranges, read_partial_die): Likewise.

gdb/testsuite/ChangeLog:

        * gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S: New file.
        * gdb.dwarf2/dw2-abs-hi-pc-hello.c: New file.
        * gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S: New file.
        * gdb.dwarf2/dw2-abs-hi-pc-world.c: New file.
        * gdb.dwarf2/dw2-abs-hi-pc.c: New file.
        * gdb.dwarf2/dw2-abs-hi-pc.exp: New file.

Tested on x86_64-linux. OK to commmit?

As a side note, it took me nearly 4 hours to manage to create
this assembly-based testcase, mostly trying to generate a valid
linetable. There is no testcase for this feature with version >=4.
Now that the hard work has been done, it should be pretty trivial
to tweak this testscase to generate another testing version 4.
I will do that once this testcase is approved. That way, if
I have to make some modifications to this one, I won't have to
make them again to the version 4 one.

---
 gdb/dwarf2read.c                                   |  27 ++--
 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S | 151 +++++++++++++++++++++
 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello.c     |  28 ++++
 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S | 151 +++++++++++++++++++++
 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world.c     |  28 ++++
 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.c           |  28 ++++
 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.exp         |  36 +++++
 7 files changed, 437 insertions(+), 12 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.exp

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 54c538a..f71730d 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -11675,11 +11675,12 @@ dwarf2_get_pc_bounds (struct die_info *die, CORE_ADDR *lowpc,
       if (attr)
         {
 	  low = DW_ADDR (attr);
-	  if (attr_high->form == DW_FORM_addr
-	      || attr_high->form == DW_FORM_GNU_addr_index)
-	    high = DW_ADDR (attr_high);
-	  else
+	  if (cu->header.version >= 4
+	      && attr_high->form != DW_FORM_addr
+	      && attr_high->form != DW_FORM_GNU_addr_index)
 	    high = low + DW_UNSND (attr_high);
+	  else
+	    high = DW_ADDR (attr_high);
 	}
       else
 	/* Found high w/o low attribute.  */
@@ -11847,11 +11848,12 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
         {
           CORE_ADDR low = DW_ADDR (attr);
 	  CORE_ADDR high;
-	  if (attr_high->form == DW_FORM_addr
-	      || attr_high->form == DW_FORM_GNU_addr_index)
-	    high = DW_ADDR (attr_high);
-	  else
+	  if (cu->header.version >= 4
+	      && attr_high->form != DW_FORM_addr
+	      && attr_high->form != DW_FORM_GNU_addr_index)
 	    high = low + DW_UNSND (attr_high);
+	  else
+	    high = DW_ADDR (attr_high);
 
           record_block_range (block, baseaddr + low, baseaddr + high - 1);
         }
@@ -15244,14 +15246,15 @@ read_partial_die (const struct die_reader_specs *reader,
 	  break;
 	case DW_AT_high_pc:
 	  has_high_pc_attr = 1;
-	  if (attr.form == DW_FORM_addr
-	      || attr.form == DW_FORM_GNU_addr_index)
-	    part_die->highpc = DW_ADDR (&attr);
-	  else
+	  if (cu->header.version >= 4
+	      && attr.form != DW_FORM_addr
+	      && attr.form != DW_FORM_GNU_addr_index)
 	    {
 	      high_pc_relative = 1;
 	      part_die->highpc = DW_UNSND (&attr);
 	    }
+	  else
+	    part_die->highpc = DW_ADDR (&attr);
 	  break;
 	case DW_AT_location:
           /* Support the .debug_loc offsets.  */
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S
new file mode 100644
index 0000000..f7ea9ae
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S
@@ -0,0 +1,151 @@
+/* Copyright 2014 Free Software Foundation, Inc.
+
+   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, see <http://www.gnu.org/licenses/>.  */
+
+	.section	.debug_info,"",@progbits
+	.4byte	.Ledebug_info0 - .Lsdebug_info0	/* Length of Compilation Unit Info */
+.Lsdebug_info0:
+	.value	0x2	/* DWARF version number */
+	.4byte	.Ldebug_abbrev0	/* Offset Into Abbrev. Section */
+	.byte	0x4	/* Pointer Size (in bytes) */
+	.uleb128 0x1	/* (DIE (0xb) DW_TAG_compile_unit) */
+        .ascii "GNU C 4.7.4 20140206 for GNAT Pro 7.3.0w (20140206)\0"
+	.byte	0x1	/* DW_AT_language */
+	.ascii	"dw2-rel-hi-pc-hello.c\0"     /* DW_AT_name */
+	.ascii  "/tmp\0"        /* DW_AT_comp_dir */
+	.4byte	.hello_start	/* DW_AT_low_pc */
+	.4byte	.hello_end	/* DW_AT_high_pc */
+	.4byte	.Ldebug_line0	/* DW_AT_stmt_list */
+	.uleb128 0x2	/* (DIE (0x2d) DW_TAG_subprogram) */
+	.byte	0x1	/* DW_AT_external */
+	.ascii	"hello\0"
+	.byte	0x1	/* DW_AT_decl_file (hello.c) */
+	.byte	0x13	/* DW_AT_decl_line */
+	.byte	0x1	/* DW_AT_prototyped */
+	.4byte	.hello_start	/* DW_AT_low_pc */
+	.4byte	.hello_end	/* DW_AT_high_pc */
+	.byte	0	/* end of children of DIE 0xb */
+.Ledebug_info0:
+
+	.section	.debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+	.uleb128 0x1	/* (abbrev code) */
+	.uleb128 0x11	/* (TAG: DW_TAG_compile_unit) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x25	/* (DW_AT_producer) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x13	/* (DW_AT_language) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x1b	/* (DW_AT_comp_dir) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x1	/* (DW_FORM_addr) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x10	/* (DW_AT_stmt_list) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.byte	0
+	.byte	0
+	.uleb128 0x2	/* (abbrev code) */
+	.uleb128 0x2e	/* (TAG: DW_TAG_subprogram) */
+	.byte	0	/* DW_children_no */
+	.uleb128 0x3f	/* (DW_AT_external) */
+	.uleb128 0xc	/* (DW_FORM_flag) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x3a	/* (DW_AT_decl_file) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3b	/* (DW_AT_decl_line) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x27	/* (DW_AT_prototyped) */
+	.uleb128 0xc	/* (DW_FORM_flag) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x1	/* (DW_FORM_addr) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.byte	0
+	.byte	0
+	.byte	0
+
+	.section	.debug_line,"",@progbits
+.Ldebug_line0:
+        .4byte  LELT-LSLT  /* Length of Source Line Info */
+LSLT:
+        .2byte  0x2    /* DWARF Version */
+        .4byte  LELTP-LASLTP     /* Prolog Length */
+LASLTP:
+        .byte   0x1      /* Minimum Instruction Length */
+        .byte   0x1      /* Default is_stmt_start flag */
+        .byte   0x1      /* Line Base Value (Special Opcodes) */
+        .byte   0x1      /* Line Range Value (Special Opcodes) */
+        .byte   0xd      /* Special Opcode Base */
+        .byte   0        /* opcode: 0x1 has 0 args */
+        .byte   0x1      /* opcode: 0x2 has 1 args */
+        .byte   0x1      /* opcode: 0x3 has 1 args */
+        .byte   0x1      /* opcode: 0x4 has 1 args */
+        .byte   0x1      /* opcode: 0x5 has 1 args */
+        .byte   0        /* opcode: 0x6 has 0 args */
+        .byte   0        /* opcode: 0x7 has 0 args */
+        .byte   0        /* opcode: 0x8 has 0 args */
+        .byte   0x1      /* opcode: 0x9 has 1 args */
+        .byte   0        /* opcode: 0xa has 0 args */
+        .byte   0        /* opcode: 0xb has 0 args */
+        .byte   0x1      /* opcode: 0xc has 1 args */
+        .byte   0        /* End directory table */
+        .ascii "dw2-rel-hi-pc-hello.c\0"       /* File Entry: 0x1 */
+        .uleb128 0
+        .uleb128 0
+        .uleb128 0
+        .byte   0        /* End file name table */
+LELTP:
+        .byte   0        /* set address to .hello_start */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .hello_start
+        .byte   0x3      /* DW_LNS_advance_line */
+        .sleb128 22      /* ... to 23 */
+        .byte   0x5      /* column 0 */
+        .uleb128 0       /* 0 */
+        .byte           1       /* DW_LNS_copy */
+
+        .byte   0        /* set address to .hello0 */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .hello0
+        .byte   0x3      /* DW_LNS_advance_line */
+        .sleb128 1       /* ... to 24 */
+        .byte   0x5      /* column 0 */
+        .uleb128 0       /* 0 */
+        .byte           1       /* DW_LNS_copy */
+
+        .byte   0        /* set address to .hello1 */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .hello1
+        .byte   0x3      /* DW_LNS_advance_line */
+        .sleb128 1       /* ... to 25 */
+        .byte   0x5      /* column 0 */
+        .uleb128 0       /* 0 */
+        .byte           1       /* DW_LNS_copy */
+
+        .byte   0        /* set address to .hello_end */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .hello_end
+        .byte   0        /* end sequence */
+        .uleb128 0x1
+        .byte   0x1
+LELT:
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello.c b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello.c
new file mode 100644
index 0000000..59c761d
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello.c
@@ -0,0 +1,28 @@
+/* Copyright 2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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, see <http://www.gnu.org/licenses/>.  */
+
+extern int v;
+
+asm (".hello_start: .globl .hello_start\n");
+void
+hello (void)
+{
+asm (".hello0: .globl .hello0\n");
+  v++;
+asm (".hello1: .globl .hello1\n");
+}
+asm (".hello_end: .globl .hello_end\n");
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S
new file mode 100644
index 0000000..0d3a3d2
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S
@@ -0,0 +1,151 @@
+/* Copyright 2014 Free Software Foundation, Inc.
+
+   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, see <http://www.gnu.org/licenses/>.  */
+
+	.section	.debug_info,"",@progbits
+	.4byte	.Ledebug_info0 - .Lsdebug_info0	/* Length of Compilation Unit Info */
+.Lsdebug_info0:
+	.value	0x2	/* DWARF version number */
+	.4byte	.Ldebug_abbrev0	/* Offset Into Abbrev. Section */
+	.byte	0x4	/* Pointer Size (in bytes) */
+	.uleb128 0x1	/* (DIE (0xb) DW_TAG_compile_unit) */
+        .ascii "GNU C 4.7.4 20140206 for GNAT Pro 7.3.0w (20140206)\0"
+	.byte	0x1	/* DW_AT_language */
+	.ascii	"dw2-rel-hi-pc-world.c\0"     /* DW_AT_name */
+	.ascii  "/tmp\0"        /* DW_AT_comp_dir */
+	.4byte	.world_start	/* DW_AT_low_pc */
+	.4byte	.world_end	/* DW_AT_high_pc */
+	.4byte	.Ldebug_line0	/* DW_AT_stmt_list */
+	.uleb128 0x2	/* (DIE (0x2d) DW_TAG_subprogram) */
+	.byte	0x1	/* DW_AT_external */
+	.ascii	"world\0"
+	.byte	0x1	/* DW_AT_decl_file (world.c) */
+	.byte	0x13	/* DW_AT_decl_line */
+	.byte	0x1	/* DW_AT_prototyped */
+	.4byte	.world_start	/* DW_AT_low_pc */
+	.4byte	.world_end	/* DW_AT_high_pc */
+	.byte	0	/* end of children of DIE 0xb */
+.Ledebug_info0:
+
+	.section	.debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+	.uleb128 0x1	/* (abbrev code) */
+	.uleb128 0x11	/* (TAG: DW_TAG_compile_unit) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x25	/* (DW_AT_producer) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x13	/* (DW_AT_language) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x1b	/* (DW_AT_comp_dir) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x1	/* (DW_FORM_addr) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x10	/* (DW_AT_stmt_list) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.byte	0
+	.byte	0
+	.uleb128 0x2	/* (abbrev code) */
+	.uleb128 0x2e	/* (TAG: DW_TAG_subprogram) */
+	.byte	0	/* DW_children_no */
+	.uleb128 0x3f	/* (DW_AT_external) */
+	.uleb128 0xc	/* (DW_FORM_flag) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x3a	/* (DW_AT_decl_file) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3b	/* (DW_AT_decl_line) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x27	/* (DW_AT_prototyped) */
+	.uleb128 0xc	/* (DW_FORM_flag) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x1	/* (DW_FORM_addr) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.byte	0
+	.byte	0
+	.byte	0
+
+	.section	.debug_line,"",@progbits
+.Ldebug_line0:
+        .4byte  LELT-LSLT  /* Length of Source Line Info */
+LSLT:
+        .2byte  0x2    /* DWARF Version */
+        .4byte  LELTP-LASLTP     /* Prolog Length */
+LASLTP:
+        .byte   0x1      /* Minimum Instruction Length */
+        .byte   0x1      /* Default is_stmt_start flag */
+        .byte   0x1      /* Line Base Value (Special Opcodes) */
+        .byte   0x1      /* Line Range Value (Special Opcodes) */
+        .byte   0xd      /* Special Opcode Base */
+        .byte   0        /* opcode: 0x1 has 0 args */
+        .byte   0x1      /* opcode: 0x2 has 1 args */
+        .byte   0x1      /* opcode: 0x3 has 1 args */
+        .byte   0x1      /* opcode: 0x4 has 1 args */
+        .byte   0x1      /* opcode: 0x5 has 1 args */
+        .byte   0        /* opcode: 0x6 has 0 args */
+        .byte   0        /* opcode: 0x7 has 0 args */
+        .byte   0        /* opcode: 0x8 has 0 args */
+        .byte   0x1      /* opcode: 0x9 has 1 args */
+        .byte   0        /* opcode: 0xa has 0 args */
+        .byte   0        /* opcode: 0xb has 0 args */
+        .byte   0x1      /* opcode: 0xc has 1 args */
+        .byte   0        /* End directory table */
+        .ascii "dw2-rel-hi-pc-world.c\0"       /* File Entry: 0x1 */
+        .uleb128 0
+        .uleb128 0
+        .uleb128 0
+        .byte   0        /* End file name table */
+LELTP:
+        .byte   0        /* set address to .world_start */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .world_start
+        .byte   0x3      /* DW_LNS_advance_line */
+        .sleb128 22      /* ... to 23 */
+        .byte   0x5      /* column 0 */
+        .uleb128 0       /* 0 */
+        .byte           1       /* DW_LNS_copy */
+
+        .byte   0        /* set address to .world0 */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .world0
+        .byte   0x3      /* DW_LNS_advance_line */
+        .sleb128 1       /* ... to 24 */
+        .byte   0x5      /* column 0 */
+        .uleb128 0       /* 0 */
+        .byte           1       /* DW_LNS_copy */
+
+        .byte   0        /* set address to .world1 */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .world1
+        .byte   0x3      /* DW_LNS_advance_line */
+        .sleb128 1       /* ... to 25 */
+        .byte   0x5      /* column 0 */
+        .uleb128 0       /* 0 */
+        .byte           1       /* DW_LNS_copy */
+
+        .byte   0        /* set address to .world_end */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .world_end
+        .byte   0        /* end sequence */
+        .uleb128 0x1
+        .byte   0x1
+LELT:
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world.c b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world.c
new file mode 100644
index 0000000..04f17bd
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world.c
@@ -0,0 +1,28 @@
+/* Copyright 2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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, see <http://www.gnu.org/licenses/>.  */
+
+extern int v;
+
+asm (".world_start: .globl .world_start\n");
+void
+world (void)
+{
+asm (".world0: .globl .world0\n");
+  v++;
+asm (".world1: .globl .world1\n");
+}
+asm (".world_end: .globl .world_end\n");
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.c b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.c
new file mode 100644
index 0000000..95773a2
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.c
@@ -0,0 +1,28 @@
+/* Copyright 2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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, see <http://www.gnu.org/licenses/>.  */
+
+extern void hello (void);
+extern void world (void);
+
+int v;
+
+int
+main (void)
+{
+  hello ();
+  world ();
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.exp b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.exp
new file mode 100644
index 0000000..9090eb5
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.exp
@@ -0,0 +1,36 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# 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, see <http://www.gnu.org/licenses/>.
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+standard_testfile
+set executable ${testfile}
+
+if {[build_executable ${testfile}.exp ${executable} "${testfile}.c ${testfile}-hello-dbg.S ${testfile}-hello.c ${testfile}-world-dbg.S ${testfile}-world.c"] == -1} {
+    return -1
+}
+
+clean_restart $executable
+
+gdb_test "break hello" \
+    "Breakpoint $decimal at $hex: file .*dw2-rel-hi-pc-hello\\.c, line 24\\."
+
+
+gdb_test "break world" \
+    "Breakpoint $decimal at $hex: file .*dw2-rel-hi-pc-world\\.c, line 24\\."
-- 
1.8.3.2

Re: [RFA/DWARF] constant class of DW_AT_high_pc is offset for version >=4 only.

[Mark Wielaard]

Hi Joel,

On Sat, 2014-02-15 at 19:40 +0400, Joel Brobecker wrote:
> Starting with DWARF version 4, the description of the DW_AT_high_pc
> attribute was amended to say:
> 
>    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.
> 
> A change was made in Apr 27th, 2012 to reflect that change:
> 
>   | commit 91da14142c0171e58a91ad58a32fd010b700e761
>   | Author: Mark Wielaard <mjw@redhat.com>
>   | Date:   Fri Apr 27 18:55:19 2012 +0000
>   |
>   |     * dwarf2read.c (dwarf2_get_pc_bounds): Check DW_AT_high_pc form to
>   |     see whether it is an address or a constant offset from DW_AT_low_pc.
>   |     (dwarf2_record_block_ranges): Likewise.
>   |     (read_partial_die): Likewise.
> 
> Unfortunately, this new interpretation is now used regardless of
> the CU's DWARF version. It turns out that one of WindRiver's compilers
> (FTR: Diabdata 4.4) is generating DWARF version 2 info with
> DW_AT_high_pc attributes using the data4 form. Because of that,
> we miscompute all high PCs incorrectly.

Sorry about that. I read the spec as disallowing anything except
DW_FORM_addr to encode class address. And since before DWARF4
DW_AT_high_pc only allowed class address it hadn't occurred to me that
it could have been encoded differently. I do think this really is a bug
in the compiler that generated this.

> This patch fixes it by ammending a bit the changes from the commit
> mentioned above to use the new interpretation of a relative address
> only when the CU's DWARF version is 4 or greater.

That should work fine. GCC at least doesn't generate DW_AT_high_pc
attributes as offsets (constant class) unless it generates DWARF4+.

Cheers,

Mark

Re: [RFA/DWARF] constant class of DW_AT_high_pc is offset for version >=4 only.

[Joel Brobecker]

Hi Mark,

> Sorry about that. I read the spec as disallowing anything except
> DW_FORM_addr to encode class address. And since before DWARF4
> DW_AT_high_pc only allowed class address it hadn't occurred to me that
> it could have been encoded differently. I do think this really is a bug
> in the compiler that generated this.

Not to worry! I did find the use of data4 for a high_pc to be unusual,
for sure, and had to read the DWARF standard a couple of times to make
sure there was no such restriction. I'm pretty sure that I would have
made the same decision.

> > This patch fixes it by ammending a bit the changes from the commit
> > mentioned above to use the new interpretation of a relative address
> > only when the CU's DWARF version is 4 or greater.
> 
> That should work fine. GCC at least doesn't generate DW_AT_high_pc
> attributes as offsets (constant class) unless it generates DWARF4+.

Yes, I forgot to mention that I double-checked that it wouldn't
break GCC-compiled binaries.

Thanks!
-- 
Joel

PS: Since I have you - do you think binutils would need the same
    sort of change? I thought so too, when I looked at the code,
    but I couldn't get readelf to print the wrong address. Do you
    happen to know?

Re: [RFA/DWARF] constant class of DW_AT_high_pc is offset for version >=4 only.

[Joel Brobecker]

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

Hello,

Testing reveals that the test failed on some platforms:

        (gdb) b world
        Breakpoint 1 at 0x4005c4

Upon investigating it, it turns out that we were accessing the value
of the attribute using:

    high = DW_ADDR (attr_high);

This is not correct, since DW_ADDR assumes the attribute's form
is an address, whereas the test was using a data form, which means
we should have been using DW_UNSND. On some platforms, DW_ADDR
happens to work, but that's only pure luck related to the two
components being stored within the same union. But on ppc-linux,
the attribute value was stored in the DW_UNSND component, and
reading it back using DW_ADDR returned zero/garbage.

In addition to revealing a bug in my patch, this revealed a slightly
more general issue, which is the assumption that address attributes
are always encoded using address forms. So, I amended my patch to
read addresses in a way that takes the attribute's form into account.
Apart from increasing the number of edits, all mostly mechanical,
I find that it simplified the handling for DW_AT_high_pc quite
a bit, thanks to the introduction of a function getting the attribute
value.

Changes compared to the previous version:

  1. The introduction of attr_value_as_address, to be used
     in place of DW_ADDR when dealing with address attributes.
     I left a few uses of this macro in the situations where
     we actually know that the form is an address form.

  2. Instead of duplicating everywhere the conditions for
     non-address forms in the handling of DW_AT_high_pc
     attributes, I used the attr_form_is_constant function
     instead.  It's not stricly the same, but I think it is
     closer to the DWARF reference.

  3. Some very minor modifications of the assembly files in our
     testcase to make it compilable on a larger number of targets:
     Removing some of the extra parameters for the .section
     pseudo-op, as well as using .2byte instead of .value.

  4. In our testcase, I also adjusted the DW_AT_low_pc attributes
     to be encoded using DW_FORM_data4, to be consistent with
     the DW_AT_high_pc attributes, as well as to test reading
     of that attribute in constant form.

gdb/ChangeLog:

        * dwarf2read.c (attr_value_as_address): New function.
        (dwarf2_find_base_address, read_call_site_scope): Use
        attr_value_as_address in place of DW_ADDR.
        (dwarf2_get_pc_bounds): Use attr_value_as_address to get
        the low and high addresses.  Slight rework of the handling
        of the high pc being a constant form, and limit it to
        DWARF verson 4 or higher.
        (dwarf2_record_block_ranges): Likewise.
        (read_partial_die): Likewise.
        (new_symbol_full): Use attr_value_as_address in place of DW_ADDR.

gdb/testsuite/ChangeLog:

        * gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S: New file.
        * gdb.dwarf2/dw2-abs-hi-pc-hello.c: New file.
        * gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S: New file.
        * gdb.dwarf2/dw2-abs-hi-pc-world.c: New file.
        * gdb.dwarf2/dw2-abs-hi-pc.c: New file.
        * gdb.dwarf2/dw2-abs-hi-pc.exp: New file.

Tested on x86_64-linux. The testcase was also testing on ppc-linux,
where it was failing prior to the second iteration of this patch.

OK to commit?

Thanks,
-- 
Joel

[-- Attachment #2: 0001-DWARF-Read-constant-class-addresses-correctly.patch --]
[-- Type: text/x-diff, Size: 26215 bytes --]

From 228e0cc1973bae4d1cc3c80582f4bf8acc71b482 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Sat, 15 Feb 2014 19:09:58 +0400
Subject: [PATCH] DWARF: Read constant-class addresses correctly

Starting with DWARF version 4, the description of the DW_AT_high_pc
attribute was amended to say:

   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.

A change was made in Apr 27th, 2012 to reflect that change:

  | commit 91da14142c0171e58a91ad58a32fd010b700e761
  | Author: Mark Wielaard <mjw@redhat.com>
  | Date:   Fri Apr 27 18:55:19 2012 +0000
  |
  |     * dwarf2read.c (dwarf2_get_pc_bounds): Check DW_AT_high_pc form to
  |     see whether it is an address or a constant offset from DW_AT_low_pc.
  |     (dwarf2_record_block_ranges): Likewise.
  |     (read_partial_die): Likewise.

Unfortunately, this new interpretation is now used regardless of
the CU's DWARF version. It turns out that one of WindRiver's compilers
(FTR: Diabdata 4.4) is generating DWARF version 2 info with
DW_AT_high_pc attributes using the data4 form. Because of that,
we miscompute all high PCs incorrectly. This leads to a lot of
symtabs having overlapping ranges, which in turn causes havoc
in pc-to-symtab-and-line translations.

One visible effect is when inserting a breakpoint on a given function:

    (gdb) b world
    Breakpoint 1 at 0x4005c4

The source location of the breakpoint is missing. The output should be:

    (gdb) b world
    Breakpoint 1 at 0x4005c8: file dw2-rel-hi-pc-world.c, line 24.

What happens in this case is that the pc-to-SAL translation first
starts be trying to find the symtab associated to our PC using
each symtab's ranges. Because of the high_pc miscomputation,
many symtabs end up matching, and the heuristic trying to select
the most probable one unfortunately returns one that is unrelated
(it really had no change in this case to do any better). Once we
have the wrong symtab, the start searching the associated linetable,
where the addresses are correct, thus finding no match, and therefore
no SAL.

More generally, this issue revealed that GDB makes the implicit
assumption that all address attributes are always encoded using
an address form (because we use the DW_ADDR macro to access the
attribute value, regardless of its form).

This patch introduces a new function "attr_value_as_address" which
uses the correct accessor for getting the value of a given attribute.
It then adjust the code throughout this unit to use this function
instead of assuming that addresses always have the DW_FORM_addr
format.  In particular, this allows us to handle attributes such
as DW_AT_low_pc and DW_AT_entry_pc correctly when using constant
forms.

It also fixes the original issue of miscomputing the high_pc
by limiting the new interpretation of constant form DW_AT_high_pc
attributes to units using DWARF version 4 or later.

gdb/ChangeLog:

        * dwarf2read.c (attr_value_as_address): New function.
        (dwarf2_find_base_address, read_call_site_scope): Use
        attr_value_as_address in place of DW_ADDR.
        (dwarf2_get_pc_bounds): Use attr_value_as_address to get
        the low and high addresses.  Slight rework of the handling
        of the high pc being a constant form, and limit it to
        DWARF verson 4 or higher.
        (dwarf2_record_block_ranges): Likewise.
        (read_partial_die): Likewise.
        (new_symbol_full): Use attr_value_as_address in place of DW_ADDR.

gdb/testsuite/ChangeLog:

        * gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S: New file.
        * gdb.dwarf2/dw2-abs-hi-pc-hello.c: New file.
        * gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S: New file.
        * gdb.dwarf2/dw2-abs-hi-pc-world.c: New file.
        * gdb.dwarf2/dw2-abs-hi-pc.c: New file.
        * gdb.dwarf2/dw2-abs-hi-pc.exp: New file.

Tested on x86_64-linux.
---
 gdb/dwarf2read.c                                   |  62 +++++----
 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S | 151 +++++++++++++++++++++
 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello.c     |  28 ++++
 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S | 151 +++++++++++++++++++++
 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world.c     |  28 ++++
 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.c           |  28 ++++
 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.exp         |  36 +++++
 7 files changed, 456 insertions(+), 28 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.exp

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 54c538a..999765b 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1939,6 +1939,22 @@ byte_swap (offset_type value)
 #define MAYBE_SWAP(V) (V)
 #endif /* WORDS_BIGENDIAN */
 
+/* Read the given attribute value as an address, taking the attribute's
+   form into account.  */
+
+static CORE_ADDR
+attr_value_as_address (struct attribute *attr)
+{
+  CORE_ADDR addr;
+
+  if (attr->form != DW_FORM_addr && attr->form != DW_FORM_GNU_addr_index)
+    addr = DW_UNSND (attr);
+  else
+    addr = DW_ADDR (attr);
+
+  return addr;
+}
+
 /* The suffix for an index file.  */
 #define INDEX_SUFFIX ".gdb-index"
 
@@ -4201,7 +4217,7 @@ dwarf2_find_base_address (struct die_info *die, struct dwarf2_cu *cu)
   attr = dwarf2_attr (die, DW_AT_entry_pc, cu);
   if (attr)
     {
-      cu->base_address = DW_ADDR (attr);
+      cu->base_address = attr_value_as_address (attr);
       cu->base_known = 1;
     }
   else
@@ -4209,7 +4225,7 @@ dwarf2_find_base_address (struct die_info *die, struct dwarf2_cu *cu)
       attr = dwarf2_attr (die, DW_AT_low_pc, cu);
       if (attr)
 	{
-	  cu->base_address = DW_ADDR (attr);
+	  cu->base_address = attr_value_as_address (attr);
 	  cu->base_known = 1;
 	}
     }
@@ -11233,7 +11249,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
 		 die->offset.sect_off, objfile_name (objfile));
       return;
     }
-  pc = DW_ADDR (attr) + baseaddr;
+  pc = attr_value_as_address (attr) + baseaddr;
 
   if (cu->call_site_htab == NULL)
     cu->call_site_htab = htab_create_alloc_ex (16, core_addr_hash, core_addr_eq,
@@ -11674,12 +11690,10 @@ dwarf2_get_pc_bounds (struct die_info *die, CORE_ADDR *lowpc,
       attr = dwarf2_attr (die, DW_AT_low_pc, cu);
       if (attr)
         {
-	  low = DW_ADDR (attr);
-	  if (attr_high->form == DW_FORM_addr
-	      || attr_high->form == DW_FORM_GNU_addr_index)
-	    high = DW_ADDR (attr_high);
-	  else
-	    high = low + DW_UNSND (attr_high);
+	  low = attr_value_as_address (attr);
+	  high = attr_value_as_address (attr_high);
+	  if (cu->header.version >= 4 && attr_form_is_constant (attr_high))
+	    high += low;
 	}
       else
 	/* Found high w/o low attribute.  */
@@ -11845,13 +11859,11 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
       attr = dwarf2_attr (die, DW_AT_low_pc, cu);
       if (attr)
         {
-          CORE_ADDR low = DW_ADDR (attr);
-	  CORE_ADDR high;
-	  if (attr_high->form == DW_FORM_addr
-	      || attr_high->form == DW_FORM_GNU_addr_index)
-	    high = DW_ADDR (attr_high);
-	  else
-	    high = low + DW_UNSND (attr_high);
+          CORE_ADDR low = attr_value_as_address (attr);
+	  CORE_ADDR high = attr_value_as_address (attr_high);
+
+	  if (cu->header.version >= 4 && attr_form_is_constant (attr_high))
+	    high += low;
 
           record_block_range (block, baseaddr + low, baseaddr + high - 1);
         }
@@ -15240,18 +15252,13 @@ read_partial_die (const struct die_reader_specs *reader,
 	  break;
 	case DW_AT_low_pc:
 	  has_low_pc_attr = 1;
-	  part_die->lowpc = DW_ADDR (&attr);
+	  part_die->lowpc = attr_value_as_address (&attr);
 	  break;
 	case DW_AT_high_pc:
 	  has_high_pc_attr = 1;
-	  if (attr.form == DW_FORM_addr
-	      || attr.form == DW_FORM_GNU_addr_index)
-	    part_die->highpc = DW_ADDR (&attr);
-	  else
-	    {
-	      high_pc_relative = 1;
-	      part_die->highpc = DW_UNSND (&attr);
-	    }
+	  part_die->highpc = attr_value_as_address (&attr);
+	  if (cu->header.version >= 4 && attr_form_is_constant (&attr))
+		high_pc_relative = 1;
 	  break;
 	case DW_AT_location:
           /* Support the .debug_loc offsets.  */
@@ -17464,9 +17471,8 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	case DW_TAG_label:
 	  attr = dwarf2_attr (die, DW_AT_low_pc, cu);
 	  if (attr)
-	    {
-	      SYMBOL_VALUE_ADDRESS (sym) = DW_ADDR (attr) + baseaddr;
-	    }
+	    SYMBOL_VALUE_ADDRESS (sym)
+	      = attr_value_as_address (attr) + baseaddr;
 	  SYMBOL_TYPE (sym) = objfile_type (objfile)->builtin_core_addr;
 	  SYMBOL_DOMAIN (sym) = LABEL_DOMAIN;
 	  SYMBOL_ACLASS_INDEX (sym) = LOC_LABEL;
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S
new file mode 100644
index 0000000..03f9442
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S
@@ -0,0 +1,151 @@
+/* Copyright 2014 Free Software Foundation, Inc.
+
+   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, see <http://www.gnu.org/licenses/>.  */
+
+	.section	.debug_info
+	.4byte	.Ledebug_info0 - .Lsdebug_info0	/* Length of Compilation Unit Info */
+.Lsdebug_info0:
+	.2byte	0x2	/* DWARF version number */
+	.4byte	.Ldebug_abbrev0	/* Offset Into Abbrev. Section */
+	.byte	0x4	/* Pointer Size (in bytes) */
+	.uleb128 0x1	/* (DIE (0xb) DW_TAG_compile_unit) */
+        .ascii "GNU C 4.7.4 20140206 for GNAT Pro 7.3.0w (20140206)\0"
+	.byte	0x1	/* DW_AT_language */
+	.ascii	"dw2-abs-hi-pc-hello.c\0"     /* DW_AT_name */
+	.ascii  "/tmp\0"        /* DW_AT_comp_dir */
+	.4byte	.hello_start	/* DW_AT_low_pc */
+	.4byte	.hello_end	/* DW_AT_high_pc */
+	.4byte	.Ldebug_line0	/* DW_AT_stmt_list */
+	.uleb128 0x2	/* (DIE (0x2d) DW_TAG_subprogram) */
+	.byte	0x1	/* DW_AT_external */
+	.ascii	"hello\0"
+	.byte	0x1	/* DW_AT_decl_file (hello.c) */
+	.byte	0x13	/* DW_AT_decl_line */
+	.byte	0x1	/* DW_AT_prototyped */
+	.4byte	.hello_start	/* DW_AT_low_pc */
+	.4byte	.hello_end	/* DW_AT_high_pc */
+	.byte	0	/* end of children of DIE 0xb */
+.Ledebug_info0:
+
+	.section	.debug_abbrev
+.Ldebug_abbrev0:
+	.uleb128 0x1	/* (abbrev code) */
+	.uleb128 0x11	/* (TAG: DW_TAG_compile_unit) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x25	/* (DW_AT_producer) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x13	/* (DW_AT_language) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x1b	/* (DW_AT_comp_dir) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x10	/* (DW_AT_stmt_list) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.byte	0
+	.byte	0
+	.uleb128 0x2	/* (abbrev code) */
+	.uleb128 0x2e	/* (TAG: DW_TAG_subprogram) */
+	.byte	0	/* DW_children_no */
+	.uleb128 0x3f	/* (DW_AT_external) */
+	.uleb128 0xc	/* (DW_FORM_flag) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x3a	/* (DW_AT_decl_file) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3b	/* (DW_AT_decl_line) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x27	/* (DW_AT_prototyped) */
+	.uleb128 0xc	/* (DW_FORM_flag) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.byte	0
+	.byte	0
+	.byte	0
+
+	.section	.debug_line
+.Ldebug_line0:
+        .4byte  LELT-LSLT  /* Length of Source Line Info */
+LSLT:
+        .2byte  0x2    /* DWARF Version */
+        .4byte  LELTP-LASLTP     /* Prolog Length */
+LASLTP:
+        .byte   0x1      /* Minimum Instruction Length */
+        .byte   0x1      /* Default is_stmt_start flag */
+        .byte   0x1      /* Line Base Value (Special Opcodes) */
+        .byte   0x1      /* Line Range Value (Special Opcodes) */
+        .byte   0xd      /* Special Opcode Base */
+        .byte   0        /* opcode: 0x1 has 0 args */
+        .byte   0x1      /* opcode: 0x2 has 1 args */
+        .byte   0x1      /* opcode: 0x3 has 1 args */
+        .byte   0x1      /* opcode: 0x4 has 1 args */
+        .byte   0x1      /* opcode: 0x5 has 1 args */
+        .byte   0        /* opcode: 0x6 has 0 args */
+        .byte   0        /* opcode: 0x7 has 0 args */
+        .byte   0        /* opcode: 0x8 has 0 args */
+        .byte   0x1      /* opcode: 0x9 has 1 args */
+        .byte   0        /* opcode: 0xa has 0 args */
+        .byte   0        /* opcode: 0xb has 0 args */
+        .byte   0x1      /* opcode: 0xc has 1 args */
+        .byte   0        /* End directory table */
+        .ascii "dw2-abs-hi-pc-hello.c\0"       /* File Entry: 0x1 */
+        .uleb128 0
+        .uleb128 0
+        .uleb128 0
+        .byte   0        /* End file name table */
+LELTP:
+        .byte   0        /* set address to .hello_start */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .hello_start
+        .byte   0x3      /* DW_LNS_advance_line */
+        .sleb128 22      /* ... to 23 */
+        .byte   0x5      /* column 0 */
+        .uleb128 0       /* 0 */
+        .byte           1       /* DW_LNS_copy */
+
+        .byte   0        /* set address to .hello0 */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .hello0
+        .byte   0x3      /* DW_LNS_advance_line */
+        .sleb128 1       /* ... to 24 */
+        .byte   0x5      /* column 0 */
+        .uleb128 0       /* 0 */
+        .byte           1       /* DW_LNS_copy */
+
+        .byte   0        /* set address to .hello1 */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .hello1
+        .byte   0x3      /* DW_LNS_advance_line */
+        .sleb128 1       /* ... to 25 */
+        .byte   0x5      /* column 0 */
+        .uleb128 0       /* 0 */
+        .byte           1       /* DW_LNS_copy */
+
+        .byte   0        /* set address to .hello_end */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .hello_end
+        .byte   0        /* end sequence */
+        .uleb128 0x1
+        .byte   0x1
+LELT:
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello.c b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello.c
new file mode 100644
index 0000000..59c761d
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello.c
@@ -0,0 +1,28 @@
+/* Copyright 2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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, see <http://www.gnu.org/licenses/>.  */
+
+extern int v;
+
+asm (".hello_start: .globl .hello_start\n");
+void
+hello (void)
+{
+asm (".hello0: .globl .hello0\n");
+  v++;
+asm (".hello1: .globl .hello1\n");
+}
+asm (".hello_end: .globl .hello_end\n");
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S
new file mode 100644
index 0000000..c3e89a6
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S
@@ -0,0 +1,151 @@
+/* Copyright 2014 Free Software Foundation, Inc.
+
+   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, see <http://www.gnu.org/licenses/>.  */
+
+	.section	.debug_info
+	.4byte	.Ledebug_info0 - .Lsdebug_info0	/* Length of Compilation Unit Info */
+.Lsdebug_info0:
+	.2byte	0x2	/* DWARF version number */
+	.4byte	.Ldebug_abbrev0	/* Offset Into Abbrev. Section */
+	.byte	0x4	/* Pointer Size (in bytes) */
+	.uleb128 0x1	/* (DIE (0xb) DW_TAG_compile_unit) */
+        .ascii "GNU C 4.7.4 20140206 for GNAT Pro 7.3.0w (20140206)\0"
+	.byte	0x1	/* DW_AT_language */
+	.ascii	"dw2-abs-hi-pc-world.c\0"     /* DW_AT_name */
+	.ascii  "/tmp\0"        /* DW_AT_comp_dir */
+	.4byte	.world_start	/* DW_AT_low_pc */
+	.4byte	.world_end	/* DW_AT_high_pc */
+	.4byte	.Ldebug_line0	/* DW_AT_stmt_list */
+	.uleb128 0x2	/* (DIE (0x2d) DW_TAG_subprogram) */
+	.byte	0x1	/* DW_AT_external */
+	.ascii	"world\0"
+	.byte	0x1	/* DW_AT_decl_file (world.c) */
+	.byte	0x13	/* DW_AT_decl_line */
+	.byte	0x1	/* DW_AT_prototyped */
+	.4byte	.world_start	/* DW_AT_low_pc */
+	.4byte	.world_end	/* DW_AT_high_pc */
+	.byte	0	/* end of children of DIE 0xb */
+.Ledebug_info0:
+
+	.section	.debug_abbrev
+.Ldebug_abbrev0:
+	.uleb128 0x1	/* (abbrev code) */
+	.uleb128 0x11	/* (TAG: DW_TAG_compile_unit) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x25	/* (DW_AT_producer) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x13	/* (DW_AT_language) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x1b	/* (DW_AT_comp_dir) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x10	/* (DW_AT_stmt_list) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.byte	0
+	.byte	0
+	.uleb128 0x2	/* (abbrev code) */
+	.uleb128 0x2e	/* (TAG: DW_TAG_subprogram) */
+	.byte	0	/* DW_children_no */
+	.uleb128 0x3f	/* (DW_AT_external) */
+	.uleb128 0xc	/* (DW_FORM_flag) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x3a	/* (DW_AT_decl_file) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3b	/* (DW_AT_decl_line) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x27	/* (DW_AT_prototyped) */
+	.uleb128 0xc	/* (DW_FORM_flag) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.byte	0
+	.byte	0
+	.byte	0
+
+	.section	.debug_line
+.Ldebug_line0:
+        .4byte  LELT-LSLT  /* Length of Source Line Info */
+LSLT:
+        .2byte  0x2    /* DWARF Version */
+        .4byte  LELTP-LASLTP     /* Prolog Length */
+LASLTP:
+        .byte   0x1      /* Minimum Instruction Length */
+        .byte   0x1      /* Default is_stmt_start flag */
+        .byte   0x1      /* Line Base Value (Special Opcodes) */
+        .byte   0x1      /* Line Range Value (Special Opcodes) */
+        .byte   0xd      /* Special Opcode Base */
+        .byte   0        /* opcode: 0x1 has 0 args */
+        .byte   0x1      /* opcode: 0x2 has 1 args */
+        .byte   0x1      /* opcode: 0x3 has 1 args */
+        .byte   0x1      /* opcode: 0x4 has 1 args */
+        .byte   0x1      /* opcode: 0x5 has 1 args */
+        .byte   0        /* opcode: 0x6 has 0 args */
+        .byte   0        /* opcode: 0x7 has 0 args */
+        .byte   0        /* opcode: 0x8 has 0 args */
+        .byte   0x1      /* opcode: 0x9 has 1 args */
+        .byte   0        /* opcode: 0xa has 0 args */
+        .byte   0        /* opcode: 0xb has 0 args */
+        .byte   0x1      /* opcode: 0xc has 1 args */
+        .byte   0        /* End directory table */
+        .ascii "dw2-abs-hi-pc-world.c\0"       /* File Entry: 0x1 */
+        .uleb128 0
+        .uleb128 0
+        .uleb128 0
+        .byte   0        /* End file name table */
+LELTP:
+        .byte   0        /* set address to .world_start */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .world_start
+        .byte   0x3      /* DW_LNS_advance_line */
+        .sleb128 22      /* ... to 23 */
+        .byte   0x5      /* column 0 */
+        .uleb128 0       /* 0 */
+        .byte           1       /* DW_LNS_copy */
+
+        .byte   0        /* set address to .world0 */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .world0
+        .byte   0x3      /* DW_LNS_advance_line */
+        .sleb128 1       /* ... to 24 */
+        .byte   0x5      /* column 0 */
+        .uleb128 0       /* 0 */
+        .byte           1       /* DW_LNS_copy */
+
+        .byte   0        /* set address to .world1 */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .world1
+        .byte   0x3      /* DW_LNS_advance_line */
+        .sleb128 1       /* ... to 25 */
+        .byte   0x5      /* column 0 */
+        .uleb128 0       /* 0 */
+        .byte           1       /* DW_LNS_copy */
+
+        .byte   0        /* set address to .world_end */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .world_end
+        .byte   0        /* end sequence */
+        .uleb128 0x1
+        .byte   0x1
+LELT:
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world.c b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world.c
new file mode 100644
index 0000000..04f17bd
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world.c
@@ -0,0 +1,28 @@
+/* Copyright 2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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, see <http://www.gnu.org/licenses/>.  */
+
+extern int v;
+
+asm (".world_start: .globl .world_start\n");
+void
+world (void)
+{
+asm (".world0: .globl .world0\n");
+  v++;
+asm (".world1: .globl .world1\n");
+}
+asm (".world_end: .globl .world_end\n");
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.c b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.c
new file mode 100644
index 0000000..95773a2
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.c
@@ -0,0 +1,28 @@
+/* Copyright 2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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, see <http://www.gnu.org/licenses/>.  */
+
+extern void hello (void);
+extern void world (void);
+
+int v;
+
+int
+main (void)
+{
+  hello ();
+  world ();
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.exp b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.exp
new file mode 100644
index 0000000..79f1556
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.exp
@@ -0,0 +1,36 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# 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, see <http://www.gnu.org/licenses/>.
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+standard_testfile
+set executable ${testfile}
+
+if {[build_executable ${testfile}.exp ${executable} "${testfile}.c ${testfile}-hello-dbg.S ${testfile}-hello.c ${testfile}-world-dbg.S ${testfile}-world.c"] == -1} {
+    return -1
+}
+
+clean_restart $executable
+
+gdb_test "break hello" \
+    "Breakpoint $decimal at $hex: file .*dw2-abs-hi-pc-hello\\.c, line 24\\."
+
+
+gdb_test "break world" \
+    "Breakpoint $decimal at $hex: file .*dw2-abs-hi-pc-world\\.c, line 24\\."
-- 
1.8.3.2

Re: [RFA/DWARF] constant class of DW_AT_high_pc is offset for version >=4 only.

[Mark Wielaard]

On Tue, 2014-02-18 at 14:30 +0100, Joel Brobecker wrote:
>   1. The introduction of attr_value_as_address, to be used
>      in place of DW_ADDR when dealing with address attributes.
>      I left a few uses of this macro in the situations where
>      we actually know that the form is an address form.

This accepts any form as address/unsigned. I would at least check that
it is either DW_FORM_data4 or DW_FORM_data8 (even better would be to
check the CU address width too, although that would require to pass
around cu too, which might not be practical). Also I would add a comment
that this is really to work around buggy producers.

>   2. Instead of duplicating everywhere the conditions for
>      non-address forms in the handling of DW_AT_high_pc
>      attributes, I used the attr_form_is_constant function
>      instead.  It's not stricly the same, but I think it is
>      closer to the DWARF reference.

It is closer when used like you do, combined with a check for
cu->header.version >= 4.

> @@ -4201,7 +4217,7 @@ dwarf2_find_base_address (struct die_info *die, struct dwarf2_cu *cu)
>    attr = dwarf2_attr (die, DW_AT_entry_pc, cu);
>    if (attr)
>      {
> -      cu->base_address = DW_ADDR (attr);
> +      cu->base_address = attr_value_as_address (attr);
>        cu->base_known = 1;
>      }

Note that this might break for DWARF5. See http://dwarfstd.org/ShowIssue.php?issue=120719.1

In general I would only use attr_value_as_address for attributes (low_pc
and high_pc) which you know a buggy producer might encode with
DW_FORM_data[48].

Cheers,

Mark.

Re: [RFA/DWARF] constant class of DW_AT_high_pc is offset for version >=4 only.

[Joel Brobecker]

Hi Mark,

First of all, thanks for the comments!

On Tue, Feb 18, 2014 at 05:02:49PM +0100, Mark Wielaard wrote:
> On Tue, 2014-02-18 at 14:30 +0100, Joel Brobecker wrote:
> >   1. The introduction of attr_value_as_address, to be used
> >      in place of DW_ADDR when dealing with address attributes.
> >      I left a few uses of this macro in the situations where
> >      we actually know that the form is an address form.
> 
> This accepts any form as address/unsigned. I would at least check that
> it is either DW_FORM_data4 or DW_FORM_data8 (even better would be to
> check the CU address width too, although that would require to pass
> around cu too, which might not be practical).

I don't mind adding that, but the real question is what would we do
if we found an unexpected size? We could emit a complaint, but
I wouldn't necessarily stop processing the unit's debugging info.
I think the guideline here is that we should try to do our best
within reasonable constraints. I think adding a complaint, here,
is of marginal value as presumably the address read in the odd
format might be correct. On the other hand, any dump of the debugging
info should quickly reveal the format used for those addresses.

> Also I would add a comment
> that this is really to work around buggy producers.

I will definitely add a comment saying something about some compiler
producing odd debugging info. It seems logical to me that address
attributes would naturally be encoded using an address form, but is
a constant form clearly forbidden by the (older) standard(s)?
I would therefore just label the format chosen as unusual. :-).

> > @@ -4201,7 +4217,7 @@ dwarf2_find_base_address (struct die_info *die, struct dwarf2_cu *cu)
> >    attr = dwarf2_attr (die, DW_AT_entry_pc, cu);
> >    if (attr)
> >      {
> > -      cu->base_address = DW_ADDR (attr);
> > +      cu->base_address = attr_value_as_address (attr);
> >        cu->base_known = 1;
> >      }
> 
> Note that this might break for DWARF5. See http://dwarfstd.org/ShowIssue.php?issue=120719.1

Interesting. I am curious why you would handle this attribute as
an offset even when the value is encoded in address form? Would
that not help improve backward compatibility with older versions
of DWARF?

But regardless, I think my change doesn't break the current behavior;
and to support the DWARF5 standard, the behavior implemented by
the function is still correct (reading the data from the correct
union field). You will need to add some code right after the call,
regardless, which adds the base address if version >= 5.

> In general I would only use attr_value_as_address for attributes (low_pc
> and high_pc) which you know a buggy producer might encode with
> DW_FORM_data[48].

I think this suggestion goes against the spirit of trying to do our
best. Not using the function means reading the attribute value from
the wrong field of the attribute union, which means increasing our
chances of getting it wrong. On the other hand, using the function
might allow us to read the correct address and get things to actually
work.

Also, I don't have acces to said compiler, so I can't try to study
its output and list the attributes using this constant form. But
if the low/hi pc attributes use a constant form, why not the
entry_pc?

-- 
Joel

Re: [RFA/DWARF] constant class of DW_AT_high_pc is offset for version >=4 only.

[Doug Evans]

On Tue, Feb 18, 2014 at 10:49 AM, Joel Brobecker <brobecker@adacore.com> wrote:
>> Note that this might break for DWARF5. See http://dwarfstd.org/ShowIssue.php?issue=120719.1
>
> Interesting. I am curious why you would handle this attribute as
> an offset even when the value is encoded in address form? Would
> that not help improve backward compatibility with older versions
> of DWARF?

I can imagine this is another space saving measure (removes a
relocation, and relocations add up).

Re: [RFA/DWARF] constant class of DW_AT_high_pc is offset for version >=4 only.

[Mark Wielaard]

On Tue, 2014-02-18 at 19:49 +0100, Joel Brobecker wrote:
> First of all, thanks for the comments!

Just trying to make up for breaking your setup with my original patch.
Although I might be too pedantic in my DWARF spec reading and I cannot
actually approve the patch. So it might be of little help. Sorry about
that.

> On Tue, Feb 18, 2014 at 05:02:49PM +0100, Mark Wielaard wrote:
> > On Tue, 2014-02-18 at 14:30 +0100, Joel Brobecker wrote:
> > >   1. The introduction of attr_value_as_address, to be used
> > >      in place of DW_ADDR when dealing with address attributes.
> > >      I left a few uses of this macro in the situations where
> > >      we actually know that the form is an address form.
> > 
> > This accepts any form as address/unsigned. I would at least check that
> > it is either DW_FORM_data4 or DW_FORM_data8 (even better would be to
> > check the CU address width too, although that would require to pass
> > around cu too, which might not be practical).
> 
> I don't mind adding that, but the real question is what would we do
> if we found an unexpected size? We could emit a complaint, but
> I wouldn't necessarily stop processing the unit's debugging info.
> I think the guideline here is that we should try to do our best
> within reasonable constraints. I think adding a complaint, here,
> is of marginal value as presumably the address read in the odd
> format might be correct. On the other hand, any dump of the debugging
> info should quickly reveal the format used for those addresses.

I think emitting a complaint is the right thing to do. IMHO this really
is broken DWARF and accepting random DW_FORM_foo here might hide real
issues (which as you showed might accidentally seem to work just because
the union values overlap and produce something that is slightly but not
completely wrong).

> > Also I would add a comment
> > that this is really to work around buggy producers.
> 
> I will definitely add a comment saying something about some compiler
> producing odd debugging info. It seems logical to me that address
> attributes would naturally be encoded using an address form, but is
> a constant form clearly forbidden by the (older) standard(s)?
> I would therefore just label the format chosen as unusual. :-).

Sadly DWARF doesn't seem to forbid anything. If it doesn't seem to make
sense then a producer and consumer just have to come to an agreement
about the meaning somehow. But even DWARF2 says that the only possible
encoding of attribute values of class address is DW_FORM_addr.

> > Note that this might break for DWARF5. See http://dwarfstd.org/ShowIssue.php?issue=120719.1
> 
> Interesting. I am curious why you would handle this attribute as
> an offset even when the value is encoded in address form?

As Doug said, it is a space saving (offsets are often small) and it
saves a relocation (linkers have to resolve all DW_FORM_addr values and
they add up).

A more general form of this saving is the DW_FORM_GNU_addr_index
extension which is also proposed as a DWARF5 update:
http://dwarfstd.org/ShowIssue.php?issue=130313.2

> But regardless, I think my change doesn't break the current behavior;
> and to support the DWARF5 standard, the behavior implemented by
> the function is still correct (reading the data from the correct
> union field). You will need to add some code right after the call,
> regardless, which adds the base address if version >= 5.

Yes, agreed.

> > In general I would only use attr_value_as_address for attributes (low_pc
> > and high_pc) which you know a buggy producer might encode with
> > DW_FORM_data[48].
> 
> I think this suggestion goes against the spirit of trying to do our
> best. Not using the function means reading the attribute value from
> the wrong field of the attribute union, which means increasing our
> chances of getting it wrong. On the other hand, using the function
> might allow us to read the correct address and get things to actually
> work.

Be liberal in what you accept. Yeah. OK.

I admit I am mostly worried because GDB is seen as the gold standard of
DWARF consumers. When GDB accepts some DWARF then basically all other
DWARF consumers have to adapt. And in this case I had some trouble
recently with producers and consumers not agreeing on the meaning of
DW_FORM_data[1248] (which I still have to report to the DWARF
committee), so I am a little hyper-sensitive to accepting even more
stuff encoded as DW_FORM_data... sorry about that.

I do think your patch is basically fine. I am just pedantic about
interpreting the DWARF standard. Because I do worry this will make
things harder in the future.

> Also, I don't have acces to said compiler, so I can't try to study
> its output and list the attributes using this constant form. But
> if the low/hi pc attributes use a constant form, why not the
> entry_pc?

OK, that makes things even harder. But could you ask whether they encode
addresses always as DW_FORM_data[48] and whether an update will produce
DW_FORM_addr? It would be good to know if this is just historical and
will not be an issue in the future.

Thanks,

Mark

Re: [RFA/DWARF] constant class of DW_AT_high_pc is offset for version >=4 only.

[Joel Brobecker]

> Just trying to make up for breaking your setup with my original patch.
> Although I might be too pedantic in my DWARF spec reading and I cannot
> actually approve the patch. So it might be of little help. Sorry about
> that.

No - I think you're being very helpful, because you seem very
knowledgeable about present and future DWARF.

> Sadly DWARF doesn't seem to forbid anything. [...] But even DWARF2
> says that the only possible encoding of attribute values of class
> address is DW_FORM_addr.
[...]
> I admit I am mostly worried because GDB is seen as the gold standard of
> DWARF consumers. When GDB accepts some DWARF then basically all other
> DWARF consumers have to adapt.
[...]
> I am just pedantic about interpreting the DWARF standard. Because I do
> worry this will make things harder in the future.

OK, I see where you are coming from. In that case, I agree we should
be adding the complaint. The intention behind my patch then becomes:
Yes, we accept this format but its meaning is undefined. We interpret
it the best we can hoping that it may actually work in your case,
but no guarantees.

So, overall, the plan now is to adjust version #2 in the following
ways:
  - Add a comment in the function documentation explaining that
    this is to help trying to read broken DWARF;
  - Add a complaint inside the function when the attribute has
    the wrong format
The generalized usage of the new function is maintained.

> > > Note that this might break for DWARF5. See http://dwarfstd.org/ShowIssue.php?issue=120719.1
> > 
> > Interesting. I am curious why you would handle this attribute as
> > an offset even when the value is encoded in address form?
> 
> As Doug said, it is a space saving (offsets are often small) and it
> saves a relocation (linkers have to resolve all DW_FORM_addr values and
> they add up).

I was wondering why we don't "simply" require a constant format in
this case, instead of allowing both formats.

Thanks,
-- 
Joel

Re: [RFA/DWARF] constant class of DW_AT_high_pc is offset for version >=4 only.

[Mark Wielaard]

On Wed, 2014-02-19 at 08:23 +0100, Joel Brobecker wrote:
> So, overall, the plan now is to adjust version #2 in the following
> ways:
>   - Add a comment in the function documentation explaining that
>     this is to help trying to read broken DWARF;
>   - Add a complaint inside the function when the attribute has
>     the wrong format
> The generalized usage of the new function is maintained.

Sounds like a good plan.

> > > > Note that this might break for DWARF5. See http://dwarfstd.org/ShowIssue.php?issue=120719.1
> > > 
> > > Interesting. I am curious why you would handle this attribute as
> > > an offset even when the value is encoded in address form?
> > 
> > As Doug said, it is a space saving (offsets are often small) and it
> > saves a relocation (linkers have to resolve all DW_FORM_addr values and
> > they add up).
> 
> I was wondering why we don't "simply" require a constant format in
> this case, instead of allowing both formats.

DWARF in general seems to prefer only extensions, not deprecation of old
supported ways of doing things. So that DWARFvX is (mostly) valid
DWARFvX+1.

In the case of DW_AT_high_pc you could in principle support only one way
to do it, since it is always used in combination with DW_AT_low_pc as a
pair of attributes. So in that case only supporting an offset in
constant form is fine. But in the case of DW_AT_entry_pc the offset in
constant form variant only makes sense if there is a DW_AT_ranges or
DW_AT_low_pc attribute that gives the "base address". But in theory
(though very unlikely) DW_AT_entry_pc could occur without either of
those other attributes present. So I think also supporting the address
form is still required there.

Cheers,

Mark

Re: [RFA/DWARF] constant class of DW_AT_high_pc is offset for version >=4 only.

[Joel Brobecker]

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

Attached is the latest version of the patch.

I implemented the complaint, and then reverted it, for reasons that
I explained in the added comment inside the new attr_value_as_address
function. Basically, I don't want to be doing extra work as well as
pessimize this function's number of arguments just for a very rare
case of broken compiler.

The added comment should make it clear that there is no guaranty of
correct behavior, though.

gdb/ChangeLog:

        * dwarf2read.c (attr_value_as_address): New function.
        (dwarf2_find_base_address, read_call_site_scope): Use
        attr_value_as_address in place of DW_ADDR.
        (dwarf2_get_pc_bounds): Use attr_value_as_address to get
        the low and high addresses.  Slight rework of the handling
        of the high pc being a constant form, and limit it to
        DWARF verson 4 or higher.
        (dwarf2_record_block_ranges): Likewise.
        (read_partial_die): Likewise.
        (new_symbol_full): Use attr_value_as_address in place of DW_ADDR.

gdb/testsuite/ChangeLog:

        * gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S: New file.
        * gdb.dwarf2/dw2-abs-hi-pc-hello.c: New file.
        * gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S: New file.
        * gdb.dwarf2/dw2-abs-hi-pc-world.c: New file.
        * gdb.dwarf2/dw2-abs-hi-pc.c: New file.
        * gdb.dwarf2/dw2-abs-hi-pc.exp: New file.

Tested on x86_64-linux.

-- 
Joel

[-- Attachment #2: 0001-DWARF-Read-constant-class-addresses-correctly.patch --]
[-- Type: text/x-diff, Size: 26701 bytes --]

From 8b7db9fab8363f568e8337124aaa1dd2e35d0056 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Sat, 15 Feb 2014 19:09:58 +0400
Subject: [PATCH] DWARF: Read constant-class addresses correctly

Starting with DWARF version 4, the description of the DW_AT_high_pc
attribute was amended to say:

   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.

A change was made in Apr 27th, 2012 to reflect that change:

  | commit 91da14142c0171e58a91ad58a32fd010b700e761
  | Author: Mark Wielaard <mjw@redhat.com>
  | Date:   Fri Apr 27 18:55:19 2012 +0000
  |
  |     * dwarf2read.c (dwarf2_get_pc_bounds): Check DW_AT_high_pc form to
  |     see whether it is an address or a constant offset from DW_AT_low_pc.
  |     (dwarf2_record_block_ranges): Likewise.
  |     (read_partial_die): Likewise.

Unfortunately, this new interpretation is now used regardless of
the CU's DWARF version. It turns out that one of WindRiver's compilers
(FTR: Diabdata 4.4) is generating DWARF version 2 info with
DW_AT_high_pc attributes improperly using the data4 form. Because of
that, we miscompute all high PCs incorrectly. This leads to a lot of
symtabs having overlapping ranges, which in turn causes havoc in
pc-to-symtab-and-line translations.

One visible effect is when inserting a breakpoint on a given function:

    (gdb) b world
    Breakpoint 1 at 0x4005c4

The source location of the breakpoint is missing. The output should be:

    (gdb) b world
    Breakpoint 1 at 0x4005c8: file dw2-rel-hi-pc-world.c, line 24.

What happens in this case is that the pc-to-SAL translation first
starts be trying to find the symtab associated to our PC using
each symtab's ranges. Because of the high_pc miscomputation,
many symtabs end up matching, and the heuristic trying to select
the most probable one unfortunately returns one that is unrelated
(it really had no change in this case to do any better). Once we
have the wrong symtab, the start searching the associated linetable,
where the addresses are correct, thus finding no match, and therefore
no SAL.

This patch is an attempt at handling the situation as gracefully
as we can, without guarantees.  It introduces a new function
"attr_value_as_address" which uses the correct accessor for getting
the value of a given attribute.  It then adjust the code throughout
this unit to use this function instead of assuming that addresses always
have the DW_FORM_addr format.

It also fixes the original issue of miscomputing the high_pc
by limiting the new interpretation of constant form DW_AT_high_pc
attributes to units using DWARF version 4 or later.

gdb/ChangeLog:

        * dwarf2read.c (attr_value_as_address): New function.
        (dwarf2_find_base_address, read_call_site_scope): Use
        attr_value_as_address in place of DW_ADDR.
        (dwarf2_get_pc_bounds): Use attr_value_as_address to get
        the low and high addresses.  Slight rework of the handling
        of the high pc being a constant form, and limit it to
        DWARF verson 4 or higher.
        (dwarf2_record_block_ranges): Likewise.
        (read_partial_die): Likewise.
        (new_symbol_full): Use attr_value_as_address in place of DW_ADDR.

gdb/testsuite/ChangeLog:

        * gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S: New file.
        * gdb.dwarf2/dw2-abs-hi-pc-hello.c: New file.
        * gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S: New file.
        * gdb.dwarf2/dw2-abs-hi-pc-world.c: New file.
        * gdb.dwarf2/dw2-abs-hi-pc.c: New file.
        * gdb.dwarf2/dw2-abs-hi-pc.exp: New file.

Tested on x86_64-linux.
---
 gdb/dwarf2read.c                                   |  76 +++++++----
 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S | 151 +++++++++++++++++++++
 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello.c     |  28 ++++
 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S | 151 +++++++++++++++++++++
 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world.c     |  28 ++++
 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.c           |  28 ++++
 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.exp         |  35 +++++
 7 files changed, 469 insertions(+), 28 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.exp

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 54c538a..2afd7c4 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1939,6 +1939,36 @@ byte_swap (offset_type value)
 #define MAYBE_SWAP(V) (V)
 #endif /* WORDS_BIGENDIAN */
 
+/* Read the given attribute value as an address, taking the attribute's
+   form into account.  */
+
+static CORE_ADDR
+attr_value_as_address (struct attribute *attr)
+{
+  CORE_ADDR addr;
+
+  if (attr->form != DW_FORM_addr && attr->form != DW_FORM_GNU_addr_index)
+    {
+      /* Aside from a few clearly defined exceptions, attributes that
+	 contain an address must always be in DW_FORM_addr form.
+	 Unfortunately, some compilers happen to be violating this
+	 requirement by encoding addresses using other forms, such
+	 as DW_FORM_data4 for example.  For those broken compilers,
+	 we try to do our best, without any guarantee of success,
+	 to interpret the address correctly.  It would also be nice
+	 to generate a complaint, but that would require us to maintain
+	 a list of legitimate cases where a non-address form is allowed,
+	 as well as update callers to pass in at least the CU's DWARF
+	 version.  This is more overhead than what we're willing to
+	 expand for a pretty rare case.  */
+      addr = DW_UNSND (attr);
+    }
+  else
+    addr = DW_ADDR (attr);
+
+  return addr;
+}
+
 /* The suffix for an index file.  */
 #define INDEX_SUFFIX ".gdb-index"
 
@@ -4201,7 +4231,7 @@ dwarf2_find_base_address (struct die_info *die, struct dwarf2_cu *cu)
   attr = dwarf2_attr (die, DW_AT_entry_pc, cu);
   if (attr)
     {
-      cu->base_address = DW_ADDR (attr);
+      cu->base_address = attr_value_as_address (attr);
       cu->base_known = 1;
     }
   else
@@ -4209,7 +4239,7 @@ dwarf2_find_base_address (struct die_info *die, struct dwarf2_cu *cu)
       attr = dwarf2_attr (die, DW_AT_low_pc, cu);
       if (attr)
 	{
-	  cu->base_address = DW_ADDR (attr);
+	  cu->base_address = attr_value_as_address (attr);
 	  cu->base_known = 1;
 	}
     }
@@ -11233,7 +11263,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
 		 die->offset.sect_off, objfile_name (objfile));
       return;
     }
-  pc = DW_ADDR (attr) + baseaddr;
+  pc = attr_value_as_address (attr) + baseaddr;
 
   if (cu->call_site_htab == NULL)
     cu->call_site_htab = htab_create_alloc_ex (16, core_addr_hash, core_addr_eq,
@@ -11674,12 +11704,10 @@ dwarf2_get_pc_bounds (struct die_info *die, CORE_ADDR *lowpc,
       attr = dwarf2_attr (die, DW_AT_low_pc, cu);
       if (attr)
         {
-	  low = DW_ADDR (attr);
-	  if (attr_high->form == DW_FORM_addr
-	      || attr_high->form == DW_FORM_GNU_addr_index)
-	    high = DW_ADDR (attr_high);
-	  else
-	    high = low + DW_UNSND (attr_high);
+	  low = attr_value_as_address (attr);
+	  high = attr_value_as_address (attr_high);
+	  if (cu->header.version >= 4 && attr_form_is_constant (attr_high))
+	    high += low;
 	}
       else
 	/* Found high w/o low attribute.  */
@@ -11845,13 +11873,11 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
       attr = dwarf2_attr (die, DW_AT_low_pc, cu);
       if (attr)
         {
-          CORE_ADDR low = DW_ADDR (attr);
-	  CORE_ADDR high;
-	  if (attr_high->form == DW_FORM_addr
-	      || attr_high->form == DW_FORM_GNU_addr_index)
-	    high = DW_ADDR (attr_high);
-	  else
-	    high = low + DW_UNSND (attr_high);
+          CORE_ADDR low = attr_value_as_address (attr);
+	  CORE_ADDR high = attr_value_as_address (attr_high);
+
+	  if (cu->header.version >= 4 && attr_form_is_constant (attr_high))
+	    high += low;
 
           record_block_range (block, baseaddr + low, baseaddr + high - 1);
         }
@@ -15240,18 +15266,13 @@ read_partial_die (const struct die_reader_specs *reader,
 	  break;
 	case DW_AT_low_pc:
 	  has_low_pc_attr = 1;
-	  part_die->lowpc = DW_ADDR (&attr);
+	  part_die->lowpc = attr_value_as_address (&attr);
 	  break;
 	case DW_AT_high_pc:
 	  has_high_pc_attr = 1;
-	  if (attr.form == DW_FORM_addr
-	      || attr.form == DW_FORM_GNU_addr_index)
-	    part_die->highpc = DW_ADDR (&attr);
-	  else
-	    {
-	      high_pc_relative = 1;
-	      part_die->highpc = DW_UNSND (&attr);
-	    }
+	  part_die->highpc = attr_value_as_address (&attr);
+	  if (cu->header.version >= 4 && attr_form_is_constant (&attr))
+		high_pc_relative = 1;
 	  break;
 	case DW_AT_location:
           /* Support the .debug_loc offsets.  */
@@ -17464,9 +17485,8 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	case DW_TAG_label:
 	  attr = dwarf2_attr (die, DW_AT_low_pc, cu);
 	  if (attr)
-	    {
-	      SYMBOL_VALUE_ADDRESS (sym) = DW_ADDR (attr) + baseaddr;
-	    }
+	    SYMBOL_VALUE_ADDRESS (sym)
+	      = attr_value_as_address (attr) + baseaddr;
 	  SYMBOL_TYPE (sym) = objfile_type (objfile)->builtin_core_addr;
 	  SYMBOL_DOMAIN (sym) = LABEL_DOMAIN;
 	  SYMBOL_ACLASS_INDEX (sym) = LOC_LABEL;
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S
new file mode 100644
index 0000000..03f9442
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S
@@ -0,0 +1,151 @@
+/* Copyright 2014 Free Software Foundation, Inc.
+
+   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, see <http://www.gnu.org/licenses/>.  */
+
+	.section	.debug_info
+	.4byte	.Ledebug_info0 - .Lsdebug_info0	/* Length of Compilation Unit Info */
+.Lsdebug_info0:
+	.2byte	0x2	/* DWARF version number */
+	.4byte	.Ldebug_abbrev0	/* Offset Into Abbrev. Section */
+	.byte	0x4	/* Pointer Size (in bytes) */
+	.uleb128 0x1	/* (DIE (0xb) DW_TAG_compile_unit) */
+        .ascii "GNU C 4.7.4 20140206 for GNAT Pro 7.3.0w (20140206)\0"
+	.byte	0x1	/* DW_AT_language */
+	.ascii	"dw2-abs-hi-pc-hello.c\0"     /* DW_AT_name */
+	.ascii  "/tmp\0"        /* DW_AT_comp_dir */
+	.4byte	.hello_start	/* DW_AT_low_pc */
+	.4byte	.hello_end	/* DW_AT_high_pc */
+	.4byte	.Ldebug_line0	/* DW_AT_stmt_list */
+	.uleb128 0x2	/* (DIE (0x2d) DW_TAG_subprogram) */
+	.byte	0x1	/* DW_AT_external */
+	.ascii	"hello\0"
+	.byte	0x1	/* DW_AT_decl_file (hello.c) */
+	.byte	0x13	/* DW_AT_decl_line */
+	.byte	0x1	/* DW_AT_prototyped */
+	.4byte	.hello_start	/* DW_AT_low_pc */
+	.4byte	.hello_end	/* DW_AT_high_pc */
+	.byte	0	/* end of children of DIE 0xb */
+.Ledebug_info0:
+
+	.section	.debug_abbrev
+.Ldebug_abbrev0:
+	.uleb128 0x1	/* (abbrev code) */
+	.uleb128 0x11	/* (TAG: DW_TAG_compile_unit) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x25	/* (DW_AT_producer) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x13	/* (DW_AT_language) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x1b	/* (DW_AT_comp_dir) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x10	/* (DW_AT_stmt_list) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.byte	0
+	.byte	0
+	.uleb128 0x2	/* (abbrev code) */
+	.uleb128 0x2e	/* (TAG: DW_TAG_subprogram) */
+	.byte	0	/* DW_children_no */
+	.uleb128 0x3f	/* (DW_AT_external) */
+	.uleb128 0xc	/* (DW_FORM_flag) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x3a	/* (DW_AT_decl_file) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3b	/* (DW_AT_decl_line) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x27	/* (DW_AT_prototyped) */
+	.uleb128 0xc	/* (DW_FORM_flag) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.byte	0
+	.byte	0
+	.byte	0
+
+	.section	.debug_line
+.Ldebug_line0:
+        .4byte  LELT-LSLT  /* Length of Source Line Info */
+LSLT:
+        .2byte  0x2    /* DWARF Version */
+        .4byte  LELTP-LASLTP     /* Prolog Length */
+LASLTP:
+        .byte   0x1      /* Minimum Instruction Length */
+        .byte   0x1      /* Default is_stmt_start flag */
+        .byte   0x1      /* Line Base Value (Special Opcodes) */
+        .byte   0x1      /* Line Range Value (Special Opcodes) */
+        .byte   0xd      /* Special Opcode Base */
+        .byte   0        /* opcode: 0x1 has 0 args */
+        .byte   0x1      /* opcode: 0x2 has 1 args */
+        .byte   0x1      /* opcode: 0x3 has 1 args */
+        .byte   0x1      /* opcode: 0x4 has 1 args */
+        .byte   0x1      /* opcode: 0x5 has 1 args */
+        .byte   0        /* opcode: 0x6 has 0 args */
+        .byte   0        /* opcode: 0x7 has 0 args */
+        .byte   0        /* opcode: 0x8 has 0 args */
+        .byte   0x1      /* opcode: 0x9 has 1 args */
+        .byte   0        /* opcode: 0xa has 0 args */
+        .byte   0        /* opcode: 0xb has 0 args */
+        .byte   0x1      /* opcode: 0xc has 1 args */
+        .byte   0        /* End directory table */
+        .ascii "dw2-abs-hi-pc-hello.c\0"       /* File Entry: 0x1 */
+        .uleb128 0
+        .uleb128 0
+        .uleb128 0
+        .byte   0        /* End file name table */
+LELTP:
+        .byte   0        /* set address to .hello_start */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .hello_start
+        .byte   0x3      /* DW_LNS_advance_line */
+        .sleb128 22      /* ... to 23 */
+        .byte   0x5      /* column 0 */
+        .uleb128 0       /* 0 */
+        .byte           1       /* DW_LNS_copy */
+
+        .byte   0        /* set address to .hello0 */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .hello0
+        .byte   0x3      /* DW_LNS_advance_line */
+        .sleb128 1       /* ... to 24 */
+        .byte   0x5      /* column 0 */
+        .uleb128 0       /* 0 */
+        .byte           1       /* DW_LNS_copy */
+
+        .byte   0        /* set address to .hello1 */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .hello1
+        .byte   0x3      /* DW_LNS_advance_line */
+        .sleb128 1       /* ... to 25 */
+        .byte   0x5      /* column 0 */
+        .uleb128 0       /* 0 */
+        .byte           1       /* DW_LNS_copy */
+
+        .byte   0        /* set address to .hello_end */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .hello_end
+        .byte   0        /* end sequence */
+        .uleb128 0x1
+        .byte   0x1
+LELT:
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello.c b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello.c
new file mode 100644
index 0000000..59c761d
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-hello.c
@@ -0,0 +1,28 @@
+/* Copyright 2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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, see <http://www.gnu.org/licenses/>.  */
+
+extern int v;
+
+asm (".hello_start: .globl .hello_start\n");
+void
+hello (void)
+{
+asm (".hello0: .globl .hello0\n");
+  v++;
+asm (".hello1: .globl .hello1\n");
+}
+asm (".hello_end: .globl .hello_end\n");
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S
new file mode 100644
index 0000000..c3e89a6
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S
@@ -0,0 +1,151 @@
+/* Copyright 2014 Free Software Foundation, Inc.
+
+   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, see <http://www.gnu.org/licenses/>.  */
+
+	.section	.debug_info
+	.4byte	.Ledebug_info0 - .Lsdebug_info0	/* Length of Compilation Unit Info */
+.Lsdebug_info0:
+	.2byte	0x2	/* DWARF version number */
+	.4byte	.Ldebug_abbrev0	/* Offset Into Abbrev. Section */
+	.byte	0x4	/* Pointer Size (in bytes) */
+	.uleb128 0x1	/* (DIE (0xb) DW_TAG_compile_unit) */
+        .ascii "GNU C 4.7.4 20140206 for GNAT Pro 7.3.0w (20140206)\0"
+	.byte	0x1	/* DW_AT_language */
+	.ascii	"dw2-abs-hi-pc-world.c\0"     /* DW_AT_name */
+	.ascii  "/tmp\0"        /* DW_AT_comp_dir */
+	.4byte	.world_start	/* DW_AT_low_pc */
+	.4byte	.world_end	/* DW_AT_high_pc */
+	.4byte	.Ldebug_line0	/* DW_AT_stmt_list */
+	.uleb128 0x2	/* (DIE (0x2d) DW_TAG_subprogram) */
+	.byte	0x1	/* DW_AT_external */
+	.ascii	"world\0"
+	.byte	0x1	/* DW_AT_decl_file (world.c) */
+	.byte	0x13	/* DW_AT_decl_line */
+	.byte	0x1	/* DW_AT_prototyped */
+	.4byte	.world_start	/* DW_AT_low_pc */
+	.4byte	.world_end	/* DW_AT_high_pc */
+	.byte	0	/* end of children of DIE 0xb */
+.Ledebug_info0:
+
+	.section	.debug_abbrev
+.Ldebug_abbrev0:
+	.uleb128 0x1	/* (abbrev code) */
+	.uleb128 0x11	/* (TAG: DW_TAG_compile_unit) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x25	/* (DW_AT_producer) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x13	/* (DW_AT_language) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x1b	/* (DW_AT_comp_dir) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x10	/* (DW_AT_stmt_list) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.byte	0
+	.byte	0
+	.uleb128 0x2	/* (abbrev code) */
+	.uleb128 0x2e	/* (TAG: DW_TAG_subprogram) */
+	.byte	0	/* DW_children_no */
+	.uleb128 0x3f	/* (DW_AT_external) */
+	.uleb128 0xc	/* (DW_FORM_flag) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8    /* (DW_FORM_string) */
+	.uleb128 0x3a	/* (DW_AT_decl_file) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3b	/* (DW_AT_decl_line) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x27	/* (DW_AT_prototyped) */
+	.uleb128 0xc	/* (DW_FORM_flag) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x6	/* (DW_FORM_data4) */
+	.byte	0
+	.byte	0
+	.byte	0
+
+	.section	.debug_line
+.Ldebug_line0:
+        .4byte  LELT-LSLT  /* Length of Source Line Info */
+LSLT:
+        .2byte  0x2    /* DWARF Version */
+        .4byte  LELTP-LASLTP     /* Prolog Length */
+LASLTP:
+        .byte   0x1      /* Minimum Instruction Length */
+        .byte   0x1      /* Default is_stmt_start flag */
+        .byte   0x1      /* Line Base Value (Special Opcodes) */
+        .byte   0x1      /* Line Range Value (Special Opcodes) */
+        .byte   0xd      /* Special Opcode Base */
+        .byte   0        /* opcode: 0x1 has 0 args */
+        .byte   0x1      /* opcode: 0x2 has 1 args */
+        .byte   0x1      /* opcode: 0x3 has 1 args */
+        .byte   0x1      /* opcode: 0x4 has 1 args */
+        .byte   0x1      /* opcode: 0x5 has 1 args */
+        .byte   0        /* opcode: 0x6 has 0 args */
+        .byte   0        /* opcode: 0x7 has 0 args */
+        .byte   0        /* opcode: 0x8 has 0 args */
+        .byte   0x1      /* opcode: 0x9 has 1 args */
+        .byte   0        /* opcode: 0xa has 0 args */
+        .byte   0        /* opcode: 0xb has 0 args */
+        .byte   0x1      /* opcode: 0xc has 1 args */
+        .byte   0        /* End directory table */
+        .ascii "dw2-abs-hi-pc-world.c\0"       /* File Entry: 0x1 */
+        .uleb128 0
+        .uleb128 0
+        .uleb128 0
+        .byte   0        /* End file name table */
+LELTP:
+        .byte   0        /* set address to .world_start */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .world_start
+        .byte   0x3      /* DW_LNS_advance_line */
+        .sleb128 22      /* ... to 23 */
+        .byte   0x5      /* column 0 */
+        .uleb128 0       /* 0 */
+        .byte           1       /* DW_LNS_copy */
+
+        .byte   0        /* set address to .world0 */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .world0
+        .byte   0x3      /* DW_LNS_advance_line */
+        .sleb128 1       /* ... to 24 */
+        .byte   0x5      /* column 0 */
+        .uleb128 0       /* 0 */
+        .byte           1       /* DW_LNS_copy */
+
+        .byte   0        /* set address to .world1 */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .world1
+        .byte   0x3      /* DW_LNS_advance_line */
+        .sleb128 1       /* ... to 25 */
+        .byte   0x5      /* column 0 */
+        .uleb128 0       /* 0 */
+        .byte           1       /* DW_LNS_copy */
+
+        .byte   0        /* set address to .world_end */
+        .uleb128 0x5
+        .byte   0x2
+        .4byte  .world_end
+        .byte   0        /* end sequence */
+        .uleb128 0x1
+        .byte   0x1
+LELT:
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world.c b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world.c
new file mode 100644
index 0000000..04f17bd
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc-world.c
@@ -0,0 +1,28 @@
+/* Copyright 2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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, see <http://www.gnu.org/licenses/>.  */
+
+extern int v;
+
+asm (".world_start: .globl .world_start\n");
+void
+world (void)
+{
+asm (".world0: .globl .world0\n");
+  v++;
+asm (".world1: .globl .world1\n");
+}
+asm (".world_end: .globl .world_end\n");
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.c b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.c
new file mode 100644
index 0000000..95773a2
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.c
@@ -0,0 +1,28 @@
+/* Copyright 2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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, see <http://www.gnu.org/licenses/>.  */
+
+extern void hello (void);
+extern void world (void);
+
+int v;
+
+int
+main (void)
+{
+  hello ();
+  world ();
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.exp b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.exp
new file mode 100644
index 0000000..3353fda
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-abs-hi-pc.exp
@@ -0,0 +1,35 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# 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, see <http://www.gnu.org/licenses/>.
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+standard_testfile
+set executable ${testfile}
+
+if {[build_executable ${testfile}.exp ${executable} "${testfile}.c ${testfile}-hello-dbg.S ${testfile}-hello.c ${testfile}-world-dbg.S ${testfile}-world.c"] == -1} {
+    return -1
+}
+
+clean_restart $executable
+
+gdb_test "break hello" \
+    "Breakpoint $decimal at $hex: file .*dw2-abs-hi-pc-hello\\.c, line 24\\."
+
+gdb_test "break world" \
+    "Breakpoint $decimal at $hex: file .*dw2-abs-hi-pc-world\\.c, line 24\\."
-- 
1.8.3.2

Re: [RFA/DWARF] constant class of DW_AT_high_pc is offset for version >=4 only.

[Mark Wielaard]

On Fri, 2014-02-21 at 19:42 +0100, Joel Brobecker wrote:
> Attached is the latest version of the patch.
> 
> I implemented the complaint, and then reverted it, for reasons that
> I explained in the added comment inside the new attr_value_as_address
> function. Basically, I don't want to be doing extra work as well as
> pessimize this function's number of arguments just for a very rare
> case of broken compiler.

Obviously I would like lots of screaming and yelling here (and the
function/argument will probably be inlined anyway, so I don't think the
extra argument passing will actually hurt that much, but that is
speculation). I do understand gdb isn't a DWARF-linter, and how you do
things is more in line with the "be liberal in what you accept"
principle.

No complaints (pun intended) to this patch.

Cheers,

Mark

pushed: [RFA/DWARF] constant class of DW_AT_high_pc is offset for version >=4 only.

[Joel Brobecker]

> gdb/ChangeLog:
> 
>         * dwarf2read.c (attr_value_as_address): New function.
>         (dwarf2_find_base_address, read_call_site_scope): Use
>         attr_value_as_address in place of DW_ADDR.
>         (dwarf2_get_pc_bounds): Use attr_value_as_address to get
>         the low and high addresses.  Slight rework of the handling
>         of the high pc being a constant form, and limit it to
>         DWARF verson 4 or higher.
>         (dwarf2_record_block_ranges): Likewise.
>         (read_partial_die): Likewise.
>         (new_symbol_full): Use attr_value_as_address in place of DW_ADDR.
> 
> gdb/testsuite/ChangeLog:
> 
>         * gdb.dwarf2/dw2-abs-hi-pc-hello-dbg.S: New file.
>         * gdb.dwarf2/dw2-abs-hi-pc-hello.c: New file.
>         * gdb.dwarf2/dw2-abs-hi-pc-world-dbg.S: New file.
>         * gdb.dwarf2/dw2-abs-hi-pc-world.c: New file.
>         * gdb.dwarf2/dw2-abs-hi-pc.c: New file.
>         * gdb.dwarf2/dw2-abs-hi-pc.exp: New file.

FYI: This patch has now been pushed.  Thanks again to MarkW for all
the insightful comments.

-- 
Joel