RE: [PATCH 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 review of my patch.

Best
stephan

> -----Original Message-----
> From: Rohr, Stephan <stephan.rohr@intel.com>
> Sent: Friday, April 22, 2022 4:44 PM
> To: gdb-patches@sourceware.org
> Cc: Rohr, Stephan <stephan.rohr@intel.com>; tom@tromey.com
> Subject: [PATCH 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, it should be
> sufficient to check if the type's length (potentially shifted by
> 'embedded_offset') does not exceed the enclosing type's length which was
> used for memory allocation.
> ---
>  gdb/dwarf2/expr.c                       |  6 ++----
>  gdb/testsuite/gdb.dwarf2/shortpiece.exp | 12 ++++++++++++
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c index
> 99862583336..6330a5787fc 100644
> --- a/gdb/dwarf2/expr.c
> +++ b/gdb/dwarf2/expr.c
> @@ -174,10 +174,8 @@ rw_pieced_value (value *v, value *from, bool
> check_optimized)
>      }
>    else
>      {
> -      if (value_type (v) != value_enclosing_type (v))
> -	internal_error (__FILE__, __LINE__,
> -			_("Should not be able to create a lazy value with "
> -			  "an enclosing type"));
> +      gdb_assert ((TYPE_LENGTH (value_type (v)) + value_embedded_offset
> (v))
> +		  <= TYPE_LENGTH (value_enclosing_type (v)));
>        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..19cdec83193 100644
> --- a/gdb/testsuite/gdb.dwarf2/shortpiece.exp
> +++ b/gdb/testsuite/gdb.dwarf2/shortpiece.exp
> @@ -98,3 +98,15 @@ 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>\\}"
> --
> 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