From 5bd1ef568c63cbcf6ed99a083eeb18cf940871dd Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Tue, 18 Jun 2013 18:11:19 +0000 Subject: [PATCH] Fix PR symtab/15391 PR symtab/15391 is a failure with the DW_OP_GNU_implicit_pointer feature. I tracked it down to a logic error in read_pieced_value. The code truncates this_size_bits according to the type size and offset too early -- it should do it after taking bits_to_skip into account. This patch fixes the bug. While testing this, I also tripped across a latent bug because indirect_pieced_value does not sign-extend where needed. This patch fixes this bug as well. Finally, Pedro pointed out that a previous version implemented sign extension incorrectly. This version introduces a new gdb_sign_extend function for this. A couple of notes on this function: * It has the gdb_ prefix to avoid clashes with various libraries that felt free to avoid proper namespacing. There is a "sign_extend" function in a Tile GX header, in an SOM-related BFD header (and in sh64-tdep.c and as a macro in arm-wince-tdep.c, but those are ours...) * I looked at all the sign extensions in gdb and didn't see ones that I felt comfortable converting to use this function; in large part because I don't have a good way to test the conversion. Built and regtested on x86-64 Fedora 18. New test cases included; this required a minor addition to the DWARF assembler. Note that the DWARF CU made by implptrpiece.exp uses a funny pointer size in order to show the sign-extension bug on all platforms. * dwarf2loc.c (read_pieced_value): Truncate this_size_bits after taking bits_to_skip into account. Sign extend byte_offset. * utils.h (gdb_sign_extend): Declare. * utils.c (gdb_sign_extend): New function. * gdb.dwarf2/implptrpiece.exp: New file. * gdb.dwarf2/implptrconst.exp (d): New variable. Print d. * lib/dwarf2.exp (Dwarf::_location): Handle DW_OP_piece. --- gdb/ChangeLog | 8 ++ gdb/dwarf2loc.c | 15 ++- gdb/testsuite/ChangeLog | 7 ++ gdb/testsuite/gdb.dwarf2/implptrconst.exp | 9 ++ gdb/testsuite/gdb.dwarf2/implptrpiece.exp | 131 ++++++++++++++++++++++ gdb/testsuite/lib/dwarf.exp | 7 +- gdb/utils.c | 17 +++ gdb/utils.h | 5 + 8 files changed, 194 insertions(+), 5 deletions(-) create mode 100644 gdb/testsuite/gdb.dwarf2/implptrpiece.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 1743bfa1403..16fc3515442 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,11 @@ +2013-06-18 Tom Tromey + + PR symtab/15391: + * dwarf2loc.c (read_pieced_value): Truncate this_size_bits + after taking bits_to_skip into account. Sign extend byte_offset. + * utils.h (gdb_sign_extend): Declare. + * utils.c (gdb_sign_extend): New function. + 2013-06-18 Jan Kratochvil * dwarf2read.c (write_psymtabs_to_index): Ignore NULL PSYMTAB. diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c index 9e44096f575..bb600d1243c 100644 --- a/gdb/dwarf2loc.c +++ b/gdb/dwarf2loc.c @@ -1641,8 +1641,6 @@ read_pieced_value (struct value *v) bits_to_skip -= this_size_bits; continue; } - if (this_size_bits > type_len - offset) - this_size_bits = type_len - offset; if (bits_to_skip > 0) { dest_offset_bits = 0; @@ -1655,6 +1653,8 @@ read_pieced_value (struct value *v) dest_offset_bits = offset; source_offset_bits = 0; } + if (this_size_bits > type_len - offset) + this_size_bits = type_len - offset; this_size = (this_size_bits + source_offset_bits % 8 + 7) / 8; source_offset = source_offset_bits / 8; @@ -2087,8 +2087,15 @@ indirect_pieced_value (struct value *value) frame = get_selected_frame (_("No frame selected.")); - /* This is an offset requested by GDB, such as value subcripts. */ + /* This is an offset requested by GDB, such as value subscripts. + However, due to how synthetic pointers are implemented, this is + always presented to us as a pointer type. This means we have to + sign-extend it manually as appropriate. */ byte_offset = value_as_address (value); + if (TYPE_LENGTH (value_type (value)) < sizeof (LONGEST)) + byte_offset = gdb_sign_extend (byte_offset, + 8 * TYPE_LENGTH (value_type (value))); + byte_offset += piece->v.ptr.offset; gdb_assert (piece); baton @@ -2099,7 +2106,7 @@ indirect_pieced_value (struct value *value) if (baton.data != NULL) return dwarf2_evaluate_loc_desc_full (TYPE_TARGET_TYPE (type), frame, baton.data, baton.size, baton.per_cu, - piece->v.ptr.offset + byte_offset); + byte_offset); { struct obstack temp_obstack; diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 7c659ca210e..2c9e13a263d 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2013-06-18 Tom Tromey + + * gdb.dwarf2/implptrpiece.exp: New file. + * gdb.dwarf2/implptrconst.exp (d): New variable. + Print d. + * lib/dwarf2.exp (Dwarf::_location): Handle DW_OP_piece. + 2013-06-18 Tom Tromey * lib/selftest-support.exp (do_self_tests): Reject remote or diff --git a/gdb/testsuite/gdb.dwarf2/implptrconst.exp b/gdb/testsuite/gdb.dwarf2/implptrconst.exp index 1c89c43002e..4e822951c3b 100644 --- a/gdb/testsuite/gdb.dwarf2/implptrconst.exp +++ b/gdb/testsuite/gdb.dwarf2/implptrconst.exp @@ -73,6 +73,14 @@ Dwarf::assemble $asm_file { GNU_implicit_pointer $var_label 0 } SPECIAL_expr} } + + DW_TAG_variable { + {name d} + {type :$ptr_label} + {location { + GNU_implicit_pointer $var_label 2 + } SPECIAL_expr} + } } } } @@ -103,3 +111,4 @@ if ![runto_main] { } gdb_test "print *c" " = 114 'r'" +gdb_test "print d\[-2\]" " = 114 'r'" diff --git a/gdb/testsuite/gdb.dwarf2/implptrpiece.exp b/gdb/testsuite/gdb.dwarf2/implptrpiece.exp new file mode 100644 index 00000000000..3f14a66b6e6 --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/implptrpiece.exp @@ -0,0 +1,131 @@ +# Copyright 2013 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 . + +load_lib dwarf.exp + +# This test can only be run on targets which support DWARF-2 and use gas. +if {![dwarf2_support]} { + return 0 +} + +if { [skip_cplus_tests] } { continue } + +standard_testfile main.c implptrpiece-dw.S + +# Make some DWARF for the test. +set asm_file [standard_output_file $srcfile2] + +Dwarf::assemble $asm_file { + # Using a funny address size here and in the pointer type lets us + # also check for a sign-extension bug in the + # DW_OP_GNU_implicit_pointer code. + cu { addr_size 2 } { + compile_unit {} { + declare_labels struct_label short_type_label + declare_labels char_type_label ptr_label + declare_labels var_label + + struct_label: structure_type { + {name S} + {byte_size 4 DW_FORM_sdata} + } { + member { + {name a} + {type :$short_type_label} + {data_member_location 0 DW_FORM_sdata} + } + member { + {name b} + {type :$char_type_label} + {data_member_location 2 DW_FORM_sdata} + } + member { + {name c} + {type :$char_type_label} + {data_member_location 3 DW_FORM_sdata} + } + } + + short_type_label: base_type { + {name "short int"} + {encoding @DW_ATE_signed} + {byte_size 2 DW_FORM_sdata} + } + + char_type_label: base_type { + {name "signed char"} + {encoding @DW_ATE_signed} + {byte_size 1 DW_FORM_sdata} + } + + # See comment above to understand the pointer size. + ptr_label: pointer_type { + {byte_size 2 DW_FORM_sdata} + {type :$char_type_label} + } + + var_label: DW_TAG_variable { + {name s} + {type :$struct_label} + {location { + const1u 1 + stack_value + piece 2 + const1u 2 + stack_value + piece 1 + const1u 3 + stack_value + piece 1 + } SPECIAL_expr} + } + + DW_TAG_variable { + {name p} + {type :$ptr_label} + {location { + GNU_implicit_pointer $var_label 2 + } SPECIAL_expr} + } + } + } +} + +if {[gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile}1.o \ + object {nodebug}] != ""} { + return -1 +} + +if {[gdb_compile $asm_file ${binfile}2.o object {nodebug}] != ""} { + return -1 +} + +if {[gdb_compile [list ${binfile}1.o ${binfile}2.o] \ + "${binfile}" executable {}] != ""} { + return -1 +} + +# We need --readnow because otherwise we never read in the CU we +# created above. +set saved_gdbflags $GDBFLAGS +set GDBFLAGS "$GDBFLAGS -readnow" +clean_restart ${testfile} +set GDBFLAGS $saved_gdbflags + +if ![runto_main] { + return -1 +} + +gdb_test "print/d p\[-1\]" " = 0" diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp index 7efaacac907..5b19bb8d297 100644 --- a/gdb/testsuite/lib/dwarf.exp +++ b/gdb/testsuite/lib/dwarf.exp @@ -611,6 +611,7 @@ namespace eval Dwarf { variable _constants variable _cu_label variable _cu_addr_size + variable _cu_offset_size foreach line [split $body \n] { if {[lindex $line 0] == ""} { @@ -651,6 +652,10 @@ namespace eval Dwarf { _op .sleb128 [lindex $line 1] } + DW_OP_piece { + _op .uleb128 [lindex $line 1] + } + DW_OP_GNU_implicit_pointer { if {[llength $line] != 3} { error "usage: DW_OP_GNU_implicit_pointer LABEL OFFSET" @@ -658,7 +663,7 @@ namespace eval Dwarf { # Here label is a section offset. set label [lindex $line 1] - _op .${_cu_addr_size}byte $label + _op .${_cu_offset_size}byte $label _op .sleb128 [lindex $line 2] } diff --git a/gdb/utils.c b/gdb/utils.c index 18ee9bbf98b..f5c133974fb 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -3234,6 +3234,23 @@ align_down (ULONGEST v, int n) return (v & -n); } +/* See utils.h. */ + +LONGEST +gdb_sign_extend (LONGEST value, int bit) +{ + gdb_assert (bit >= 1 && bit <= 8 * sizeof (LONGEST)); + + if (((value >> (bit - 1)) & 1) != 0) + { + LONGEST signbit = ((LONGEST) 1) << (bit - 1); + + value = (value ^ signbit) - signbit; + } + + return value; +} + /* Allocation function for the libiberty hash table which uses an obstack. The obstack is passed as DATA. */ diff --git a/gdb/utils.h b/gdb/utils.h index 71ce8671895..9356658a49b 100644 --- a/gdb/utils.h +++ b/gdb/utils.h @@ -377,4 +377,9 @@ extern int myread (int, char *, int); extern ULONGEST align_up (ULONGEST v, int n); extern ULONGEST align_down (ULONGEST v, int n); +/* Sign extend VALUE. BIT is the (1-based) index of the bit in VALUE + to sign-extend. */ + +extern LONGEST gdb_sign_extend (LONGEST value, int bit); + #endif /* UTILS_H */ -- 2.30.2