[Bug testsuite/27787] FAIL: gdb.python/flexible-array-member.exp: python print(zs['items'].type.range())

["simark at simark dot ca" <sourceware-bugzilla@sourceware.org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=27787

--- Comment #7 from Simon Marchi <simark at simark dot ca> ---
> LGTM.  Please apply (given that our discussion may not end up somewhere
> better).

Done.

> 
> > - Choose either 0 or -1 to indicate "there's no high bound" and output that
> > consistently.  It would be a user-visible behavior change for some cases but
> > not others.
> > - Emit "None" to indicate "there's no high bound".  This is probably a more
> > Pythonic way of doing things, but the downside is that it's a user-visible
> > change for all cases involved.
> > 
> 
> I think the current way of representing things makes no sense given that the
> same (0,0) is printed for wildly different things.
> 
> My guess would be that a reasonable api client checks type.Dynamic, and if
> true only check the lower bound, and not the upper, so I'm not sure how
> serious usage breakage would be.

I don't think this is a case of "type.dynamic == True".  Dynamic would be when
the range depends on a runtime value, and can be determined by GDB at runtime. 
Like VLAs.

Here, the type's range never changes.

> 
> Anyway, I'm in favor of maximum clarity, so I'd say: "None".

When getting a range for an array of size of (`foo[1]`), the range is also (0,
0).  That means the upper bound is inclusive.  For this reason, giving (0, 0)
when the upper bound is unknown is pretty bad, as there's no way to
differentiate foo[] or foo[0] from foo[1].

Because the current behavior doesn't really make sense, I think it's worth
breaking the behavior and change it to (0, None).

> 
> Having said that, I tried to implement it, like so:
> ...
> diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
> index 4f5f42529c2..0b9d972c42e 100644
> --- a/gdb/python/py-type.c
> +++ b/gdb/python/py-type.c
> @@ -577,6 +577,7 @@ typy_range (PyObject *self, PyObject *args)
>    struct type *type = ((type_object *) self)->type;
>    /* Initialize these to appease GCC warnings.  */
>    LONGEST low = 0, high = 0;
> +  bool has_low = false, has_high = false;
>  
>    if (type->code () != TYPE_CODE_ARRAY
>        && type->code () != TYPE_CODE_STRING
> @@ -593,32 +594,48 @@ typy_range (PyObject *self, PyObject *args)
>      case TYPE_CODE_STRING:
>      case TYPE_CODE_RANGE:
>        if (type->bounds ()->low.kind () == PROP_CONST)
> -       low = type->bounds ()->low.const_val ();
> -      else
> -       low = 0;
> +       {
> +         low = type->bounds ()->low.const_val ();
> +         has_low = true;
> +       }
>  
> -      if (type->bounds ()->high.kind () == PROP_CONST)
> -       high = type->bounds ()->high.const_val ();
> -      else
> -       high = 0;
> +      if (type->bounds ()->high.kind () == PROP_CONST
> +         && !is_dynamic_type (type))
> +       {
> +         high = type->bounds ()->high.const_val ();
> +         has_high = true;
> +       }
>        break;
>      }
>  
> -  gdbpy_ref<> low_bound = gdb_py_object_from_longest (low);
> -  if (low_bound == NULL)
> -    return NULL;
> +  gdbpy_ref<> low_bound;
> +  if (has_low)
> +    {--
> +      low_bound = gdb_py_object_from_longest (low);
> +      if (low_bound == NULL)
> +       return NULL;
> +    }
>  
> -  gdbpy_ref<> high_bound = gdb_py_object_from_longest (high);
> -  if (high_bound == NULL)
> -    return NULL;
> +  gdbpy_ref<> high_bound;
> +  if (has_high)
> +    {
> +      high_bound = gdb_py_object_from_longest (high);
> +      if (high_bound == NULL)
> +       return NULL;
> +    }
>  
>    gdbpy_ref<> result (PyTuple_New (2));
>    if (result == NULL)
>      return NULL;
>  
> -  if (PyTuple_SetItem (result.get (), 0, low_bound.release ()) != 0
> -      || PyTuple_SetItem (result.get (), 1, high_bound.release ()) != 0)
> +  PyObject *pyobject_low = has_low ? low_bound.release () : Py_None;
> +  if (PyTuple_SetItem (result.get (), 0, pyobject_low) != 0)
> +    return NULL;
> +
> +  PyObject *pyobject_high = has_high ? high_bound.release () : Py_None;
> +  if (PyTuple_SetItem (result.get (), 1, pyobject_high) != 0)
>      return NULL;
> +
>    return result.release ();
>  }
>  
> diff --git a/gdb/testsuite/gdb.python/flexible-array-member.exp
> b/gdb/testsuite/gdb.python/
> flexible-array-member.exp
> index 349670cb7e7..580626d64c7 100644
> --- a/gdb/testsuite/gdb.python/flexible-array-member.exp
> +++ b/gdb/testsuite/gdb.python/flexible-array-member.exp
> @@ -75,10 +75,8 @@ gdb_test "python print(zs\['items'\]\[0\].address + 1 ==
> zs\['items
> '\]\[1\].addr
>  gdb_test "python print(zso\['items'\] == zso\['items'\]\[0\].address)"
> "True"
>  gdb_test "python print(zso\['items'\]\[0\].address + 1 ==
> zso\['items'\]\[1\].address)" "True
> "
>  
> -# Verify the range attribute.  It looks a bit inconsistent that the high
> bound
> -# is sometimes 0, sometimes -1, but that's what GDB produces today, so
> that's
> -# what we test.
> +# Verify the range attribute.
>  
> -gdb_test "python print(ns\['items'\].type.range())" "\\(0, 0\\)"
> -gdb_test "python print(zs\['items'\].type.range())" "\\(0, -1\\)"
> -gdb_test "python print(zso\['items'\].type.range())" "\\(0, -1\\)"
> +gdb_test "python print(ns\['items'\].type.range())" "\\(0, None\\)"
> +gdb_test "python print(zs\['items'\].type.range())" "\\(0, None\\)"
> +gdb_test "python print(zso\['items'\].type.range())" "\\(0, \None\\)"
> e\\)"
> ...
> 
> And that works for gcc 7, but for say gcc 10, I get:
> ...
> (gdb) python print(ns['items'].type.range())^M
> (0, None)^M
> (gdb) PASS: gdb.python/flexible-array-member.exp: python
> print(ns['items'].type.range())
> python print(zs['items'].type.range())^M
> (0, -1)^M
> (gdb) FAIL: gdb.python/flexible-array-member.exp: python
> print(zs['items'].type.range())
> python print(zso['items'].type.range())^M
> (0, -1)^M
> (gdb) FAIL: gdb.python/flexible-array-member.exp: python
> print(zso['items'].type.range())
> ...
> 
> So, appearantly the underlying type for a zero-size array is represented in
> gdb using a highbound with PROP_CONST, and it's not a dynamic type.  The
> latter strikes me as wrong.  WDYT?

I don't really have time to dive into the implementation right now.  But I
think it's expected that the upper bound is not dynamic, it should probably be
left to PROP_UNDEFINED.  That would mean a change in the DWARF reader is needed
to produce the same range bounds regardless of the debug info.

-- 
You are receiving this mail because:
You are on the CC list for the bug.