[1/9] Add test for modifiable DWARF locations

  This adds a test for read/write access to variables with various types of
DWARF locations.  It uses register- and memory locations and composite
locations with register- and memory pieces.

Since the new test calls gdb_test_no_output with commands that contain
braces, it is necessary for string_to_regexp to quote braces as well.
This was not done before.

gdb/testsuite/ChangeLog:

	* gdb.dwarf2/var-access.c: New file.
	* gdb.dwarf2/var-access.exp: New test.
	* lib/gdb-utils.exp (string_to_regexp): Quote braces as well.
---
 gdb/testsuite/gdb.dwarf2/var-access.c   |  25 +++++
 gdb/testsuite/gdb.dwarf2/var-access.exp | 193 ++++++++++++++++++++++++++++++++
 gdb/testsuite/lib/gdb-utils.exp         |   2 +-
 3 files changed, 219 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/var-access.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/var-access.exp
  

On 2017-04-07 13:38, Andreas Arnez wrote:
> This adds a test for read/write access to variables with various types 
> of
> DWARF locations.  It uses register- and memory locations and composite
> locations with register- and memory pieces.
> 
> Since the new test calls gdb_test_no_output with commands that contain
> braces, it is necessary for string_to_regexp to quote braces as well.
> This was not done before.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.dwarf2/var-access.c: New file.
> 	* gdb.dwarf2/var-access.exp: New test.
> 	* lib/gdb-utils.exp (string_to_regexp): Quote braces as well.
> ---
>  gdb/testsuite/gdb.dwarf2/var-access.c   |  25 +++++
>  gdb/testsuite/gdb.dwarf2/var-access.exp | 193 
> ++++++++++++++++++++++++++++++++
>  gdb/testsuite/lib/gdb-utils.exp         |   2 +-
>  3 files changed, 219 insertions(+), 1 deletion(-)
>  create mode 100644 gdb/testsuite/gdb.dwarf2/var-access.c
>  create mode 100644 gdb/testsuite/gdb.dwarf2/var-access.exp
> 
> diff --git a/gdb/testsuite/gdb.dwarf2/var-access.c
> b/gdb/testsuite/gdb.dwarf2/var-access.c
> new file mode 100644
> index 0000000..108be82
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/var-access.c
> @@ -0,0 +1,25 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2017 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/>.  */
> +
> +char buf[] = {0, 1, 2, 3, 4, 5, 6, 7};
> +
> +int
> +main (void)
> +{
> +  asm volatile ("main_label: .globl main_label");
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.dwarf2/var-access.exp
> b/gdb/testsuite/gdb.dwarf2/var-access.exp
> new file mode 100644
> index 0000000..ee93b93
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/var-access.exp
> @@ -0,0 +1,193 @@
> +# Copyright 2017 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/>.
> +
> +# Test reading/writing variables with non-trivial DWARF locations.
> +
> +load_lib dwarf.exp
> +
> +# This test can only be run on targets which support DWARF-2 and use 
> gas.
> +if {![dwarf2_support]} {
> +    return 0
> +}
> +
> +# Choose suitable integer registers for the test.
> +
> +set dwarf_regnum {0 1}
> +
> +if { [istarget "aarch64*-*-*"] } {
> +    set regname {x0 x1}
> +} elseif { [istarget "arm*-*-*"]
> +	   || [istarget "s390*-*-*" ]
> +	   || [istarget "powerpc*-*-*"]
> +	   || [istarget "rs6000*-*-aix*"] } {
> +    set regname {r0 r1}
> +} elseif { [istarget "i?86-*-*"] } {
> +    set regname {eax edx}
> +} elseif { [istarget "x86_64-*-*"] } {
> +    set regname {rax rdx}
> +} else {
> +    verbose "Skipping tests for accessing DWARF-described variables."
> +    return
> +}
> +
> +standard_testfile .c ${gdb_test_file_name}-dw.S
> +
> +# Make some DWARF for the test.
> +
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> +    global srcdir subdir srcfile
> +    global dwarf_regnum regname
> +
> +    set main_func \
> +	[function_range main [list ${srcdir}/${subdir}/$srcfile]]
> +    set buf_var [gdb_target_symbol buf]
> +
> +    cu {} {
> +	DW_TAG_compile_unit {
> +		{DW_AT_name var-pieces-dw.c}
> +		{DW_AT_comp_dir /tmp}
> +	} {
> +	    declare_labels char_type_label
> +	    declare_labels int_type_label short_type_label
> +	    declare_labels array_a8_label struct_s_label
> +
> +	    char_type_label: base_type {
> +		{name "char"}
> +		{encoding @DW_ATE_unsigned_char}
> +		{byte_size 1 DW_FORM_sdata}
> +	    }
> +
> +	    int_type_label: base_type {
> +		{name "int"}
> +		{encoding @DW_ATE_signed}
> +		{byte_size 4 DW_FORM_sdata}
> +	    }
> +
> +	    array_a8_label: array_type {
> +		{type :$char_type_label}
> +	    } {
> +		subrange_type {
> +		    {type :$int_type_label}
> +		    {upper_bound 7 DW_FORM_udata}
> +		}
> +	    }
> +
> +	    struct_s_label: structure_type {
> +		{name "s"}
> +		{byte_size 4 DW_FORM_sdata}
> +	    } {
> +		member {
> +		    {name "a"}
> +		    {type :$char_type_label}
> +		    {data_member_location 0 DW_FORM_udata}
> +		}
> +		member {
> +		    {name "b"}
> +		    {type :$char_type_label}
> +		    {data_member_location 1 DW_FORM_udata}
> +		}
> +		member {
> +		    {name "c"}
> +		    {type :$char_type_label}
> +		    {data_member_location 2 DW_FORM_udata}
> +		}
> +		member {
> +		    {name "d"}
> +		    {type :$char_type_label}
> +		    {data_member_location 3 DW_FORM_udata}
> +		}
> +	    }
> +
> +	    DW_TAG_subprogram {
> +		{name "main"}
> +		{DW_AT_external 1 flag}
> +		{low_pc [lindex $main_func 0] DW_FORM_addr}
> +		{high_pc [lindex $main_func 1] DW_FORM_udata}
> +	    } {
> +		# Simple memory location.
> +		DW_TAG_variable {
> +		    {name "a"}
> +		    {type :$array_a8_label}
> +		    {location {
> +			addr $buf_var
> +		    } SPECIAL_expr}
> +		}
> +		# Memory pieces.
> +		DW_TAG_variable {
> +		    {name "s1"}
> +		    {type :$struct_s_label}
> +		    {location {
> +			addr $buf_var
> +			plus_uconst 2
> +			piece 2
> +			addr $buf_var
> +			piece 2
> +		    } SPECIAL_expr}
> +		}
> +		# Register- and memory pieces.
> +		DW_TAG_variable {
> +		    {name "s2"}
> +		    {type :$struct_s_label}
> +		    {location {
> +			regx [lindex $dwarf_regnum 0]
> +			piece 1
> +			addr "$buf_var + 4"
> +			piece 1
> +			regx [lindex $dwarf_regnum 1]
> +			piece 1
> +			addr "$buf_var + 5"
> +			piece 1
> +		    } SPECIAL_expr}
> +		}
> +	    }
> +	}
> +    }
> +}
> +
> +if { [prepare_for_testing ${testfile}.exp ${testfile} \
> +	  [list $srcfile $asm_file] {nodebug}] } {
> +    return -1
> +}
> +
> +if ![runto_main] {
> +    return -1
> +}
> +
> +# Byte-aligned memory pieces.
> +gdb_test "print/d s1" " = \\{a = 2, b = 3, c = 0, d = 1\\}" \
> +    "s1 == re-ordered buf"
> +gdb_test_no_output "set var s1.a = 63"
> +gdb_test "print/d s1" " = \\{a = 63, b = 3, c = 0, d = 1\\}" \
> +    "verify s1.a"
> +gdb_test "print/d a" " = \\{0, 1, 63, 3, 4, 5, 6, 7\\}" \
> +    "verify s1.a through a"
> +
> +# Byte-aligned register- and memory pieces.
> +gdb_test_no_output "set var \$[lindex $regname 0] = 81" \
> +    "init reg for s2.a"
> +gdb_test_no_output "set var \$[lindex $regname 1] = 28" \
> +    "init reg for s2.c"
> +gdb_test "print/d s2" " = \\{a = 81, b = 4, c = 28, d = 5\\}" \
> +    "initialized s2 from mem and regs"
> +gdb_test_no_output "set var s2.c += s2.a + s2.b - s2.d"
> +gdb_test "print/d s2" " = \\{a = 81, b = 4, c = 108, d = 5\\}" \
> +    "verify s2.c"
> +gdb_test "print/d \$[lindex $regname 1]" " = 108" \
> +    "verify s2.c through reg"
> +gdb_test_no_output "set var s2 = {191, 73, 231, 123}" \
> +    "re-initialize s2"
> +gdb_test "print/d s2"  " = \\{a = 191, b = 73, c = 231, d = 123\\}" \
> +    "verify re-initialized s2"
> diff --git a/gdb/testsuite/lib/gdb-utils.exp 
> b/gdb/testsuite/lib/gdb-utils.exp
> index ff1b24a..37abb14 100644
> --- a/gdb/testsuite/lib/gdb-utils.exp
> +++ b/gdb/testsuite/lib/gdb-utils.exp
> @@ -34,6 +34,6 @@ proc gdb_init_commands {} {
> 
>  proc string_to_regexp {str} {
>      set result $str
> -    regsub -all {[]*+.|()^$\[\\]} $str {\\&} result
> +    regsub -all {[]*+.|(){}^$\[\\]} $str {\\&} result
>      return $result
>  }

Hi Andreas,

To be honest, I didn't know what a DWARF piece was before, well, two 
hours ago.  I've spent some time reading about that, including your 
write-up on the problem [1] (very informative, thanks) and the 
implementation of read_pieced_value.  I can't say I have a very good 
grasp on the problem you are tackling, but I got enough to understand 
all the lines in this patch, and it looks good to me.

It will probably take someone with more experience about this stuff than 
me to review the more intricate parts of the series, but I'll keep on 
reading when I have time, even if it's just for my own 
knowledge/enjoyment :).  Although while going through read_pieced_value 
line by line, I've found some areas that could be improved.  I suspect 
that there might be some overlap with some of the following patches of 
your series, so I might be able to understand and review those.

