From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id BA97E3945C38; Tue, 4 May 2021 15:28:56 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BA97E3945C38 From: "simark at simark dot ca" 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 15:28:56 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gdb X-Bugzilla-Component: testsuite X-Bugzilla-Version: HEAD X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: simark at simark dot ca X-Bugzilla-Status: NEW X-Bugzilla-Resolution: X-Bugzilla-Priority: P2 X-Bugzilla-Assigned-To: unassigned at sourceware dot org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://sourceware.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-BeenThere: gdb-prs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-prs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 04 May 2021 15:28:56 -0000 https://sourceware.org/bugzilla/show_bug.cgi?id=3D27787 --- Comment #7 from Simon Marchi --- > LGTM. Please apply (given that our discussion may not end up somewhere > better). Done. >=20 > > - 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 case= s 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-visi= ble > > change for all cases involved. > >=20 >=20 > I think the current way of representing things makes no sense given that = the > same (0,0) is printed for wildly different things. >=20 > 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 =3D=3D True". Dynamic would = be when the range depends on a runtime value, and can be determined by GDB at runti= me.=20 Like VLAs. Here, the type's range never changes. >=20 > 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). >=20 > 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 =3D ((type_object *) self)->type; > /* Initialize these to appease GCC warnings. */ > LONGEST low =3D 0, high =3D 0; > + bool has_low =3D false, has_high =3D false; >=20=20 > if (type->code () !=3D TYPE_CODE_ARRAY > && type->code () !=3D 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 () =3D=3D PROP_CONST) > - low =3D type->bounds ()->low.const_val (); > - else > - low =3D 0; > + { > + low =3D type->bounds ()->low.const_val (); > + has_low =3D true; > + } >=20=20 > - if (type->bounds ()->high.kind () =3D=3D PROP_CONST) > - high =3D type->bounds ()->high.const_val (); > - else > - high =3D 0; > + if (type->bounds ()->high.kind () =3D=3D PROP_CONST > + && !is_dynamic_type (type)) > + { > + high =3D type->bounds ()->high.const_val (); > + has_high =3D true; > + } > break; > } >=20=20 > - gdbpy_ref<> low_bound =3D gdb_py_object_from_longest (low); > - if (low_bound =3D=3D NULL) > - return NULL; > + gdbpy_ref<> low_bound; > + if (has_low) > + {-- > + low_bound =3D gdb_py_object_from_longest (low); > + if (low_bound =3D=3D NULL) > + return NULL; > + } >=20=20 > - gdbpy_ref<> high_bound =3D gdb_py_object_from_longest (high); > - if (high_bound =3D=3D NULL) > - return NULL; > + gdbpy_ref<> high_bound; > + if (has_high) > + { > + high_bound =3D gdb_py_object_from_longest (high); > + if (high_bound =3D=3D NULL) > + return NULL; > + } >=20=20 > gdbpy_ref<> result (PyTuple_New (2)); > if (result =3D=3D NULL) > return NULL; >=20=20 > - if (PyTuple_SetItem (result.get (), 0, low_bound.release ()) !=3D 0 > - || PyTuple_SetItem (result.get (), 1, high_bound.release ()) !=3D = 0) > + PyObject *pyobject_low =3D has_low ? low_bound.release () : Py_None; > + if (PyTuple_SetItem (result.get (), 0, pyobject_low) !=3D 0) > + return NULL; > + > + PyObject *pyobject_high =3D has_high ? high_bound.release () : Py_None; > + if (PyTuple_SetItem (result.get (), 1, pyobject_high) !=3D 0) > return NULL; > + > return result.release (); > } >=20=20 > 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 = =3D=3D > zs\['items > '\]\[1\].addr > gdb_test "python print(zso\['items'\] =3D=3D zso\['items'\]\[0\].address= )" > "True" > gdb_test "python print(zso\['items'\]\[0\].address + 1 =3D=3D > zso\['items'\]\[1\].address)" "True > " >=20=20 > -# 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. >=20=20 > -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\\)" > ... >=20 > 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()) > ... >=20 > 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 ne= eded to produce the same range bounds regardless of the debug info. --=20 You are receiving this mail because: You are on the CC list for the bug.=