diff mbox

[v2] gdb: Handle multiple base address in debug_ranges data.

Message ID 20151103103505.GO23628@embecosm.com
State New
Headers show

Commit Message

Andrew Burgess Nov. 3, 2015, 10:35 a.m. UTC
Below is a second attempt at fixing the multiple base address bug in
the gdb DWARF parsing code.

The actual fix is identical to the first patch, the big change here is
in the testing.

Gone is the x86-64 assembler example, and instead I have tried[1] to
extend the dwarf assembler to generate sufficient information to
trigger this bug, this required extensions to the .debug_line
generation, and new code to handle .debug_ranges generation.

I've tested this new tests on x86-64 Linux compiling as a 64-bit
target, and when passing '-m32' to the test to compile as a 32-bit
target.

My TCL skills are pretty weak[2] so constructive guidance on how to
improve this code would be great, alternatively if someone (anyone)
would like to show me how easy this is, please do improve on this
patch.

Alternatively, how do you feel about letting this in ... for now.

Thanks,
Andrew


[1] Really, I tried.  No matter how bad the TCL code might look,
    please don't think I've not tried :)

[2] As evidence I present .... this patch :)

---

It is possible to use multiple base addresses within a single address
range series, within the .debug_ranges section.  The following is a
simplified example for 32-bit addresses:

  .section ".debug_ranges"
  .4byte	0xffffffff
  .4byte	BASE_1
  .4byte	START_OFFSET_1
  .4byte	END_OFFSET_1
  .4byte	START_OFFSET_2
  .4byte	END_OFFSET_2
  .4byte	0xffffffff
  .4byte	BASE_2
  .4byte	START_OFFSET_3
  .4byte	END_OFFSET_3
  .4byte	0
  .4byte	0

In this example START/END 1 and 2 are relative to BASE_1, while
START/END 3 are relative to BASE_2.

Currently gdb does not correctly parse this DWARF, resulting in
corrupted address range information.  This commit fixes this issue, and
adds a new test to cover this case.

In order to support testing of this feature extensions were made to the
testsuite dwarf assembler, additional functionality was added to the
.debug_line generation function, and a new function for generating the
.debug_ranges section was added.

gdb/ChangeLog:

	* dwarf2read.c (dwarf2_ranges_read): Unify and fix base address
	reading code.

gdb/testsuite/ChangeLog:

	* gdb.dwarf2/dw2-ranges-base.c: New file.
	* gdb.dwarf2/dw2-ranges-base.exp: New file.
	* lib/dwarf.exp (namespace eval Dwarf): Add new variables to
	support additional line table, and debug ranges generation.
	(Dwarf::ranges): New function, generate .debug_ranges.
	(Dwarf::lines): Support generating simple line table programs.
	(Dwarf::assemble): Initialise new namespace variables.
---
 gdb/ChangeLog                                |   5 +
 gdb/dwarf2read.c                             |  19 +--
 gdb/testsuite/ChangeLog                      |  10 ++
 gdb/testsuite/gdb.dwarf2/dw2-ranges-base.c   |  36 ++++++
 gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp | 143 ++++++++++++++++++++++
 gdb/testsuite/lib/dwarf.exp                  | 175 +++++++++++++++++++++++++--
 6 files changed, 362 insertions(+), 26 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-ranges-base.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp

Comments

Andrew Burgess Nov. 25, 2015, 10:53 a.m. UTC | #1
Ping!

* Andrew Burgess <andrew.burgess@embecosm.com> [2015-11-03 10:35:06 +0000]:

