[PATCH] gdb: handle non-const property types in ada_modulus (PR ada/26318)

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

[PATCH] gdb: handle non-const property types in ada_modulus (PR ada/26318)

Sourceware - gdb-patches mailing list
PR 26318 shows that running `maint print symbols` on an Ada binary,
compiled with an Ada distribution that includes debug info for the
standard library, triggers this assertion:

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

The problem is in particular when printing type
`system__object_reader__decoded_ada_name__TTdecodedSP1___XDL_0`, which
has a dynamic high bound (PROP_LOCLIST kind).  When printing a concrete
value of this type, this type gets resolved to a type with a constant
high bound, so ada_modulus can return this constant value.

However, when printing the dynamic range type on its own, such as with
`maint print symbols`, the high bound is still of kind PROP_LOCLIST.
When ada_modulus tries to access the property as a const value, the
assert triggers.

There's no sensible numerical value to return in this case.  Ideally,
ada_modulus would return something to the caller indicating that the
value is dynamic and therefore can't be returned as an integer.  The
callers would handle it, for example `maint print symbols` would say
that the high bound of the type is dynamic.

However, this patch implements the simpler fix of returning 0 in that
case.  It kind of restores the previous behavior of before we validated
the dynamic property kind in the getters, where we would just return
whatever random integer value was in `const_val`.  Except now it's
consistently 0.

This is what we had before we added dynamic property getters:

$ ./gdb -q ~/foo -ex "maint expand-symtabs" -ex "maint print symbols" -batch | grep 'typedef <system__object_reader__decoded_ada_name__TTdecodedSP1'
     typedef <system__object_reader__decoded_ada_name__TTdecodedSP1: mod 107820865988257;

and this is what we have now:

$ ./gdb -q ~/foo -ex "maint expand-symtabs" -ex "maint print symbols" -batch | grep 'typedef <system__object_reader__decoded_ada_name__TTdecodedSP1'
     typedef <system__object_reader__decoded_ada_name__TTdecodedSP1: mod 0;

The value 107820865988257 is the `baton` field of the property's union
interpreted as an integer, so a bogus value.

gdb/ChangeLog:

        PR ada/26318
        * ada-lang.c (ada_modulus): Return 0 if property is not of const
        kind.

Change-Id: I3f6d343a9c3cd7cd62a4fc591943a43541223d50
---
 gdb/ada-lang.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 2be3fe45b35d..29951528e5e5 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -11452,7 +11452,14 @@ ada_is_modular_type (struct type *type)
 ULONGEST
 ada_modulus (struct type *type)
 {
-  return (ULONGEST) type->bounds ()->high.const_val () + 1;
+  const dynamic_prop &high = type->bounds ()->high;
+
+  if (high.kind () == PROP_CONST)
+    return (ULONGEST) high.const_val () + 1;
+
+  /* If TYPE is unresolved, the high bound might be a location list.  Return
+     0, for lack of a better value to return.  */
+  return 0;
 }
 
 
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gdb: handle non-const property types in ada_modulus (PR ada/26318)

Tom de Vries
On 7/30/20 5:03 PM, Simon Marchi via Gdb-patches wrote:

> PR 26318 shows that running `maint print symbols` on an Ada binary,
> compiled with an Ada distribution that includes debug info for the
> standard library, triggers this assertion:
>
>     /home/simark/src/binutils-gdb/gdb/gdbtypes.h:526: internal-error: LONGEST dynamic_prop::const_val() const: Assertion `m_kind == PROP_CONST' failed.
>
> The problem is in particular when printing type
> `system__object_reader__decoded_ada_name__TTdecodedSP1___XDL_0`, which
> has a dynamic high bound (PROP_LOCLIST kind).  When printing a concrete
> value of this type, this type gets resolved to a type with a constant
> high bound, so ada_modulus can return this constant value.
>
> However, when printing the dynamic range type on its own, such as with
> `maint print symbols`, the high bound is still of kind PROP_LOCLIST.
> When ada_modulus tries to access the property as a const value, the
> assert triggers.
>
> There's no sensible numerical value to return in this case.  Ideally,
> ada_modulus would return something to the caller indicating that the
> value is dynamic and therefore can't be returned as an integer.  The
> callers would handle it, for example `maint print symbols` would say
> that the high bound of the type is dynamic.
>
> However, this patch implements the simpler fix of returning 0 in that
> case.  It kind of restores the previous behavior of before we validated
> the dynamic property kind in the getters, where we would just return
> whatever random integer value was in `const_val`.  Except now it's
> consistently 0.
>
> This is what we had before we added dynamic property getters:
>
> $ ./gdb -q ~/foo -ex "maint expand-symtabs" -ex "maint print symbols" -batch | grep 'typedef <system__object_reader__decoded_ada_name__TTdecodedSP1'
>      typedef <system__object_reader__decoded_ada_name__TTdecodedSP1: mod 107820865988257;
>
> and this is what we have now:
>
> $ ./gdb -q ~/foo -ex "maint expand-symtabs" -ex "maint print symbols" -batch | grep 'typedef <system__object_reader__decoded_ada_name__TTdecodedSP1'
>      typedef <system__object_reader__decoded_ada_name__TTdecodedSP1: mod 0;
>
> The value 107820865988257 is the `baton` field of the property's union
> interpreted as an integer, so a bogus value.
>
> gdb/ChangeLog:
>
> PR ada/26318
> * ada-lang.c (ada_modulus): Return 0 if property is not of const
> kind.
>
> Change-Id: I3f6d343a9c3cd7cd62a4fc591943a43541223d50
> ---
>  gdb/ada-lang.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 2be3fe45b35d..29951528e5e5 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -11452,7 +11452,14 @@ ada_is_modular_type (struct type *type)
>  ULONGEST
>  ada_modulus (struct type *type)
>  {
> -  return (ULONGEST) type->bounds ()->high.const_val () + 1;
> +  const dynamic_prop &high = type->bounds ()->high;
> +
> +  if (high.kind () == PROP_CONST)
> +    return (ULONGEST) high.const_val () + 1;
> +
> +  /* If TYPE is unresolved, the high bound might be a location list.  Return
> +     0, for lack of a better value to return.  */
> +  return 0;
>  }
>  

Hi,

not an ada-maintainer, but LGTM.

Thanks,
- Tom

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gdb: handle non-const property types in ada_modulus (PR ada/26318)

Tom Tromey-2
In reply to this post by Sourceware - gdb-patches mailing list
>>>>> "Simon" == Simon Marchi via Gdb-patches <[hidden email]> writes:

Simon> gdb/ChangeLog:

Simon> PR ada/26318
Simon> * ada-lang.c (ada_modulus): Return 0 if property is not of const
Simon> kind.

Thanks for doing this.  I think this is ok.

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gdb: handle non-const property types in ada_modulus (PR ada/26318)

Sourceware - gdb-patches mailing list
On 2020-07-30 11:39 a.m., Tom Tromey wrote:

>>>>>> "Simon" == Simon Marchi via Gdb-patches <[hidden email]> writes:
>
> Simon> gdb/ChangeLog:
>
> Simon> PR ada/26318
> Simon> * ada-lang.c (ada_modulus): Return 0 if property is not of const
> Simon> kind.
>
> Thanks for doing this.  I think this is ok.
>
> Tom
>

Thanks Tom & Tom, pushed.

Simon