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

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

[vries at gcc dot gnu.org]

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

            Bug ID: 27787
           Summary: FAIL: gdb.python/flexible-array-member.exp: python
                    print(zs['items'].type.range())
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: testsuite
          Assignee: unassigned at sourceware dot org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

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

First in more detail:
...
(gdb) FAIL: gdb.python/flexible-array-member.exp: python
print(zs['items'].type.range())
python print(zso['items'].type.range())^M
(0, 0)^M
...

Related test-case portion:
...
# 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.                                                                 

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\\)"
...

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

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

[simark at simark dot ca]

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

Simon Marchi <simark at simark dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |simark at simark dot ca

--- Comment #1 from Simon Marchi <simark at simark dot ca> ---
Which compiler?  I presume this is very debug-info sensitive.

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

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

[vries at gcc dot gnu.org]

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

--- Comment #2 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Simon Marchi from comment #1)
> Which compiler?  I presume this is very debug-info sensitive.

Fails with gcc 7.5.0 and 8.4.0.

Passes with gcc 9.3.1, 10.3.0 and 11.0.1.

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

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

[simark at simark dot ca]

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

--- Comment #3 from Simon Marchi <simark at simark dot ca> ---
(In reply to Tom de Vries from comment #2)
> (In reply to Simon Marchi from comment #1)
> > Which compiler?  I presume this is very debug-info sensitive.
> 
> Fails with gcc 7.5.0 and 8.4.0.
> 
> Passes with gcc 9.3.1, 10.3.0 and 11.0.1.

Thanks, I'll try it soon.  I am guessing that we'll just get different values,
and that we'll end up changing the regexp to accept these values too.

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

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

[vries at gcc dot gnu.org]

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

--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Simon Marchi from comment #3)
> (In reply to Tom de Vries from comment #2)
> > (In reply to Simon Marchi from comment #1)
> > > Which compiler?  I presume this is very debug-info sensitive.
> > 
> > Fails with gcc 7.5.0 and 8.4.0.
> > 
> > Passes with gcc 9.3.1, 10.3.0 and 11.0.1.
> 
> Thanks, I'll try it soon.  I am guessing that we'll just get different
> values, and that we'll end up changing the regexp to accept these values too.

I compared 8.4.0 and 9.3.1, and found a difference in debug info.

In case of 8.4.0, we have variable zs:
...
 <1><1f0>: Abbrev Number: 9 (DW_TAG_variable)
    <1f1>   DW_AT_name        : zs
    ...
    <1f7>   DW_AT_type        : <0x205>
...
with type:
...
 <1><205>: Abbrev Number: 10 (DW_TAG_pointer_type)
    <206>   DW_AT_byte_size   : 8
    <207>   DW_AT_type        : <0x176>
...
pointing to struct type:
...
 <1><176>: Abbrev Number: 4 (DW_TAG_structure_type)
    <177>   DW_AT_name        : zero_size
...
with member:
...
 <2><18e>: Abbrev Number: 6 (DW_TAG_member)
    <18f>   DW_AT_name        : items
    <196>   DW_AT_type        : <0x19c>
...
with type:
...
 <1><19c>: Abbrev Number: 7 (DW_TAG_array_type)
    <19d>   DW_AT_type        : <0xfb>
    <1a1>   DW_AT_sibling     : <0x1ab>
 <2><1a5>: Abbrev Number: 8 (DW_TAG_subrange_type)
    <1a6>   DW_AT_type        : <0xf4>
 <2><1aa>: Abbrev Number: 0
...

The same for 9.3.1, but with DW_AT_count 0 in the subrange:
...
 <1><19c>: Abbrev Number: 7 (DW_TAG_array_type)
    <19d>   DW_AT_type        : <0xfb>
    <1a1>   DW_AT_sibling     : <0x1ac>
 <2><1a5>: Abbrev Number: 9 (DW_TAG_subrange_type)
    <1a6>   DW_AT_type        : <0xf4>
    <1aa>   DW_AT_count       : 0
...

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

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

[simark at simark dot ca]

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

--- Comment #5 from Simon Marchi <simark at simark dot ca> ---
Ok, so we could simply fix the test to accept both values (see patch below).

But do you think we should address the underlying issue and make GDB output
something consistent in that case, whatever the compiler and the method used to
specify the flexible array member?

- 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.


>From 67eeb4e645c7c6d02f642bb2864ad3286665b6af Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>
Date: Fri, 30 Apr 2021 09:12:35 -0400
Subject: [PATCH] fix

Change-Id: Ie4e06d99fe9315527f04577888f48284d649ca4c
---
 gdb/testsuite/gdb.python/flexible-array-member.exp | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.python/flexible-array-member.exp
b/gdb/testsuite/gdb.python/flexible-array-member.exp
index 349670cb7e7..38c8ee5f21f 100644
--- a/gdb/testsuite/gdb.python/flexible-array-member.exp
+++ b/gdb/testsuite/gdb.python/flexible-array-member.exp
@@ -76,9 +76,10 @@ 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.
+# is sometimes 0, sometimes -1.  It depends on the way the flexible array
+# member is specified and on the compiler version (the debug info is
+# different).  But that's what GDB produces today, so that's what we test.

 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(zs\['items'\].type.range())" "\\(0, (0|-1)\\)"
+gdb_test "python print(zso\['items'\].type.range())" "\\(0, (0|-1)\\)"
-- 
2.31.1

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

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

[vries at gcc dot gnu.org]

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

--- Comment #6 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Simon Marchi from comment #5)
> Ok, so we could simply fix the test to accept both values (see patch below).
> 

LGTM.  Please apply (given that our discussion may not end up somewhere
better).

> But do you think we should address the underlying issue and make GDB output
> something consistent in that case, whatever the compiler and the method used
> to specify the flexible array member?
> 

I think so, yes.

> - 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.

Anyway, I'm in favor of maximum clarity, so I'd say: "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?

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

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

[simark at simark dot ca]

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.