> Below is a second attempt at fixing the multiple base address bug in
> the gdb DWARF parsing code.
> 
> The actual fix is identical to the first patch, the big change here is
> in the testing.
> 
> Gone is the x86-64 assembler example, and instead I have tried[1] to
> extend the dwarf assembler to generate sufficient information to
> trigger this bug, this required extensions to the .debug_line
> generation, and new code to handle .debug_ranges generation.
> 
> I've tested this new tests on x86-64 Linux compiling as a 64-bit
> target, and when passing '-m32' to the test to compile as a 32-bit
> target.
> 
> My TCL skills are pretty weak[2] so constructive guidance on how to
> improve this code would be great, alternatively if someone (anyone)
> would like to show me how easy this is, please do improve on this
> patch.
> 
> Alternatively, how do you feel about letting this in ... for now.
> 
> Thanks,
> Andrew
> 
> 
> [1] Really, I tried.  No matter how bad the TCL code might look,
>     please don't think I've not tried :)
> 
> [2] As evidence I present .... this patch :)
> 
> ---
> 
> It is possible to use multiple base addresses within a single address
> range series, within the .debug_ranges section.  The following is a
> simplified example for 32-bit addresses:
> 
>   .section ".debug_ranges"
>   .4byte	0xffffffff
>   .4byte	BASE_1
>   .4byte	START_OFFSET_1
>   .4byte	END_OFFSET_1
>   .4byte	START_OFFSET_2
>   .4byte	END_OFFSET_2
>   .4byte	0xffffffff
>   .4byte	BASE_2
>   .4byte	START_OFFSET_3
>   .4byte	END_OFFSET_3
>   .4byte	0
>   .4byte	0
> 
> In this example START/END 1 and 2 are relative to BASE_1, while
> START/END 3 are relative to BASE_2.
> 
> Currently gdb does not correctly parse this DWARF, resulting in
> corrupted address range information.  This commit fixes this issue, and
> adds a new test to cover this case.
> 
> In order to support testing of this feature extensions were made to the
> testsuite dwarf assembler, additional functionality was added to the
> .debug_line generation function, and a new function for generating the
> .debug_ranges section was added.
> 
> gdb/ChangeLog:
> 
> 	* dwarf2read.c (dwarf2_ranges_read): Unify and fix base address
> 	reading code.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.dwarf2/dw2-ranges-base.c: New file.
> 	* gdb.dwarf2/dw2-ranges-base.exp: New file.
> 	* lib/dwarf.exp (namespace eval Dwarf): Add new variables to
> 	support additional line table, and debug ranges generation.
> 	(Dwarf::ranges): New function, generate .debug_ranges.
> 	(Dwarf::lines): Support generating simple line table programs.
> 	(Dwarf::assemble): Initialise new namespace variables.
> ---
>  gdb/ChangeLog                                |   5 +
>  gdb/dwarf2read.c                             |  19 +--
>  gdb/testsuite/ChangeLog                      |  10 ++
>  gdb/testsuite/gdb.dwarf2/dw2-ranges-base.c   |  36 ++++++
>  gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp | 143 ++++++++++++++++++++++
>  gdb/testsuite/lib/dwarf.exp                  | 175 +++++++++++++++++++++++++--
>  6 files changed, 362 insertions(+), 26 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-ranges-base.c
>  create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 38a42ea..3a9e992 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,8 @@
> +2015-10-15  Andrew Burgess  <andrew.burgess@embecosm.com>
> +
> +	* dwarf2read.c (dwarf2_ranges_read): Unify and fix base address
> +	reading code.
> +
>  2015-10-30  Pedro Alves  <palves@redhat.com>
>  
>  	* breakpoint.c (breakpoint_in_range_p)
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 87dc8b4..a560ed8 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -11894,7 +11894,6 @@ dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return,
>    int found_base;
>    unsigned int dummy;
>    const gdb_byte *buffer;
> -  CORE_ADDR marker;
>    int low_set;
>    CORE_ADDR low = 0;
>    CORE_ADDR high = 0;
> @@ -11913,18 +11912,6 @@ dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return,
>      }
>    buffer = dwarf2_per_objfile->ranges.buffer + offset;
>  
> -  /* Read in the largest possible address.  */
> -  marker = read_address (obfd, buffer, cu, &dummy);
> -  if ((marker & mask) == mask)
> -    {
> -      /* If we found the largest possible address, then
> -	 read the base address.  */
> -      base = read_address (obfd, buffer + addr_size, cu, &dummy);
> -      buffer += 2 * addr_size;
> -      offset += 2 * addr_size;
> -      found_base = 1;
> -    }
> -
>    low_set = 0;
>  
>    baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
> @@ -11949,9 +11936,9 @@ dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return,
>  	 the base address.  Check for a base address here.  */
>        if ((range_beginning & mask) == mask)
>  	{
> -	  /* If we found the largest possible address, then
> -	     read the base address.  */
> -	  base = read_address (obfd, buffer + addr_size, cu, &dummy);
> +	  /* If we found the largest possible address, then we already
> +	     have the base address in range_end.  */
> +	  base = range_end;
>  	  found_base = 1;
>  	  continue;
>  	}
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index e01ee86..5ab6199 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,13 @@
> +2015-10-16  Andrew Burgess  <andrew.burgess@embecosm.com>
> +
> +	* gdb.dwarf2/dw2-ranges-base.c: New file.
> +	* gdb.dwarf2/dw2-ranges-base.exp: New file.
> +	* lib/dwarf.exp (namespace eval Dwarf): Add new variables to
> +	support additional line table, and debug ranges generation.
> +	(Dwarf::ranges): New function, generate .debug_ranges.
> +	(Dwarf::lines): Support generating simple line table programs.
> +	(Dwarf::assemble): Initialise new namespace variables.
> +
>  2015-10-30  Yao Qi  <yao.qi@linaro.org>
>  
>  	* gdb.threads/wp-replication.c (watch_count_done): Remove.
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.c b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.c
> new file mode 100644
> index 0000000..4d52b6e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.c
> @@ -0,0 +1,36 @@
> +/*
> +   Copyright 2015 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/>.  */
> +
> +void __attribute__ ((section (".text.3")))
> +frame3 (void)
> +{
> +  asm ("frame3_label: .globl frame3_label");
> +}
> +
> +void __attribute__ ((section (".text.2")))
> +frame2 (void)
> +{
> +  asm ("frame2_label: .globl frame2_label");
> +  frame3 ();
> +}
> +
> +void __attribute__ ((section (".text.1")))
> +main (void)
> +{
> +  asm ("main_label: .globl main_label");
> +  frame2 ();
> +}
> +
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
> new file mode 100644
> index 0000000..ae891c3
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
> @@ -0,0 +1,143 @@
> +# Copyright 2015 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
> +
> +# Test DW_TAG_compile_unit with no children and with neither DW_AT_low_pc nor
> +# DW_AT_high_pc but with DW_AT_ranges instead.
> +
> +# This test can only be run on targets which support DWARF-2 and use gas.
> +if {![dwarf2_support]} {
> +    verbose "Skipping DW_AT_ranges test."
> +    return 0
> +}
> +
> +# The .c files use __attribute__.
> +if [get_compiler_info] {
> +    return -1
> +}
> +if !$gcc_compiled {
> +    verbose "Skipping DW_AT_ranges test."
> +    return 0
> +}
> +
> +standard_testfile dw2-ranges-base.c dw2-ranges-base-dw.S
> +
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> +    global srcdir subdir srcfile srcfile2
> +    declare_labels ranges_label;
> +    declare_labels L;
> +
> +    # Find start address and length for our functions.
> +    set main_func \
> +	[function_range main [list ${srcdir}/${subdir}/$srcfile]]
> +    set frame2_func \
> +	[function_range frame2 [list ${srcdir}/${subdir}/$srcfile]]
> +    set frame3_func \
> +	[function_range frame3 [list ${srcdir}/${subdir}/$srcfile]]
> +
> +    # Very simple info for this test program.  We don't care about
> +    # this information being correct (w.r.t. funtion / argument types)
> +    # just so long as the compilation using makes use of the
> +    # .debug_ranges data then the test achieves its objective.
> +    cu {} {
> +	compile_unit {
> +	    {language @DW_LANG_C}
> +	    {name dw-ranges-base.c}
> +	    {stmt_list $L DW_FORM_sec_offset}
> +	    {ranges ${ranges_label} DW_FORM_sec_offset}
> +	} {
> +	    subprogram {
> +		{external 1 flag}
> +		{name main}
> +	    }
> +	    subprogram {
> +		{external 1 flag}
> +		{name frame2}
> +	    }
> +	    subprogram {
> +		{external 1 flag}
> +		{name frame3}
> +	    }
> +	}
> +    }
> +
> +    lines {version 2} L {
> +	include_dir "${srcdir}/${subdir}"
> +	file_name "$srcfile" 1
> +
> +	# Generate simple line table program.  The line table
> +	# information contained here is not correct, and we really
> +	# don't care, just so long as each function has some line
> +	# table data associated with it.  We do make use of the fake
> +	# line numbers that we pick here in the tests below.
> +	program {
> +	    {DW_LNE_set_address [lindex $main_func 0]}
> +	    {DW_LNS_advance_line 10}
> +	    {DW_LNS_copy}
> +	    {DW_LNS_advance_pc [lindex $main_func 1]}
> +	    {DW_LNS_advance_line 19}
> +	    {DW_LNS_copy}
> +	    {DW_LNE_end_sequence}
> +
> +	    {DW_LNE_set_address [lindex $frame2_func 0]}
> +	    {DW_LNS_advance_line 20}
> +	    {DW_LNS_copy}
> +	    {DW_LNS_advance_pc [lindex $frame2_func 1]}
> +	    {DW_LNS_advance_line 29}
> +	    {DW_LNS_copy}
> +	    {DW_LNE_end_sequence}
> +
> +	    {DW_LNE_set_address [lindex $frame3_func 0]}
> +	    {DW_LNS_advance_line 30}
> +	    {DW_LNS_copy}
> +	    {DW_LNS_advance_pc [lindex $frame3_func 1]}
> +	    {DW_LNS_advance_line 39}
> +	    {DW_LNS_copy}
> +	    {DW_LNE_end_sequence}
> +	}
> +    }
> +
> +    # Generate ranges data.  This is the point of this whole test
> +    # file, we must have multiple bases specified, so we use a new
> +    # base for each function.
> +    ranges {is_64 [is_64_target]} {
> +	ranges_label: sequence {
> +	    {base [lindex $main_func 0]}
> +	    {range 0 [lindex $main_func 1]}
> +	    {base [lindex $frame2_func 0]}
> +	    {range 0 [lindex $frame2_func 1]}
> +	    {base [lindex $frame3_func 0]}
> +	    {range 0 [lindex $frame3_func 1]}
> +	}
> +    }
> +}
> +
> +if { [prepare_for_testing ${testfile}.exp ${testfile} \
> +	  [list $srcfile $asm_file] {nodebug}] } {
> +    return -1
> +}
> +
> +if ![runto_main] {
> +    return -1
> +}
> +
> +# Make use of the line numbers we faked in the .debug_line table above.
> +gdb_test "info line main" \
> +    "Line 11 of .* starts at address .* and ends at .*"
> +gdb_test "info line frame2" \
> +    "Line 21 of .* starts at address .* and ends at .*"
> +gdb_test "info line frame3" \
> +    "Line 31 of .* starts at address .* and ends at .*"
> diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
> index 5dc7ea8..03b59ce 100644
> --- a/gdb/testsuite/lib/dwarf.exp
> +++ b/gdb/testsuite/lib/dwarf.exp
> @@ -303,6 +303,15 @@ namespace eval Dwarf {
>      # Whether a file_name entry was seen.
>      variable _line_saw_file
>  
> +    # Whether a line table program has been seen.
> +    variable _line_saw_program
> +
> +    # A Label for line table header generation.
> +    variable _line_header_end_label
> +
> +    # The address size for debug ranges section.
> +    variable _debug_ranges_64_bit
> +
>      proc _process_one_constant {name value} {
>  	variable _constants
>  	variable _AT
> @@ -981,7 +990,7 @@ namespace eval Dwarf {
>  	set _cu_label [_compute_label "cu${cu_num}_begin"]
>  	set start_label [_compute_label "cu${cu_num}_start"]
>  	set end_label [_compute_label "cu${cu_num}_end"]
> -	
> +
>  	define_label $_cu_label
>  	if {$is_64} {
>  	    _op .4byte 0xffffffff
> @@ -1118,6 +1127,78 @@ namespace eval Dwarf {
>  	define_label $end_label
>      }
>  
> +    # Emit a DWARF .debug_ranges unit.
> +    # OPTIONS is a list with an even number of elements containing
> +    # option-name and option-value pairs.
> +    # Current options are:
> +    # is_64 0|1    - boolean indicating if you want to emit 64-bit DWARF
> +    #                default = 0 (32-bit)
> +    #
> +    # BODY is Tcl code that emits the content of the .debug_ranges
> +    # unit, it is evaluated in the caller's context.
> +    proc ranges {options body} {
> +	variable _debug_ranges_64_bit
> +
> +	foreach { name value } $options {
> +	    switch -exact -- $name {
> +		is_64 { set _debug_ranges_64_bit [subst $value] }
> +		default { error "unknown option $name" }
> +	    }
> +	}
> +
> +	set section ".debug_ranges"
> +	_section $section
> +
> +	proc sequence {{ranges {}}} {
> +	    variable _debug_ranges_64_bit
> +
> +	    # Emit the sequence of addresses.
> +	    set base ""
> +	    foreach range $ranges {
> +		set range [uplevel 1 "subst \"$range\""]
> +		set type [lindex $range 0]
> +		switch -exact -- $type {
> +		    base {
> +			set base [lrange $range 1 end]
> +
> +			if { $_debug_ranges_64_bit } then {
> +			    _op .8byte 0xffffffffffffffff "Base Marker"
> +			    _op .8byte $base "Base Address"
> +			} else {
> +			    _op .4byte 0xffffffff "Base Marker"
> +			    _op .4byte $base "Base Address"
> +			}
> +		    }
> +		    range {
> +			set start [lindex $range 1]
> +			set end [lrange $range 2 end]
> +
> +			if { $_debug_ranges_64_bit } then {
> +			    _op .8byte $start "Start Address"
> +			    _op .8byte $end "End Address"
> +			} else {
> +			    _op .4byte $start "Start Address"
> +			    _op .4byte $end "End Address"
> +			}
> +		    }
> +		    default { error "unknown range type: $type " }
> +		}
> +	    }
> +
> +	    # End of the sequence.
> +	    if { $_debug_ranges_64_bit } then {
> +		_op .8byte 0x0 "End of Sequence Marker (Part 1)"
> +		_op .8byte 0x0 "End of Sequence Marker (Part 2)"
> +	    } else {
> +		_op .4byte 0x0 "End of Sequence Marker (Part 1)"
> +		_op .4byte 0x0 "End of Sequence Marker (Part 2)"
> +	    }
> +	}
> +
> +	uplevel $body
> +    }
> +
> +
>      # Emit a DWARF .debug_line unit.
>      # OPTIONS is a list with an even number of elements containing
>      # option-name and option-value pairs.
> @@ -1146,6 +1227,8 @@ namespace eval Dwarf {
>      proc lines {options label body} {
>  	variable _line_count
>  	variable _line_saw_file
> +	variable _line_saw_program
> +	variable _line_header_end_label
>  
>  	# Establish the defaults.
>  	set is_64 0
> @@ -1181,7 +1264,7 @@ namespace eval Dwarf {
>  	set unit_len_label [_compute_label "line${_line_count}_start"]
>  	set unit_end_label [_compute_label "line${_line_count}_end"]
>  	set header_len_label [_compute_label "line${_line_count}_header_start"]
> -	set header_end_label [_compute_label "line${_line_count}_header_end"]
> +	set _line_header_end_label [_compute_label "line${_line_count}_header_end"]
>  
>  	if {$is_64} {
>  	    _op .4byte 0xffffffff
> @@ -1195,20 +1278,34 @@ namespace eval Dwarf {
>  	_op .2byte $_unit_version version
>  
>  	if {$is_64} {
> -	    _op .8byte "$header_end_label - $header_len_label" "header_length"
> +	    _op .8byte "$_line_header_end_label - $header_len_label" "header_length"
>  	} else {
> -	    _op .4byte "$header_end_label - $header_len_label" "header_length"
> +	    _op .4byte "$_line_header_end_label - $header_len_label" "header_length"
>  	}
>  
>  	define_label $header_len_label
>  
>  	_op .byte 1 "minimum_instruction_length"
> -	_op .byte 0 "default_is_stmt"
> +	_op .byte 1 "default_is_stmt"
>  	_op .byte 1 "line_base"
>  	_op .byte 1 "line_range"
> -	_op .byte 1 "opcode_base"
> -	# Since we emit opcode_base==1, we skip
> -	# standard_opcode_length table altogether.
> +	_op .byte 10 "opcode_base"
> +
> +	# The standard_opcode_lengths table.  The number of arguments
> +	# for each of the standard opcodes.  Generating 9 entries here
> +	# matches the use of 10 in the opcode_base above.  These 9
> +	# entries match the 9 standard opcodes for DWARF2, making use
> +	# of only 9 should be fine, even if we are generating DWARF3
> +	# or DWARF4.
> +	_op .byte 0 "standard opcode 1"
> +	_op .byte 1 "standard opcode 2"
> +	_op .byte 1 "standard opcode 3"
> +	_op .byte 1 "standard opcode 4"
> +	_op .byte 1 "standard opcode 5"
> +	_op .byte 0 "standard opcode 6"
> +	_op .byte 0 "standard opcode 7"
> +	_op .byte 0 "standard opcode 8"
> +	_op .byte 1 "standard opcode 9"
>  
>  	proc include_dir {dirname} {
>  	    _op .ascii [_quote $dirname]
> @@ -1228,6 +1325,57 @@ namespace eval Dwarf {
>  	    _op .sleb128 0 "length"
>  	}
>  
> +	proc program {statements} {
> +	    variable _line_saw_program
> +	    variable _line_header_end_label
> +
> +	    if "! $_line_saw_program" {
> +		# Terminate the file list.
> +		_op .byte 0 "Terminator."
> +		define_label $_line_header_end_label
> +		set _line_saw_program 1
> +	    }
> +
> +	    proc DW_LNE_set_address {addr} {
> +		_op .byte 0
> +		set start [new_label "set_address_start"]
> +		set end [new_label "set_address_end"]
> +		_op .uleb128 "${end} - ${start}"
> +		define_label ${start}
> +		_op .byte 2
> +		if {[is_64_target]} {
> +		    _op .8byte ${addr}
> +		} else {
> +		    _op .4byte ${addr}
> +		}
> +		define_label ${end}
> +	    }
> +
> +	    proc DW_LNE_end_sequence {} {
> +		_op .byte 0
> +		_op .uleb128 1
> +		_op .byte 1
> +	    }
> +
> +	    proc DW_LNS_copy {} {
> +		_op .byte 1
> +	    }
> +
> +	    proc DW_LNS_advance_pc {offset} {
> +		_op .byte 2
> +		_op .uleb128 ${offset}
> +	    }
> +
> +	    proc DW_LNS_advance_line {offset} {
> +		_op .byte 3
> +		_op .sleb128 ${offset}
> +	    }
> +
> +	    foreach statement $statements {
> +		uplevel 1 $statement
> +	    }
> +	}
> +
>  	uplevel $body
>  
>  	rename include_dir ""
> @@ -1239,9 +1387,11 @@ namespace eval Dwarf {
>  	}
>  
>  	# Terminate the file list.
> -	_op .byte 0 "Terminator."
> +	if "! $_line_saw_program" {
> +	    _op .byte 0 "Terminator."
> +	    define_label $_line_header_end_label
> +	}
>  
> -	define_label $header_end_label
>  	define_label $unit_end_label
>      }
>  
> @@ -1326,6 +1476,9 @@ namespace eval Dwarf {
>  	variable _cu_count
>  	variable _line_count
>  	variable _line_saw_file
> +	variable _line_saw_program
> +	variable _line_header_end_label
> +	variable _debug_ranges_64_bit
>  
>  	if {!$_initialized} {
>  	    _read_constants
> @@ -1341,6 +1494,8 @@ namespace eval Dwarf {
>  
>  	set _line_count 0
>  	set _line_saw_file 0
> +	set _line_saw_program 0
> +	set _debug_ranges_64_bit [is_64_target]
>  
>  	# Not "uplevel" here, because we want to evaluate in this
>  	# namespace.  This is somewhat bad because it means we can't
> -- 
> 2.5.1
>
Pedro Alves Dec. 9, 2015, 12:42 p.m. UTC | #2
On 11/03/2015 10:35 AM, Andrew Burgess wrote:
> Below is a second attempt at fixing the multiple base address bug in
> the gdb DWARF parsing code.
> 
> The actual fix is identical to the first patch, the big change here is
> in the testing.
> 
> Gone is the x86-64 assembler example, and instead I have tried[1] to
> extend the dwarf assembler to generate sufficient information to
> trigger this bug, this required extensions to the .debug_line
> generation, and new code to handle .debug_ranges generation.
> 
> I've tested this new tests on x86-64 Linux compiling as a 64-bit
> target, and when passing '-m32' to the test to compile as a 32-bit
> target.
> 
> My TCL skills are pretty weak[2] so constructive guidance on how to
> improve this code would be great, alternatively if someone (anyone)
> would like to show me how easy this is, please do improve on this
> patch.
> 
> Alternatively, how do you feel about letting this in ... for now.

Thanks for doing this!

I skimmed it, and it looks like the same style of code as the rest
of the Dwarf assembler, so I'd feel quite comfortable putting
this in as is.

Tiny nit:

> +Dwarf::assemble $asm_file {
> +    global srcdir subdir srcfile srcfile2
> +    declare_labels ranges_label;
> +    declare_labels L;
> +

No need for ; at the end of those declare_labels lines.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 38a42ea..3a9e992 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@ 
+2015-10-15  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* dwarf2read.c (dwarf2_ranges_read): Unify and fix base address
+	reading code.
+
 2015-10-30  Pedro Alves  <palves@redhat.com>
 
 	* breakpoint.c (breakpoint_in_range_p)
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 87dc8b4..a560ed8 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -11894,7 +11894,6 @@  dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return,
   int found_base;
   unsigned int dummy;
   const gdb_byte *buffer;
-  CORE_ADDR marker;
   int low_set;
   CORE_ADDR low = 0;
   CORE_ADDR high = 0;
@@ -11913,18 +11912,6 @@  dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return,
     }
   buffer = dwarf2_per_objfile->ranges.buffer + offset;
 
-  /* Read in the largest possible address.  */
-  marker = read_address (obfd, buffer, cu, &dummy);
-  if ((marker & mask) == mask)
-    {
-      /* If we found the largest possible address, then
-	 read the base address.  */
-      base = read_address (obfd, buffer + addr_size, cu, &dummy);
-      buffer += 2 * addr_size;
-      offset += 2 * addr_size;
-      found_base = 1;
-    }
-
   low_set = 0;
 
   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
@@ -11949,9 +11936,9 @@  dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return,
 	 the base address.  Check for a base address here.  */
       if ((range_beginning & mask) == mask)
 	{
-	  /* If we found the largest possible address, then
-	     read the base address.  */
-	  base = read_address (obfd, buffer + addr_size, cu, &dummy);
+	  /* If we found the largest possible address, then we already
+	     have the base address in range_end.  */
+	  base = range_end;
 	  found_base = 1;
 	  continue;
 	}
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index e01ee86..5ab6199 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,13 @@ 
+2015-10-16  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.dwarf2/dw2-ranges-base.c: New file.
+	* gdb.dwarf2/dw2-ranges-base.exp: New file.
+	* lib/dwarf.exp (namespace eval Dwarf): Add new variables to
+	support additional line table, and debug ranges generation.
+	(Dwarf::ranges): New function, generate .debug_ranges.
+	(Dwarf::lines): Support generating simple line table programs.
+	(Dwarf::assemble): Initialise new namespace variables.
+
 2015-10-30  Yao Qi  <yao.qi@linaro.org>
 
 	* gdb.threads/wp-replication.c (watch_count_done): Remove.
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.c b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.c
new file mode 100644
index 0000000..4d52b6e
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.c
@@ -0,0 +1,36 @@ 
+/*
+   Copyright 2015 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/>.  */
+
+void __attribute__ ((section (".text.3")))
+frame3 (void)
+{
+  asm ("frame3_label: .globl frame3_label");
+}
+
+void __attribute__ ((section (".text.2")))
+frame2 (void)
+{
+  asm ("frame2_label: .globl frame2_label");
+  frame3 ();
+}
+
+void __attribute__ ((section (".text.1")))
+main (void)
+{
+  asm ("main_label: .globl main_label");
+  frame2 ();
+}
+
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
new file mode 100644
index 0000000..ae891c3
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
@@ -0,0 +1,143 @@ 
+# Copyright 2015 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
+
+# Test DW_TAG_compile_unit with no children and with neither DW_AT_low_pc nor
+# DW_AT_high_pc but with DW_AT_ranges instead.
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    verbose "Skipping DW_AT_ranges test."
+    return 0
+}
+
+# The .c files use __attribute__.
+if [get_compiler_info] {
+    return -1
+}
+if !$gcc_compiled {
+    verbose "Skipping DW_AT_ranges test."
+    return 0
+}
+
+standard_testfile dw2-ranges-base.c dw2-ranges-base-dw.S
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    global srcdir subdir srcfile srcfile2
+    declare_labels ranges_label;
+    declare_labels L;
+
+    # Find start address and length for our functions.
+    set main_func \
+	[function_range main [list ${srcdir}/${subdir}/$srcfile]]
+    set frame2_func \
+	[function_range frame2 [list ${srcdir}/${subdir}/$srcfile]]
+    set frame3_func \
+	[function_range frame3 [list ${srcdir}/${subdir}/$srcfile]]
+
+    # Very simple info for this test program.  We don't care about
+    # this information being correct (w.r.t. funtion / argument types)
+    # just so long as the compilation using makes use of the
+    # .debug_ranges data then the test achieves its objective.
+    cu {} {
+	compile_unit {
+	    {language @DW_LANG_C}
+	    {name dw-ranges-base.c}
+	    {stmt_list $L DW_FORM_sec_offset}
+	    {ranges ${ranges_label} DW_FORM_sec_offset}
+	} {
+	    subprogram {
+		{external 1 flag}
+		{name main}
+	    }
+	    subprogram {
+		{external 1 flag}
+		{name frame2}
+	    }
+	    subprogram {
+		{external 1 flag}
+		{name frame3}
+	    }
+	}
+    }
+
+    lines {version 2} L {
+	include_dir "${srcdir}/${subdir}"
+	file_name "$srcfile" 1
+
+	# Generate simple line table program.  The line table
+	# information contained here is not correct, and we really
+	# don't care, just so long as each function has some line
+	# table data associated with it.  We do make use of the fake
+	# line numbers that we pick here in the tests below.
+	program {
+	    {DW_LNE_set_address [lindex $main_func 0]}
+	    {DW_LNS_advance_line 10}
+	    {DW_LNS_copy}
+	    {DW_LNS_advance_pc [lindex $main_func 1]}
+	    {DW_LNS_advance_line 19}
+	    {DW_LNS_copy}
+	    {DW_LNE_end_sequence}
+
+	    {DW_LNE_set_address [lindex $frame2_func 0]}
+	    {DW_LNS_advance_line 20}
+	    {DW_LNS_copy}
+	    {DW_LNS_advance_pc [lindex $frame2_func 1]}
+	    {DW_LNS_advance_line 29}
+	    {DW_LNS_copy}
+	    {DW_LNE_end_sequence}
+
+	    {DW_LNE_set_address [lindex $frame3_func 0]}
+	    {DW_LNS_advance_line 30}
+	    {DW_LNS_copy}
+	    {DW_LNS_advance_pc [lindex $frame3_func 1]}
+	    {DW_LNS_advance_line 39}
+	    {DW_LNS_copy}
+	    {DW_LNE_end_sequence}
+	}
+    }
+
+    # Generate ranges data.  This is the point of this whole test
+    # file, we must have multiple bases specified, so we use a new
+    # base for each function.
+    ranges {is_64 [is_64_target]} {
+	ranges_label: sequence {
+	    {base [lindex $main_func 0]}
+	    {range 0 [lindex $main_func 1]}
+	    {base [lindex $frame2_func 0]}
+	    {range 0 [lindex $frame2_func 1]}
+	    {base [lindex $frame3_func 0]}
+	    {range 0 [lindex $frame3_func 1]}
+	}
+    }
+}
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+# Make use of the line numbers we faked in the .debug_line table above.
+gdb_test "info line main" \
+    "Line 11 of .* starts at address .* and ends at .*"
+gdb_test "info line frame2" \
+    "Line 21 of .* starts at address .* and ends at .*"
+gdb_test "info line frame3" \
+    "Line 31 of .* starts at address .* and ends at .*"
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index 5dc7ea8..03b59ce 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -303,6 +303,15 @@  namespace eval Dwarf {
     # Whether a file_name entry was seen.
     variable _line_saw_file
 
+    # Whether a line table program has been seen.
+    variable _line_saw_program
+
+    # A Label for line table header generation.
+    variable _line_header_end_label
+
+    # The address size for debug ranges section.
+    variable _debug_ranges_64_bit
+
     proc _process_one_constant {name value} {
 	variable _constants
 	variable _AT
@@ -981,7 +990,7 @@  namespace eval Dwarf {
 	set _cu_label [_compute_label "cu${cu_num}_begin"]
 	set start_label [_compute_label "cu${cu_num}_start"]
 	set end_label [_compute_label "cu${cu_num}_end"]
-	
+
 	define_label $_cu_label
 	if {$is_64} {
 	    _op .4byte 0xffffffff
@@ -1118,6 +1127,78 @@  namespace eval Dwarf {
 	define_label $end_label
     }
 
+    # Emit a DWARF .debug_ranges unit.
+    # OPTIONS is a list with an even number of elements containing
+    # option-name and option-value pairs.
+    # Current options are:
+    # is_64 0|1    - boolean indicating if you want to emit 64-bit DWARF
+    #                default = 0 (32-bit)
+    #
+    # BODY is Tcl code that emits the content of the .debug_ranges
+    # unit, it is evaluated in the caller's context.
+    proc ranges {options body} {
+	variable _debug_ranges_64_bit
+
+	foreach { name value } $options {
+	    switch -exact -- $name {
+		is_64 { set _debug_ranges_64_bit [subst $value] }
+		default { error "unknown option $name" }
+	    }
+	}
+
+	set section ".debug_ranges"
+	_section $section
+
+	proc sequence {{ranges {}}} {
+	    variable _debug_ranges_64_bit
+
+	    # Emit the sequence of addresses.
+	    set base ""
+	    foreach range $ranges {
+		set range [uplevel 1 "subst \"$range\""]
+		set type [lindex $range 0]
+		switch -exact -- $type {
+		    base {
+			set base [lrange $range 1 end]
+
+			if { $_debug_ranges_64_bit } then {
+			    _op .8byte 0xffffffffffffffff "Base Marker"
+			    _op .8byte $base "Base Address"
+			} else {
+			    _op .4byte 0xffffffff "Base Marker"
+			    _op .4byte $base "Base Address"
+			}
+		    }
+		    range {
+			set start [lindex $range 1]
+			set end [lrange $range 2 end]
+
+			if { $_debug_ranges_64_bit } then {
+			    _op .8byte $start "Start Address"
+			    _op .8byte $end "End Address"
+			} else {
+			    _op .4byte $start "Start Address"
+			    _op .4byte $end "End Address"
+			}
+		    }
+		    default { error "unknown range type: $type " }
+		}
+	    }
+
+	    # End of the sequence.
+	    if { $_debug_ranges_64_bit } then {
+		_op .8byte 0x0 "End of Sequence Marker (Part 1)"
+		_op .8byte 0x0 "End of Sequence Marker (Part 2)"
+	    } else {
+		_op .4byte 0x0 "End of Sequence Marker (Part 1)"
+		_op .4byte 0x0 "End of Sequence Marker (Part 2)"
+	    }
+	}
+
+	uplevel $body
+    }
+
+
     # Emit a DWARF .debug_line unit.
     # OPTIONS is a list with an even number of elements containing
     # option-name and option-value pairs.
@@ -1146,6 +1227,8 @@  namespace eval Dwarf {
     proc lines {options label body} {
 	variable _line_count
 	variable _line_saw_file
+	variable _line_saw_program
+	variable _line_header_end_label
 
 	# Establish the defaults.
 	set is_64 0
@@ -1181,7 +1264,7 @@  namespace eval Dwarf {
 	set unit_len_label [_compute_label "line${_line_count}_start"]
 	set unit_end_label [_compute_label "line${_line_count}_end"]
 	set header_len_label [_compute_label "line${_line_count}_header_start"]
-	set header_end_label [_compute_label "line${_line_count}_header_end"]
+	set _line_header_end_label [_compute_label "line${_line_count}_header_end"]
 
 	if {$is_64} {
 	    _op .4byte 0xffffffff
@@ -1195,20 +1278,34 @@  namespace eval Dwarf {
 	_op .2byte $_unit_version version
 
 	if {$is_64} {
-	    _op .8byte "$header_end_label - $header_len_label" "header_length"
+	    _op .8byte "$_line_header_end_label - $header_len_label" "header_length"
 	} else {
-	    _op .4byte "$header_end_label - $header_len_label" "header_length"
+	    _op .4byte "$_line_header_end_label - $header_len_label" "header_length"
 	}
 
 	define_label $header_len_label
 
 	_op .byte 1 "minimum_instruction_length"
-	_op .byte 0 "default_is_stmt"
+	_op .byte 1 "default_is_stmt"
 	_op .byte 1 "line_base"
 	_op .byte 1 "line_range"
-	_op .byte 1 "opcode_base"
-	# Since we emit opcode_base==1, we skip
-	# standard_opcode_length table altogether.
+	_op .byte 10 "opcode_base"
+
+	# The standard_opcode_lengths table.  The number of arguments
+	# for each of the standard opcodes.  Generating 9 entries here
+	# matches the use of 10 in the opcode_base above.  These 9
+	# entries match the 9 standard opcodes for DWARF2, making use
+	# of only 9 should be fine, even if we are generating DWARF3
+	# or DWARF4.
+	_op .byte 0 "standard opcode 1"
+	_op .byte 1 "standard opcode 2"
+	_op .byte 1 "standard opcode 3"
+	_op .byte 1 "standard opcode 4"
+	_op .byte 1 "standard opcode 5"
+	_op .byte 0 "standard opcode 6"
+	_op .byte 0 "standard opcode 7"
+	_op .byte 0 "standard opcode 8"
+	_op .byte 1 "standard opcode 9"
 
 	proc include_dir {dirname} {
 	    _op .ascii [_quote $dirname]
@@ -1228,6 +1325,57 @@  namespace eval Dwarf {
 	    _op .sleb128 0 "length"
 	}
 
+	proc program {statements} {
+	    variable _line_saw_program
+	    variable _line_header_end_label
+
+	    if "! $_line_saw_program" {
+		# Terminate the file list.
+		_op .byte 0 "Terminator."
+		define_label $_line_header_end_label
+		set _line_saw_program 1
+	    }
+
+	    proc DW_LNE_set_address {addr} {
+		_op .byte 0
+		set start [new_label "set_address_start"]
+		set end [new_label "set_address_end"]
+		_op .uleb128 "${end} - ${start}"
+		define_label ${start}
+		_op .byte 2
+		if {[is_64_target]} {
+		    _op .8byte ${addr}
+		} else {
+		    _op .4byte ${addr}
+		}
+		define_label ${end}
+	    }
+
+	    proc DW_LNE_end_sequence {} {
+		_op .byte 0
+		_op .uleb128 1
+		_op .byte 1
+	    }
+
+	    proc DW_LNS_copy {} {
+		_op .byte 1
+	    }
+
+	    proc DW_LNS_advance_pc {offset} {
+		_op .byte 2
+		_op .uleb128 ${offset}
+	    }
+
+	    proc DW_LNS_advance_line {offset} {
+		_op .byte 3
+		_op .sleb128 ${offset}
+	    }
+
+	    foreach statement $statements {
+		uplevel 1 $statement
+	    }
+	}
+
 	uplevel $body
 
 	rename include_dir ""
@@ -1239,9 +1387,11 @@  namespace eval Dwarf {
 	}
 
 	# Terminate the file list.
-	_op .byte 0 "Terminator."
+	if "! $_line_saw_program" {
+	    _op .byte 0 "Terminator."
+	    define_label $_line_header_end_label
+	}
 
-	define_label $header_end_label
 	define_label $unit_end_label
     }
 
@@ -1326,6 +1476,9 @@  namespace eval Dwarf {
 	variable _cu_count
 	variable _line_count
 	variable _line_saw_file
+	variable _line_saw_program
+	variable _line_header_end_label
+	variable _debug_ranges_64_bit
 
 	if {!$_initialized} {
 	    _read_constants
@@ -1341,6 +1494,8 @@  namespace eval Dwarf {
 
 	set _line_count 0
 	set _line_saw_file 0
+	set _line_saw_program 0
+	set _debug_ranges_64_bit [is_64_target]
 
 	# Not "uplevel" here, because we want to evaluate in this
 	# namespace.  This is somewhat bad because it means we can't