[PATCH] gdb: handle undefined properties in ada_discrete_type_{low, high}_bound

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] gdb: handle undefined properties in ada_discrete_type_{low, high}_bound

Sourceware - gdb-patches mailing list
This patch fixes a failure in test `gdb.ada/access_to_packed_array.exp`.
The failure was introduced by 8c2e4e0689ea24 ("gdb: add accessors to
struct dynamic_prop"), but I think it in fact exposed a latent buglet.

Note that to reproduce it, I had to use AdaCore's Ada "distribution"
[1].  The one that comes with my distro doesn't have debug info for the
standard library stuff, so the bug wouldn't trigger.

The bug is that while executing the `maint print symbols` command, we
are accessing the value of a range type's high bound dynamic prop as a
"const" value (PROP_CONST), when it is actually undefined
(PROP_UNDEFINED).  It results in this failed assertion:

    /home/simark/src/binutils-gdb/gdb/gdbtypes.h:526: internal-error: LONGEST dynamic_prop::const_val() const: Assertion `m_kind == PROP_CONST' failed.

`ada_discrete_type_high_bound` calls `resolve_dynamic_type`, which
eventually calls `resolve_dynamic_range`.  This one is responsible for
evaluating a range type's dynamic bounds in the current context and
returning static values.  It returns a new range type with these static
bounds.

The resulting bounds are typically properties of the PROP_CONST kind.
But when it's not possible to evaluate the properties, the properties
are PROP_UNDEFINED.  In the case we are looking at, it's not possible to
evaluate the dynamic high bound, which is of type PROP_LOCLIST.  It
would require a target with registers and a frame, but we run `maint
print symbols` without a live process.

`ada_discrete_type_high_bound` then accesses the high bound
unconditionally as a const value, which triggers the assert.

Note that the previous code in resolve_dynamic_range (before commit
8c2e4e0689ea24) did this:

    prop = &TYPE_RANGE_DATA (dyn_range_type)->high;
    if (dwarf2_evaluate_property (prop, NULL, addr_stack, &value))
      {
        high_bound.kind = PROP_CONST;
        high_bound.data.const_val = value;

        if (TYPE_RANGE_DATA (dyn_range_type)->flag_upper_bound_is_count)
  high_bound.data.const_val
   = low_bound.data.const_val + high_bound.data.const_val - 1;
      }
    else
      {
        high_bound.kind = PROP_UNDEFINED;
        high_bound.data.const_val = 0;
      }

That did not really made sense, setting the kind to `PROP_UNDEFINED` but
also setting the `const_val` field.  The `const_val` field is only
meaningful if the kind if `PROP_CONST`.  The new code
(post-8c2e4e0689ea24) simply calls `set_undefined ()`.

Fix this by making the caller, `ada_discrete_type_high_bound`, consider
that a range high bound could be of kind `PROP_UNDEFINED`, and return
0 in this case.  I made the same change in ada_discrete_type_low_bound.
I didn't encounter a problem with this function, but the same could in
theory happen there.

Returning 0 here is kind of a lie, but the goal here is just to restore
the behavior of pre-8c2e4e0689ea24.

The output of `maint print symbols` is:

     typedef <ada__exceptions__exception_data__append_info_basic_exception_information__TTnameSP1: range 1 .. 0;
     record
         ada__exceptions__exception_data__append_info_basic_exception_information__TTnameSP1: range 1 .. 0;
     end record;

Instead of `1 .. 0`, which does not make sense, we could say something
like `1 .. <dynamic>`.  But that would require more changes than I'm
willing to do at the moment.

[1] https://www.adacore.com/download

gdb/ChangeLog:

        PR ada/26235
        * gdbtypes.c (ada_discrete_type_low_bound,
        ada_discrete_type_high_bound): Handle undefined bounds.

Change-Id: Ia12167e61ef030941c0790f83294f3418e6a7c12
---
 gdb/ada-lang.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 8b437a2a9cdc..2be3fe45b35d 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -725,7 +725,21 @@ ada_discrete_type_high_bound (struct type *type)
   switch (type->code ())
     {
     case TYPE_CODE_RANGE:
-      return type->bounds ()->high.const_val ();
+      {
+ const dynamic_prop &high = type->bounds ()->high;
+
+ if (high.kind () == PROP_CONST)
+  return high.const_val ();
+ else
+  {
+    gdb_assert (high.kind () == PROP_UNDEFINED);
+
+    /* This happens when trying to evaluate a type's dynamic bound
+       without a live target.  There is nothing relevant for us to
+       return here, so return 0.  */
+    return 0;
+  }
+      }
     case TYPE_CODE_ENUM:
       return TYPE_FIELD_ENUMVAL (type, type->num_fields () - 1);
     case TYPE_CODE_BOOL:
@@ -746,7 +760,21 @@ ada_discrete_type_low_bound (struct type *type)
   switch (type->code ())
     {
     case TYPE_CODE_RANGE:
-      return type->bounds ()->low.const_val ();
+      {
+ const dynamic_prop &low = type->bounds ()->low;
+
+ if (low.kind () == PROP_CONST)
+  return low.const_val ();
+ else
+  {
+    gdb_assert (low.kind () == PROP_UNDEFINED);
+
+    /* This happens when trying to evaluate a type's dynamic bound
+       without a live target.  There is nothing relevant for us to
+       return here, so return 0.  */
+    return 0;
+  }
+      }
     case TYPE_CODE_ENUM:
       return TYPE_FIELD_ENUMVAL (type, 0);
     case TYPE_CODE_BOOL:
--
2.27.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gdb: handle undefined properties in ada_discrete_type_{low, high}_bound

Tom Tromey-2
>>>>> "Simon" == Simon Marchi via Gdb-patches <[hidden email]> writes:

Simon> gdb/ChangeLog:

Simon> PR ada/26235
Simon> * gdbtypes.c (ada_discrete_type_low_bound,
Simon> ada_discrete_type_high_bound): Handle undefined bounds.

Thanks, I think this is ok.

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gdb: handle undefined properties in ada_discrete_type_{low, high}_bound

Sourceware - gdb-patches mailing list
On 2020-07-21 2:16 p.m., Tom Tromey wrote:

>>>>>> "Simon" == Simon Marchi via Gdb-patches <[hidden email]> writes:
>
> Simon> gdb/ChangeLog:
>
> Simon> PR ada/26235
> Simon> * gdbtypes.c (ada_discrete_type_low_bound,
> Simon> ada_discrete_type_high_bound): Handle undefined bounds.
>
> Thanks, I think this is ok.
>
> Tom
>

Thanks, pushed.

Simon