gdb: handle undefined properties in ada_discrete_type_{low,high}_bound
authorSimon Marchi <simon.marchi@polymtl.ca>
Tue, 21 Jul 2020 19:12:56 +0000 (15:12 -0400)
committerSimon Marchi <simon.marchi@efficios.com>
Tue, 21 Jul 2020 19:13:21 +0000 (15:13 -0400)
commitd1fd641e0b48e75401311528450eb1ddfb3a1fa0
tree24b015596600fcbe53cc68ebbe056675a57952ad
parent6d3d6e4ba779dc08b134cd1a09b055dbd88dbf8a
gdb: handle undefined properties in ada_discrete_type_{low,high}_bound

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/ChangeLog
gdb/ada-lang.c