public inbox for gdb-prs@sourceware.org
help / color / mirror / Atom feed
From: "vries at gcc dot gnu.org" <sourceware-bugzilla@sourceware.org>
To: gdb-prs@sourceware.org
Subject: [Bug testsuite/27787] FAIL: gdb.python/flexible-array-member.exp: python print(zs['items'].type.range())
Date: Tue, 04 May 2021 09:22:07 +0000	[thread overview]
Message-ID: <bug-27787-4717-WuNDgnAEXm@http.sourceware.org/bugzilla/> (raw)
In-Reply-To: <bug-27787-4717@http.sourceware.org/bugzilla/>

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.

  parent reply	other threads:[~2021-05-04  9:22 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-28 18:22 [Bug testsuite/27787] New: " vries at gcc dot gnu.org
2021-04-28 19:23 ` [Bug testsuite/27787] " simark at simark dot ca
2021-04-28 19:32 ` vries at gcc dot gnu.org
2021-04-28 20:45 ` simark at simark dot ca
2021-04-30  9:42 ` vries at gcc dot gnu.org
2021-04-30 13:15 ` simark at simark dot ca
2021-05-04  9:22 ` vries at gcc dot gnu.org [this message]
2021-05-04 15:28 ` simark at simark dot ca

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bug-27787-4717-WuNDgnAEXm@http.sourceware.org/bugzilla/ \
    --to=sourceware-bugzilla@sourceware.org \
    --cc=gdb-prs@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).