Thanks,

Simon

[1] https://sourceware.org/ml/gdb/2016-01/msg00013.html
  

Andreas Arnez <arnez@linux.vnet.ibm.com> writes:

> This adds a test for read/write access to variables with various types of
> DWARF locations.  It uses register- and memory locations and composite
> locations with register- and memory pieces.

The last line should be put into the introduction comment of
gdb.dwarf2/var-access.exp.

> +
> +# Choose suitable integer registers for the test.
> +
> +set dwarf_regnum {0 1}
> +
> +if { [istarget "aarch64*-*-*"] } {

if { [is_aarch64_target] }

> +    set regname {x0 x1}
> +} elseif { [istarget "arm*-*-*"]

else if { [is_aarch32_target]

> +	   || [istarget "s390*-*-*" ]
> +	   || [istarget "powerpc*-*-*"]
> +	   || [istarget "rs6000*-*-aix*"] } {
> +    set regname {r0 r1}
> +} elseif { [istarget "i?86-*-*"] } {
> +    set regname {eax edx}
> +} elseif { [istarget "x86_64-*-*"] } {
> +    set regname {rax rdx}
> +} else {
> +    verbose "Skipping tests for accessing DWARF-described variables."
> +    return
> +}
> +
> +standard_testfile .c ${gdb_test_file_name}-dw.S
> +
> +# Make some DWARF for the test.
> +
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> +    global srcdir subdir srcfile
> +    global dwarf_regnum regname
> +
> +    set main_func \
> +	[function_range main [list ${srcdir}/${subdir}/$srcfile]]

> +
> +	    DW_TAG_subprogram {
> +		{name "main"}
> +		{DW_AT_external 1 flag}
> +		{low_pc [lindex $main_func 0] DW_FORM_addr}
> +		{high_pc [lindex $main_func 1] DW_FORM_udata}

You can use MACRO_AT_func, it can replace name, low_pc and high_pc.
Also, you don't need to call function_range above.

> +	    } {
> +		# Simple memory location.
> +		DW_TAG_variable {
> +		    {name "a"}
> +		    {type :$array_a8_label}
> +		    {location {
> +			addr $buf_var
> +		    } SPECIAL_expr}
> +		}
> +		# Memory pieces.

Nit: we can also describe where are these pieces in comments, like

                # Memory pieces, two bytes from &buf[0], and two
                # bytes from &buf[2].

> +		DW_TAG_variable {
> +		    {name "s1"}
> +		    {type :$struct_s_label}
> +		    {location {
> +			addr $buf_var
> +			plus_uconst 2
> +			piece 2
> +			addr $buf_var
> +			piece 2
> +		    } SPECIAL_expr}
> +		}
> +		# Register- and memory pieces.

Likewise.

OK with the changes.
  

On Thu, Apr 13 2017, Simon Marchi wrote:

> To be honest, I didn't know what a DWARF piece was before, well, two hours
> ago.  I've spent some time reading about that, including your write-up on
> the problem [1] (very informative, thanks) and the implementation of
> read_pieced_value.  I can't say I have a very good grasp on the problem
> you are tackling, but I got enough to understand all the lines in this
> patch, and it looks good to me.

Thanks for your interest in this topic; very much appreciated.  The more
people look at this, the better our chance of getting it right this
time.

> It will probably take someone with more experience about this stuff than
> me to review the more intricate parts of the series, but I'll keep on
> reading when I have time, even if it's just for my own knowledge/enjoyment
> :).  Although while going through read_pieced_value line by line, I've
> found some areas that could be improved.  I suspect that there might be
> some overlap with some of the following patches of your series, so I might
> be able to understand and review those.

Yeah, the follow-up patches rewrite most of read/write_pieced_value.

--
Andreas