RE: [PATCH v2 1/1] gdb/dwarf2: Fix 'rw_pieced_value' for values casted to different type.

["Rohr, Stephan" <stephan.rohr@intel.com>]

Hi all,

I would like to ask for feedback regarding my patch.

Thank you
stephan

> -----Original Message-----
> From: Rohr, Stephan <stephan.rohr@intel.com>
> Sent: Tuesday, October 25, 2022 3:42 PM
> To: gdb-patches@sourceware.org
> Cc: tom@tromey.com; simark@simark.ca; Rohr, Stephan
> <stephan.rohr@intel.com>
> Subject: [PATCH v2 1/1] gdb/dwarf2: Fix 'rw_pieced_value' for values casted
> to different type.
> 
> From: "Rohr, Stephan" <stephan.rohr@intel.com>
> 
> The 'rw_pieced_value' function is executed when fetching a (lazy) variable
> described by 'DW_OP_piece' or 'DW_OP_bit_piece'.  The function checks the
> 'type' and 'enclosing_type' fields of the value for identity.
> 
>   * The 'type' field describes the type of a value.
>   * In most cases, the 'enclosing_type' field is identical to the
>     'type' field.
>   * Scenarios where the 'type' and 'enclosing_type' of an object
>     differ are described in 'gdb/value.c'.  Possible cases are:
>   * If a value represents a C++ object, then the 'type' field
>     gives the object's compile-time type.  If the object actually
>     belongs to some class derived from `type', perhaps with other
>     base classes and additional members, then `type' is just a
>     subobject of the real thing, and the full object is probably
>     larger than `type' would suggest.
>   * If 'type' is a dynamic class (i.e. one with a vtable), then GDB
>     can actually determine the object's run-time type by looking at
>     the run-time type information in the vtable.  GDB may then elect
>     to read the entire object.
>   * If the user casts a variable to a different type
>     (e.g. 'print (<type> []) <variable>'), the value's type is
>     updated before reading the value.
> 
> If a lazy value is fetched, GDB allocates space based on the enclosing type's
> length and typically reads the 'full' object.  This is not implemented for pieced
> values and causes an internal error if 'type'
> and 'enclosing_type' of a value are not identical.
> 
> However, GDB can read the value based on its type.  Thus, this patch fixes
> the previously mentioned cases by removing the check for identity.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28605
> 
> gdb/ChangeLog:
> 2022-04-13  Stephan Rohr  <stephan.rohr@intel.com>
> 
> 	* dwarf2/loc.c (rw_pieced_value): Fix check on 'type' and
> 	'enlcosing_type' when reading pieced value 'v'.
> 
> gdb/testsuite/ChangeLog:
> 2022-04-13  Stephan Rohr  <stephan.rohr@intel.com>
> 
> 	* gdb.dwarf2/shortpiece.exp: Added test cases.
> ---
>  gdb/dwarf2/expr.c                       |  3 --
>  gdb/testsuite/gdb.dwarf2/shortpiece.exp | 50
> ++++++++++++++++++++++++-
>  2 files changed, 49 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c index
> 73dfd4b4ffb..fa08ad89024 100644
> --- a/gdb/dwarf2/expr.c
> +++ b/gdb/dwarf2/expr.c
> @@ -161,9 +161,6 @@ rw_pieced_value (value *v, value *from, bool
> check_optimized)
>      }
>    else
>      {
> -      if (value_type (v) != value_enclosing_type (v))
> -	internal_error (_("Should not be able to create a lazy value with "
> -			  "an enclosing type"));
>        if (check_optimized)
>  	v_contents = nullptr;
>        else
> diff --git a/gdb/testsuite/gdb.dwarf2/shortpiece.exp
> b/gdb/testsuite/gdb.dwarf2/shortpiece.exp
> index f5a933e521b..f1808bff5be 100644
> --- a/gdb/testsuite/gdb.dwarf2/shortpiece.exp
> +++ b/gdb/testsuite/gdb.dwarf2/shortpiece.exp
> @@ -29,7 +29,7 @@ Dwarf::assemble $asm_file {
> 
>      cu { addr_size 4 } {
>  	compile_unit {} {
> -	    declare_labels int_label ushort_label struct_label
> +	    declare_labels int_label ushort_label struct_label struct_label_2
> 
>  	    int_label: DW_TAG_base_type {
>  		{DW_AT_byte_size 4 DW_FORM_udata}
> @@ -59,6 +59,23 @@ Dwarf::assemble $asm_file {
>  		}
>  	    }
> 
> +	    struct_label_2: DW_TAG_structure_type {
> +		{DW_AT_name "S_2"}
> +		{DW_AT_byte_size 6 DW_FORM_udata}
> +	    } {
> +		DW_TAG_member {
> +		    {DW_AT_name "a"}
> +		    {DW_AT_type :${int_label}}
> +		    {DW_AT_data_member_location 0 DW_FORM_udata}
> +		}
> +
> +		DW_TAG_member {
> +		    {DW_AT_name "b"}
> +		    {DW_AT_type :${ushort_label}}
> +		    {DW_AT_data_member_location 4 DW_FORM_udata}
> +		}
> +	    }
> +
>  	    DW_TAG_variable {
>  		{DW_AT_name "s1"}
>  		{DW_AT_type :${struct_label}}
> @@ -86,6 +103,20 @@ Dwarf::assemble $asm_file {
>  		    DW_OP_piece 8
>  		} SPECIAL_expr}
>  	    }
> +
> +	    DW_TAG_variable {
> +		{DW_AT_name "s3"}
> +		{DW_AT_type :${struct_label_2}}
> +		{DW_AT_external 1 DW_FORM_flag}
> +		{DW_AT_location {
> +		    DW_OP_constu 0
> +		    DW_OP_stack_value
> +		    DW_OP_piece 4
> +		    DW_OP_constu 1
> +		    DW_OP_stack_value
> +		    DW_OP_piece 2
> +		} SPECIAL_expr}
> +	    }
>  	}
>      }
>  }
> @@ -98,3 +129,20 @@ if { [prepare_for_testing "failed to prepare" ${testfile}
> \  gdb_test "p s1" " = {a = 1, b = 0}"
>  gdb_test "p s2" \
>      "access outside bounds of object referenced via synthetic pointer"
> +
> +# When fetching a lazy value, GDB typically tries to fetch the 'full'
> +# object based on the enclosing type.  GDB does not support the reading
> +# of a pieced value with a (possibly larger) enclosing type.  However,
> +# the user may want to print a value casted to a different type, # e.g.
> +print (<type> []) <variable>.  This cast causes an update of the #
> +value's type.  In case of a pieced value, GDB failed to fetch the #
> +value's content.
> +# This test verifies that GDB can print a pieced value casted to a #
> +different type.
> +gdb_test "p (int \[\]) s1" " = \\{1\\, 0\\}"
> +gdb_test "p (short \[\]) s1" " = \\{1\\, 0\\, 0\\, <synthetic pointer>\\}"
> +
> +# Test for correct output if the size of the original object is not a #
> +multiple of the array's element size.
> +gdb_test "p s3" " = {a = 0, b = 1}"
> +gdb_test "p (int \[\]) s3" " = \\{0\\}"
> --
> 2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928