Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@polymtl.ca>
To: Andreas Arnez <arnez@linux.vnet.ibm.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH 1/9] Add test for modifiable DWARF locations
Date: Thu, 13 Apr 2017 04:00:00 -0000	[thread overview]
Message-ID: <aa985689fba9e00347e94188cb9c594c@polymtl.ca> (raw)
In-Reply-To: <1491586736-21296-2-git-send-email-arnez@linux.vnet.ibm.com>

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


  reply	other threads:[~2017-04-13  4:00 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-07 17:39 [PATCH 0/9] Various DWARF piece fixes Andreas Arnez
2017-04-07 17:39 ` [PATCH 1/9] Add test for modifiable DWARF locations Andreas Arnez
2017-04-13  4:00   ` Simon Marchi [this message]
2017-04-13 10:52     ` Andreas Arnez
2017-04-13  8:36   ` Yao Qi
2017-04-13 11:46     ` Andreas Arnez
2017-04-07 17:40 ` [PATCH 2/9] Fix size capping in write_pieced_value Andreas Arnez
2017-04-13  8:18   ` Yao Qi
2017-04-13 16:35     ` Andreas Arnez
2017-04-19  9:15       ` Yao Qi
2017-04-19 14:36         ` Andreas Arnez
2017-04-19 15:00           ` Yao Qi
2017-04-07 17:41 ` [PATCH 3/9] PR gdb/21226: Take DWARF stack value pieces from LSB end Andreas Arnez
2017-04-14  3:36   ` Simon Marchi
2017-04-18 16:32     ` Andreas Arnez
2017-04-18 16:43       ` Simon Marchi
2017-04-07 17:41 ` [PATCH 4/9] Remove addr_size field from struct piece_closure Andreas Arnez
2017-04-13  9:10   ` Yao Qi
2017-04-14  3:39     ` Simon Marchi
2017-04-18 17:25       ` Andreas Arnez
2017-04-18 18:49         ` Simon Marchi
2017-04-07 17:42 ` [PATCH 5/9] Fix issues in write_pieced_value when targeting bit-fields Andreas Arnez
2017-04-14  5:18   ` Simon Marchi
2017-04-27 17:54     ` Andreas Arnez
2017-05-03 13:59       ` Simon Marchi
2017-04-07 17:43 ` [PATCH 6/9] Fix handling of DWARF register pieces on big-endian targets Andreas Arnez
2017-04-14 14:11   ` Simon Marchi
2017-04-19 18:03     ` Andreas Arnez
2017-04-07 17:43 ` [PATCH 7/9] Improve logic for buffer allocation in read/write_pieced_value Andreas Arnez
2017-04-14 14:51   ` Simon Marchi
2017-04-07 17:44 ` [PATCH 8/9] Respect piece offset for DW_OP_bit_piece Andreas Arnez
2017-04-14 15:07   ` Simon Marchi
2017-04-07 17:45 ` [PATCH 9/9] Remove unnecessary copies of variables in read/write_pieced_value Andreas Arnez
2017-04-14 15:21   ` Simon Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aa985689fba9e00347e94188cb9c594c@polymtl.ca \
    --to=simon.marchi@polymtl.ca \
    --cc=arnez@linux.vnet.ibm.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox