* [PATCH] Reject ambiguous C++ field accesses
@ 2020-08-27 18:02 Pedro Alves
2020-08-27 18:45 ` Luis Machado
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Pedro Alves @ 2020-08-27 18:02 UTC (permalink / raw)
To: gdb-patches
The gdb.cp/ambiguous.exp testcase had been disabled for many years,
but recently it was re-enabled. However, it is failing everywhere.
That is because it is testing an old feature that is gone from GDB.
The testcase is expecting to see an ambiguous field warning, like:
# X is derived from A1 and A2; both A1 and A2 have a member 'x'
send_gdb "print x.x\n"
gdb_expect {
-re "warning: x ambiguous; using X::A2::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
pass "print x.x"
}
-re "warning: x ambiguous; using X::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
pass "print x.x"
}
-re ".*$gdb_prompt $" { fail "print x.x" }
timeout { fail "(timeout) print x.x" }
}
However, GDB just accesses one of the candidates without warning or
error:
print x.x
$1 = 1431655296
(gdb) FAIL: gdb.cp/ambiguous.exp: print x.x
(The weird number is because the testcase does not initialize the
variables.)
The testcase come in originally with the big HP merge:
+Sun Jan 10 23:44:11 1999 David Taylor <taylor@texas.cygnus.com>
+
+
+ The following files are part of the HP merge; some had longer
+ names at HP, but have been renamed to be no more than 14
+ characters in length.
Looking at the tree back then, we find that warning:
/* Helper function used by value_struct_elt to recurse through baseclasses.
Look for a field NAME in ARG1. Adjust the address of ARG1 by OFFSET bytes,
and search in it assuming it has (class) type TYPE.
If found, return value, else return NULL.
If LOOKING_FOR_BASECLASS, then instead of looking for struct fields,
look for a baseclass named NAME. */
static value_ptr
search_struct_field (name, arg1, offset, type, looking_for_baseclass)
char *name;
register value_ptr arg1;
int offset;
register struct type *type;
int looking_for_baseclass;
{
int found = 0;
char found_class[1024];
value_ptr v;
struct type *vbase = NULL;
found_class[0] = '\000';
v = search_struct_field_aux (name, arg1, offset, type, looking_for_baseclass, &found, found_class, &vbase);
if (found > 1)
warning ("%s ambiguous; using %s::%s. Use a cast to disambiguate.",
name, found_class, name);
return v;
}
However, in current GDB, search_struct_field does not handle the
ambiguous field case, nor is that warning found anywhere. Somehow it
got lost over the years. That seems like a regression, because the
compiler (as per language rules) rejects the ambiguous accesses as
well. E.g.:
gdb.cp/ambiguous.cc:98:5: error: request for member 'x' is ambiguous
98 | x.x = 1;
| ^
gdb.cp/ambiguous.cc:10:7: note: candidates are: 'int A2::x'
10 | int x;
| ^
gdb.cp/ambiguous.cc:4:7: note: 'int A1::x'
4 | int x;
| ^
This patch restores the feature, though implemented differently and
with better user experience, IMHO. An ambiguous access is now an
error instead of a warning, and also GDB shows you the all candidates,
like:
(gdb) print x.x
Request for member 'x' is ambiguous in type 'X'. Candidates are:
'int A1::x' (X -> A1)
'int A2::x' (X -> A2)
(gdb) print j.x
Request for member 'x' is ambiguous in type 'J'. Candidates are:
'int A1::x' (J -> K -> A1)
'int A1::x' (J -> L -> A1)
Users can then fix their commands by casting or by specifying the
baseclass explicitly, like:
(gdb) p x.A1::x
$1 = 1
(gdb) p x.A2::x
$2 = 2
(gdb) p ((A1) x).x
$3 = 1
(gdb) p ((A2) x).x
$4 = 2
(gdb) p j.K::x
$12 = 1
(gdb) p j.L::x
$13 = 2
(gdb) p j.A1::x
base class 'A1' is ambiguous in type 'J'
The last error I've not touched; could be improved to also list the
baseclass candidates.
The showing the class "path" for each candidate was inspired by GCC's
output when you try an ambiguous cast:
gdb.cp/ambiguous.cc:161:8: error: ambiguous conversion from derived class 'const JVA1' to base class 'const A1':
class JVA1 -> class KV -> class A1
class JVA1 -> class A1
(A1) jva1;
^~~~
I did not include the "class" word as it seemed unnecessarily
repetitive, but I can include it if people prefer it:
(gdb) print j.x
Request for member 'x' is ambiguous in type 'J'. Candidates are:
'int A1::x' (class J -> class K -> class A1)
'int A1::x' (class J -> class L -> class A1)
The testcase is adjusted accordingly. I also took the chance to
modernize it at the same time.
Also, as mentioned above, the testcase doesn't currently initialize
the tested variables. This patch inializes them all, giving each
field a distinct value, so that we can be sure that GDB is accessing
the right fields / offsets. The testcase is extended accordingly.
Unfortunately, this exposes some bugs, not addressed in this patch.
The bug is around a class that inherits from A1 directly and also
inherits from with two other distinct base classes that inherit
virtually from A1 in turn:
print jva1.KV::x
$51 = 1431665544
(gdb) FAIL: gdb.cp/ambiguous.exp: all fields: print jva1.KV::x
print jva1.KV::y
$52 = 21845
(gdb) FAIL: gdb.cp/ambiguous.exp: all fields: print jva1.KV::y
This shows how GDB is clearly reading the confusing the offset of the
_vptr.KV with the A1 offset:
(gdb) print /x jva1
$1 = {<KV> = {<A1> = {x = 0x2, y = 0x16}, _vptr.KV = 0x555555557b88 <vtable for JVA1+24>, i = 0x457}, ......}
^^^^^^^^^^^^^^
(gdb) print /x jva1.KV::x
$2 = 0x55557b88
^^^^^^^^^^
(gdb) print /x jva1.KV::y
$3 = 0x5555
^^^^^^
(gdb) print /x (KV)jva1
$4 = {<A1> = <invalid address>, _vptr.KV = 0x555555557b88 <vtable for JVA1+24>, i = 0x457}
(gdb) print /x (A1)(KV)jva1
Cannot access memory at address 0x0
Since that's an orthogonal issue, I'm kfailing the tests that fail
because of it. (I'll file a bug soon, unless someone can fix it
quickly!)
gdb/ChangeLog:
* valops.c (struct struct_field_searcher): New.
(update_search_result): Rename to ...
(struct_field_searcher::update_result): ... this. Simplify
prototype. Record all found fields.
(do_search_struct_field): Rename to ...
(struct_field_searcher::search): ... this. Simplify prototype.
Maintain stack of visited baseclass path. Call update_result for
fields too. Keep searching fields in baseclasses instead of
stopping at the first found field.
(search_struct_field): Use struct_field_searcher. When looking
for fields, report ambiguous access attempts.
gdb/testsuite/ChangeLog:
* gdb.cp/ambiguous.cc (marker1): Delete.
(main): Initialize all the fields of the locals. Replace marker1
call with a "set breakpoint here" marker.
* gdb.cp/ambiguous.exp: Modernize. Use gdb_continue_to_breakpoint
instead of running to marker1. Add tests printing all the
variables and all the fields of the variables.
(test_ambiguous): New proc, expecting the new GDB output when a
field access is ambiguous. Change all "warning: X ambiguous"
tests to use it.
---
gdb/testsuite/gdb.cp/ambiguous.cc | 87 ++++++++--
gdb/testsuite/gdb.cp/ambiguous.exp | 321 +++++++++++++++++++++----------------
gdb/valops.c | 219 ++++++++++++++++++-------
3 files changed, 418 insertions(+), 209 deletions(-)
diff --git a/gdb/testsuite/gdb.cp/ambiguous.cc b/gdb/testsuite/gdb.cp/ambiguous.cc
index 6ee7bc18ea9..a55686547f2 100644
--- a/gdb/testsuite/gdb.cp/ambiguous.cc
+++ b/gdb/testsuite/gdb.cp/ambiguous.cc
@@ -1,9 +1,4 @@
-void marker1()
-{
- return;
-}
-
class A1 {
public:
int x;
@@ -102,9 +97,81 @@ int main()
i += k.i + m.w + a1.x + a2.x + a3.x + x.z + l.z + n.r + j.j;
- marker1();
-
+ /* Initialize all the fields. Keep the order the same as in the
+ .exp file. */
+
+ a1.x = 1;
+ a1.y = 2;
+
+ a2.x = 1;
+ a2.y = 2;
+
+ a3.x = 1;
+ a3.y = 2;
+
+ x.A1::x = 1;
+ x.A1::y = 2;
+ x.A2::x = 3;
+ x.A2::y = 4;
+ x.z = 5;
+
+ l.x = 1;
+ l.y = 2;
+ l.z = 3;
+
+ m.x = 1;
+ m.y = 2;
+ m.w = 3;
+
+ n.A1::x = 1;
+ n.A1::y = 2;
+ n.A2::x = 3;
+ n.A2::y = 4;
+ n.w = 5;
+ n.r = 6;
+ n.z = 7;
+
+ k.x = 1;
+ k.y = 2;
+ k.i = 3;
+
+ j.K::x = 1;
+ j.K::y = 2;
+ j.L::x = 3;
+ j.L::y = 4;
+ j.i = 5;
+ j.z = 6;
+ j.j = 7;
+
+ jv.x = 1;
+ jv.y = 2;
+ jv.i = 3;
+ jv.z = 4;
+ jv.jv = 5;
+
+ jva1.KV::x = 1;
+ jva1.KV::y = 2;
+ jva1.LV::x = 3;
+ jva1.LV::y = 4;
+ jva1.z = 5;
+ jva1.i = 6;
+ jva1.jva1 = 7;
+
+ jva2.KV::x = 1;
+ jva2.KV::y = 2;
+ jva2.LV::x = 3;
+ jva2.LV::y = 4;
+ jva2.A2::x = 5;
+ jva2.A2::y = 6;
+ jva2.z = 7;
+ jva2.i = 8;
+ jva2.jva2 = 9;
+
+ jva1v.x = 1;
+ jva1v.y = 2;
+ jva1v.z = 3;
+ jva1v.i = 4;
+ jva1v.jva1v = 5;
+
+ return 0; /* set breakpoint here */
}
-
-
-
diff --git a/gdb/testsuite/gdb.cp/ambiguous.exp b/gdb/testsuite/gdb.cp/ambiguous.exp
index 17fb29f5350..74783cd0725 100644
--- a/gdb/testsuite/gdb.cp/ambiguous.exp
+++ b/gdb/testsuite/gdb.cp/ambiguous.exp
@@ -15,15 +15,9 @@
# This file is part of the gdb testsuite
-# tests relating to ambiguous class members
-# Written by Satish Pai <pai@apollo.hp.com> 1997-07-28
-
-# This file is part of the gdb testsuite
-
-#
-# test running programs
-#
-
+# Print out various class objects' members and check that the error
+# about the field or baseclass being ambiguious is emitted at the
+# right times.
if { [skip_cplus_tests] } { continue }
@@ -47,180 +41,225 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
return -1
}
-#
-# set it up at a breakpoint so we can play with the variable values
-#
+# Set it up at a breakpoint so we can play with the variable values.
+
if ![runto_main] then {
perror "couldn't run to breakpoint"
continue
}
-send_gdb "break marker1\n" ; gdb_expect -re ".*$gdb_prompt $"
- send_gdb "cont\n"
- gdb_expect {
- -re "Break.* marker1 \\(\\) at .*:$decimal.*$gdb_prompt $" {
- send_gdb "up\n"
- gdb_expect {
- -re ".*$gdb_prompt $" { pass "up from marker1" }
- timeout { fail "up from marker1" }
- }
- }
- -re "$gdb_prompt $" { fail "continue to marker1" }
- timeout { fail "(timeout) continue to marker1" }
+set lineno [gdb_get_line_number "set breakpoint here"]
+
+gdb_breakpoint $lineno
+gdb_continue_to_breakpoint "breakpoint here"
+
+set number -?$decimal
+
+with_test_prefix "all vars" {
+ gdb_test "print a1" \
+ " = \{x = 1, y = 2\}"
+
+ gdb_test "print a2" \
+ " = \{x = 1, y = 2\}"
+
+ gdb_test "print a3" \
+ " = \{x = 1, y = 2\}"
+
+ gdb_test "print x" \
+ " = \{<A1> = \{x = 1, y = 2\}, <A2> = \{x = 3, y = 4\}, z = 5\}"
+
+ gdb_test "print l" \
+ " = \{<A1> = \{x = 1, y = 2\}, z = 3\}"
+
+ gdb_test "print m" \
+ " = \{<A2> = \{x = 1, y = 2\}, w = 3\}"
+
+ gdb_test "print n" \
+ " = \{<L> = \{<A1> = \{x = 1, y = 2\}, z = 7\}, <M> = \{<A2> = \{x = 3, y = 4\}, w = 5\}, r = 6\}"
+
+ gdb_test "print k" \
+ " = \{<A1> = \{x = 1, y = 2\}, i = 3\}"
+
+ gdb_test "print j" \
+ " = {<K> = {<A1> = {x = 1, y = 2}, i = 5}, <L> = {<A1> = {x = 3, y = 4}, z = 6}, j = 7}"
+
+ gdb_test "print jv" \
+ " = \{<KV> = \{<A1> = \{x = 1, y = 2\}, _vptr.KV = $hex <vtable for JV.*>, i = 3\}, <LV> = \{_vptr.LV = $hex <VTT for JV>, z = 4\}, jv = 5\}"
+
+ # No way to initialize one of the A1's, so just take any number there.
+ gdb_test "print jva1" \
+ " = \{<KV> = \{<A1> = \{x = 3, y = 4\}, _vptr.KV = $hex <vtable for JVA1.*>, i = 6\}, <LV> = \{_vptr.LV = $hex <VTT for JVA1>, z = 5\}, <A1> = \{x = $number, y = $number\}, jva1 = 7\}"
+
+ gdb_test "print jva2" \
+ " = \{<KV> = \{<A1> = \{x = 3, y = 4\}, _vptr.KV = $hex <vtable for JVA2.*>, i = 8\}, <LV> = \{_vptr.LV = $hex <VTT for JVA2>, z = 7\}, <A2> = \{x = 5, y = 6\}, jva2 = 9\}"
+
+ gdb_test "print jva1v" \
+ " = \{<KV> = \{<A1> = \{x = 1, y = 2\}, _vptr.KV = $hex <vtable for JVA1V+.*>, i = 4\}, <LV> = \{_vptr.LV = $hex <VTT for JVA1V>, z = 3\}, jva1v = 5\}"
+}
+
+# Check that we can access all the fields correctly, using the same
+# syntax as used in the .cc file. Keep the order here in sync with
+# the .cc file.
+with_test_prefix "all fields" {
+ gdb_test "print a1.x" " = 1"
+ gdb_test "print a1.y" " = 2"
+
+ gdb_test "print a2.x" " = 1"
+ gdb_test "print a2.y" " = 2"
+
+ gdb_test "print a3.x" " = 1"
+ gdb_test "print a3.y" " = 2"
+
+ gdb_test "print x.A1::x" " = 1"
+ gdb_test "print x.A1::y" " = 2"
+ gdb_test "print x.A2::x" " = 3"
+ gdb_test "print x.A2::y" " = 4"
+ gdb_test "print x.z" " = 5"
+
+ gdb_test "print l.x" " = 1"
+ gdb_test "print l.y" " = 2"
+ gdb_test "print l.z" " = 3"
+
+ gdb_test "print m.x" " = 1"
+ gdb_test "print m.y" " = 2"
+ gdb_test "print m.w" " = 3"
+
+ gdb_test "print n.A1::x" " = 1"
+ gdb_test "print n.A1::y" " = 2"
+ gdb_test "print n.A2::x" " = 3"
+ gdb_test "print n.A2::y" " = 4"
+ gdb_test "print n.w" " = 5"
+ gdb_test "print n.r" " = 6"
+ gdb_test "print n.z" " = 7"
+
+ gdb_test "print k.x" " = 1"
+ gdb_test "print k.y" " = 2"
+ gdb_test "print k.i" " = 3"
+
+ gdb_test "print j.K::x" " = 1"
+ gdb_test "print j.K::y" " = 2"
+ gdb_test "print j.L::x" " = 3"
+ gdb_test "print j.L::y" " = 4"
+ gdb_test "print j.i" " = 5"
+ gdb_test "print j.z" " = 6"
+ gdb_test "print j.j" " = 7"
+
+ gdb_test "print jv.x" " = 1"
+ gdb_test "print jv.y" " = 2"
+ gdb_test "print jv.i" " = 3"
+ gdb_test "print jv.z" " = 4"
+ gdb_test "print jv.jv" " = 5"
+
+ setup_kfail "gdb/nnnnn" *-*-*
+ gdb_test "print jva1.KV::x" " = 1"
+ setup_kfail "gdb/nnnnn" *-*-*
+ gdb_test "print jva1.KV::y" " = 2"
+ setup_kfail "gdb/nnnnn" *-*-*
+ gdb_test "print jva1.LV::x" " = 3"
+ setup_kfail "gdb/nnnnn" *-*-*
+ gdb_test "print jva1.LV::y" " = 4"
+ gdb_test "print jva1.z" " = 5"
+ gdb_test "print jva1.i" " = 6"
+ gdb_test "print jva1.jva1" "= 7"
+
+ setup_kfail "gdb/nnnnn" *-*-*
+ gdb_test "print jva2.KV::x" " = 1"
+ setup_kfail "gdb/nnnnn" *-*-*
+ gdb_test "print jva2.KV::y" " = 2"
+ setup_kfail "gdb/nnnnn" *-*-*
+ gdb_test "print jva2.LV::x" " = 3"
+ setup_kfail "gdb/nnnnn" *-*-*
+ gdb_test "print jva2.LV::y" " = 4"
+ gdb_test "print jva2.A2::x" " = 5"
+ gdb_test "print jva2.A2::y" " = 6"
+ gdb_test "print jva2.z" " = 7"
+ gdb_test "print jva2.i" " = 8"
+ gdb_test "print jva2.jva2" "= 9"
+
+ gdb_test "print jva1v.x" " = 1"
+ gdb_test "print jva1v.y" " = 2"
+ gdb_test "print jva1v.z" " = 3"
+ gdb_test "print jva1v.i" " = 4"
+ gdb_test "print jva1v.jva1v" " = 5"
+}
+
+# Test that printing WHAT reports an error about FIELD being ambiguous
+# in TYPE, and that the candidates are CANDIDATES.
+proc test_ambiguous {what field type candidates} {
+ set msg "Request for member '$field' is ambiguous in type '$type'. Candidates are:"
+
+ foreach c $candidates {
+ set c_re [string_to_regexp $c]
+ append msg "\r\n $c_re"
}
-# print out various class objects' members. The values aren't
-# important, just check that the warning is emitted at the
-# right times.
+ gdb_test "print $what" $msg
+}
# X is derived from A1 and A2; both A1 and A2 have a member 'x'
-send_gdb "print x.x\n"
-gdb_expect {
- -re "warning: x ambiguous; using X::A2::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print x.x"
- }
- -re "warning: x ambiguous; using X::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print x.x"
- }
- -re ".*$gdb_prompt $" { fail "print x.x" }
- timeout { fail "(timeout) print x.x" }
+test_ambiguous "x.x" "x" "X" {
+ "'int A1::x' (X -> A1)"
+ "'int A2::x' (X -> A2)"
}
-
# N is derived from A1 and A2, but not immediately -- two steps
# up in the hierarchy. Both A1 and A2 have a member 'x'.
-send_gdb "print n.x\n"
-gdb_expect {
- -re "warning: x ambiguous; using N::M::A2::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print n.x"
- }
- -re "warning: x ambiguous; using N::L::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print n.x"
- }
- -re ".*$gdb_prompt $" { fail "print n.x" }
- timeout { fail "(timeout) print n.x" }
+test_ambiguous "n.x" "x" "N" {
+ "'int A1::x' (N -> L -> A1)"
+ "'int A2::x' (N -> M -> A2)"
}
-# J is derived from A1 twice. A1 has a member x.
-send_gdb "print j.x\n"
-gdb_expect {
- -re "warning: x ambiguous; using J::L::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print j.x"
- }
- -re "warning: x ambiguous; using J::K::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print j.x"
- }
- -re ".*$gdb_prompt $" { fail "print j.x" }
- timeout { fail "(timeout) print j.x" }
+# J is derived from A1 twice. A1 has a member x.
+test_ambiguous "j.x" "x" "J" {
+ "'int A1::x' (J -> K -> A1)"
+ "'int A1::x' (J -> L -> A1)"
}
# JV is derived from A1 but A1 is a virtual base. Should not
-# report an ambiguity in this case.
-send_gdb "print jv.x\n"
-gdb_expect {
- -re "warning: x ambiguous.*Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- fail "print jv.x (ambiguity reported)"
- }
- -re "\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" { pass "print jv.x" }
- -re ".*$gdb_prompt $" { fail "print jv.x (??)" }
- timeout { fail "(timeout) print jv.x" }
-}
+# report an ambiguity in this case.
+gdb_test "print jv.x" " = 1"
# JVA1 is derived from A1; A1 occurs as a virtual base in two
# ancestors, and as a non-virtual immediate base. Ambiguity must
-# be reported.
-send_gdb "print jva1.x\n"
-gdb_expect {
- -re "warning: x ambiguous; using JVA1::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print jva1.x"
- }
- -re "warning: x ambiguous; using JVA1::KV::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print jva1.x"
- }
- -re ".*$gdb_prompt $" { fail "print jva1.x" }
- timeout { fail "(timeout) print jva1.x" }
+# be reported.
+test_ambiguous "jva1.x" "x" "JVA1" {
+ "'int A1::x' (JVA1 -> KV -> A1)"
+ "'int A1::x' (JVA1 -> A1)"
}
# JVA2 is derived from A1 & A2; A1 occurs as a virtual base in two
# ancestors, and A2 is a non-virtual immediate base. Ambiguity must
# be reported as A1 and A2 both have a member 'x'.
-send_gdb "print jva2.x\n"
-gdb_expect {
- -re "warning: x ambiguous; using JVA2::A2::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print jva2.x"
- }
- -re "warning: x ambiguous; using JVA2::KV::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print jva2.x"
- }
- -re ".*$gdb_prompt $" { fail "print jva2.x" }
- timeout { fail "(timeout) print jva2.x" }
+test_ambiguous "jva2.x" "x" "JVA2" {
+ "'int A1::x' (JVA2 -> KV -> A1)"
+ "'int A2::x' (JVA2 -> A2)"
}
# JVA1V is derived from A1; A1 occurs as a virtual base in two
# ancestors, and also as a virtual immediate base. Ambiguity must
# not be reported.
-send_gdb "print jva1v.x\n"
-gdb_expect {
- -re "warning: x ambiguous.*Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- fail "print jva1v.x (ambiguity reported)"
- }
- -re "\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" { pass "print jva1v.x" }
- -re ".*$gdb_prompt $" { fail "print jva1v.x (??)" }
- timeout { fail "(timeout) print jva1v.x" }
-}
+gdb_test "print jva1v.x" " = 1"
# Now check for ambiguous bases.
# J is derived from A1 twice; report ambiguity if a J is
# cast to an A1.
-send_gdb "print (A1)j\n"
-gdb_expect {
- -re "warning: A1 ambiguous; using J::L::A1. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \{x = \[-\]*\[0-9\]*, y = \[-\]*\[0-9\]*\}\r\n$gdb_prompt $" {
- pass "print (A1)j"
- }
- -re "warning: A1 ambiguous; using J::K::A1. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \{x = \[-\]*\[0-9\]*, y = \[-\]*\[0-9\]*\}\r\n$gdb_prompt $" {
- pass "print (A1)j"
- }
- -re ".*$gdb_prompt $" { fail "print (A1)j" }
- timeout { fail "(timeout) print (A1)j" }
-}
+gdb_test "print (A1)j" "base class 'A1' is ambiguous in type 'J'"
# JV is derived from A1 twice, but A1 is a virtual base; should
# not report ambiguity when a JV is cast to an A1.
-send_gdb "print (A1)jv\n"
-gdb_expect {
- -re "warning: A1 ambiguous.*Use a cast to disambiguate.\r\n\\$\[0-9\]* = \{x = \[-\]*\[0-9\]*, y = \[-\]*\[0-9\]*\}\r\n$gdb_prompt $" {
- fail "print (A1)jv (ambiguity reported)"
- }
- -re "\\$\[0-9\]* = \{x = \[-\]*\[0-9\]*, y = \[-\]*\[0-9\]*\}\r\n$gdb_prompt $" { pass "print (A1)jv" }
- -re ".*$gdb_prompt $" { fail "print (A1)jv (??)" }
- timeout { fail "(timeout) print (A1)jv" }
-}
+gdb_test "print (A1)jv" " = {x = 1, y = 2}"
# JVA1 is derived from A1; A1 is a virtual base and also a
# non-virtual base. Must report ambiguity if a JVA1 is cast to an A1.
-send_gdb "print (A1)jva1\n"
-gdb_expect {
- -re "warning: A1 ambiguous; using JVA1::A1. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \{x = \[-\]*\[0-9\]*, y = \[-\]*\[0-9\]*\}\r\n$gdb_prompt $" {
- pass "print (A1)jva1"
- }
- -re "warning: A1 ambiguous; using JVA1::KV::A1. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \{x = \[-\]*\[0-9\]*, y = \[-\]*\[0-9\]*\}\r\n$gdb_prompt $" {
- pass "print (A1)jva1"
- }
- -re ".*$gdb_prompt $" { fail "print (A1)jva1" }
- timeout { fail "(timeout) print (A1)jva1" }
-}
+gdb_test "print (A1)jva1" "base class 'A1' is ambiguous in type 'JVA1'"
+
+# Add an intermediate cast to KV, and it should work.
+setup_kfail "gdb/nnnnn" *-*-*
+gdb_test "print (KV)jva1" " = \{<A1> = \{x = 3, y = 4\}, _vptr.KV = $hex <VTT for KV>, i = 6\}"
+setup_kfail "gdb/nnnnn" *-*-*
+gdb_test "print (A1)(KV)jva1" " = \{x = 3, y = 4\}"
# JVA1V is derived from A1; A1 is a virtual base indirectly
# and also directly; must not report ambiguity when a JVA1V is cast to an A1.
-send_gdb "print (A1)jva1v\n"
-gdb_expect {
- -re "warning: A1 ambiguous.*Use a cast to disambiguate.\r\n\\$\[0-9\]* = \{x = \[-\]*\[0-9\]*, y = \[-\]*\[0-9\]*\}\r\n$gdb_prompt $" {
- fail "print (A1)jva1v (ambiguity reported)"
- }
- -re "\\$\[0-9\]* = \{x = \[-\]*\[0-9\]*, y = \[-\]*\[0-9\]*\}\r\n$gdb_prompt $" { pass "print (A1)jva1v"
- }
- -re ".*$gdb_prompt $" { fail "print (A1)jva1v (??)" }
- timeout { fail "(timeout) print (A1)jva1v" }
-}
-
+gdb_test "print (A1)jva1v" " = {x = 1, y = 2}"
diff --git a/gdb/valops.c b/gdb/valops.c
index 0eb2b096211..0e030a03ecd 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -1766,25 +1766,105 @@ typecmp (int staticp, int varargs, int nargs,
return i + 1;
}
-/* Helper class for do_search_struct_field that updates *RESULT_PTR
- and *LAST_BOFFSET, and possibly throws an exception if the field
- search has yielded ambiguous results. */
+/* Helper class for search_struct_field that keeps track of found
+ results. See search_struct_field for description of
+ LOOKING_FOR_BASECLASS. If LOOKING_FOR_BASECLASS is true, possibly
+ throws an exception if the base class search has yielded ambiguous
+ results. If LOOKING_FOR_BASECLASS is false, found fields are
+ accumulated and the caller (search_struct_field takes) care of
+ throwing an error if the field search yielded ambiguous results.
+ The latter is done that way so that the error message can include a
+ list of all the found candidates. */
+
+struct struct_field_searcher
+{
+ /* A found field. */
+ struct found_field
+ {
+ /* Patch to the structure where the field was found. */
+ std::vector<struct type *> path;
+
+ /* The field found. */
+ struct value *field_value;
+ };
+
+ struct_field_searcher (const char *name,
+ struct type *outermost_type,
+ bool looking_for_baseclass)
+ : m_name (name),
+ m_looking_for_baseclass (looking_for_baseclass),
+ m_outermost_type (outermost_type)
+ {
+ }
-static void
-update_search_result (struct value **result_ptr, struct value *v,
- LONGEST *last_boffset, LONGEST boffset,
- const char *name, struct type *type)
+ void search (struct value *arg1, LONGEST offset,
+ struct type *type);
+
+ const std::vector<found_field> &fields ()
+ {
+ return m_fields;
+ }
+
+ struct value *baseclass ()
+ {
+ return m_baseclass;
+ }
+
+private:
+ /* Update results to include V, a found field/baseclass. */
+ void update_result (struct value *v, LONGEST boffset);
+
+ /* The name of the field/baseclass we're searching for. */
+ const char *m_name;
+
+ /* Whether we're looking for a baseclass, or a field. */
+ const bool m_looking_for_baseclass;
+
+ /* The offset of base class containing the field/baseclass we last
+ recorded. */
+ LONGEST m_last_boffset = 0;
+
+ /* If looking for a baseclass, then the result is stored here. */
+ struct value *m_baseclass = nullptr;
+
+ /* When looking for fields, the found candidates are stored
+ here. */
+ std::vector<found_field> m_fields;
+
+ /* The type of the initial type passed to search_struct_field; this
+ is used for error reporting when the lookup is ambiguous. */
+ struct type *m_outermost_type;
+
+ /* The full path to the struct being inspected. E.g. for field 'x'
+ defined in class B inherited by class A, we have A and B pushed
+ on the path. */
+ std::vector <struct type *> m_struct_path;
+};
+
+void
+struct_field_searcher::update_result (struct value *v, LONGEST boffset)
{
if (v != NULL)
{
- if (*result_ptr != NULL
- /* The result is not ambiguous if all the classes that are
- found occupy the same space. */
- && *last_boffset != boffset)
- error (_("base class '%s' is ambiguous in type '%s'"),
- name, TYPE_SAFE_NAME (type));
- *result_ptr = v;
- *last_boffset = boffset;
+ if (m_looking_for_baseclass)
+ {
+ if (m_baseclass != nullptr
+ /* The result is not ambiguous if all the classes that are
+ found occupy the same space. */
+ && m_last_boffset != boffset)
+ error (_("base class '%s' is ambiguous in type '%s'"),
+ m_name, TYPE_SAFE_NAME (m_outermost_type));
+
+ m_baseclass = v;
+ m_last_boffset = boffset;
+ }
+ else
+ {
+ /* The field is not ambiguous if it occupies the same
+ space. */
+ if (m_fields.empty () || m_last_boffset != boffset)
+ m_fields.push_back ({m_struct_path, v});
+ }
}
}
@@ -1795,25 +1875,25 @@ update_search_result (struct value **result_ptr, struct value *v,
search_struct_field; this is used for error reporting when the
lookup is ambiguous. */
-static void
-do_search_struct_field (const char *name, struct value *arg1, LONGEST offset,
- struct type *type, int looking_for_baseclass,
- struct value **result_ptr,
- LONGEST *last_boffset,
- struct type *outermost_type)
+void
+struct_field_searcher::search (struct value *arg1, LONGEST offset,
+ struct type *type)
{
int i;
int nbases;
+ m_struct_path.push_back (type);
+ SCOPE_EXIT { m_struct_path.pop_back (); };
+
type = check_typedef (type);
nbases = TYPE_N_BASECLASSES (type);
- if (!looking_for_baseclass)
+ if (!m_looking_for_baseclass)
for (i = type->num_fields () - 1; i >= nbases; i--)
{
const char *t_field_name = TYPE_FIELD_NAME (type, i);
- if (t_field_name && (strcmp_iw (t_field_name, name) == 0))
+ if (t_field_name && (strcmp_iw (t_field_name, m_name) == 0))
{
struct value *v;
@@ -1821,7 +1901,8 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset,
v = value_static_field (type, i);
else
v = value_primitive_field (arg1, offset, i, type);
- *result_ptr = v;
+
+ update_result (v, offset);
return;
}
@@ -1845,7 +1926,6 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset,
represented as a struct, with a member for each
<variant field>. */
- struct value *v = NULL;
LONGEST new_offset = offset;
/* This is pretty gross. In G++, the offset in an
@@ -1859,16 +1939,7 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset,
&& TYPE_FIELD_BITPOS (field_type, 0) == 0))
new_offset += TYPE_FIELD_BITPOS (type, i) / 8;
- do_search_struct_field (name, arg1, new_offset,
- field_type,
- looking_for_baseclass, &v,
- last_boffset,
- outermost_type);
- if (v)
- {
- *result_ptr = v;
- return;
- }
+ search (arg1, new_offset, field_type);
}
}
}
@@ -1880,10 +1951,10 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset,
/* If we are looking for baseclasses, this is what we get when
we hit them. But it could happen that the base part's member
name is not yet filled in. */
- int found_baseclass = (looking_for_baseclass
+ int found_baseclass = (m_looking_for_baseclass
&& TYPE_BASECLASS_NAME (type, i) != NULL
- && (strcmp_iw (name,
- TYPE_BASECLASS_NAME (type,
+ && (strcmp_iw (m_name,
+ TYPE_BASECLASS_NAME (type,
i)) == 0));
LONGEST boffset = value_embedded_offset (arg1) + offset;
@@ -1924,28 +1995,17 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset,
if (found_baseclass)
v = v2;
else
- {
- do_search_struct_field (name, v2, 0,
- TYPE_BASECLASS (type, i),
- looking_for_baseclass,
- result_ptr, last_boffset,
- outermost_type);
- }
+ search (v2, 0, TYPE_BASECLASS (type, i));
}
else if (found_baseclass)
v = value_primitive_field (arg1, offset, i, type);
else
{
- do_search_struct_field (name, arg1,
- offset + TYPE_BASECLASS_BITPOS (type,
- i) / 8,
- basetype, looking_for_baseclass,
- result_ptr, last_boffset,
- outermost_type);
+ search (arg1, offset + TYPE_BASECLASS_BITPOS (type, i) / 8,
+ basetype);
}
- update_search_result (result_ptr, v, last_boffset,
- boffset, name, outermost_type);
+ update_result (v, boffset);
}
}
@@ -1960,12 +2020,55 @@ static struct value *
search_struct_field (const char *name, struct value *arg1,
struct type *type, int looking_for_baseclass)
{
- struct value *result = NULL;
- LONGEST boffset = 0;
+ struct_field_searcher searcher (name, type, looking_for_baseclass);
- do_search_struct_field (name, arg1, 0, type, looking_for_baseclass,
- &result, &boffset, type);
- return result;
+ searcher.search (arg1, 0, type);
+
+ if (!looking_for_baseclass)
+ {
+ const auto &fields = searcher.fields ();
+
+ if (fields.empty ())
+ return nullptr;
+ else if (fields.size () > 1)
+ {
+ std::string candidates;
+
+ for (auto &&candidate : fields)
+ {
+ gdb_assert (!candidate.path.empty ());
+
+ struct type *field_type = value_type (candidate.field_value);
+ struct type *struct_type = candidate.path.back ();
+
+ std::string path;
+ bool first = true;
+ for (struct type *t : candidate.path)
+ {
+ if (first)
+ first = false;
+ else
+ path += " -> ";
+ path += t->name ();
+ }
+
+ candidates += string_printf ("\n '%s %s::%s' (%s)",
+ TYPE_SAFE_NAME (field_type),
+ TYPE_SAFE_NAME (struct_type),
+ name,
+ path.c_str ());
+ }
+
+ error (_("Request for member '%s' is ambiguous in type '%s'."
+ " Candidates are:%s"),
+ name, TYPE_SAFE_NAME (type),
+ candidates.c_str ());
+ }
+ else
+ return fields[0].field_value;
+ }
+ else
+ return searcher.baseclass ();
}
/* Helper function used by value_struct_elt to recurse through
base-commit: 8c51f2f291a5459e1eabd000b2c52e5de52b4c56
--
2.14.5
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Reject ambiguous C++ field accesses
2020-08-27 18:02 [PATCH] Reject ambiguous C++ field accesses Pedro Alves
@ 2020-08-27 18:45 ` Luis Machado
2020-08-27 19:12 ` Pedro Alves
2020-08-27 19:58 ` Luis Machado
2020-08-28 14:38 ` Gary Benson
2 siblings, 1 reply; 10+ messages in thread
From: Luis Machado @ 2020-08-27 18:45 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 8/27/20 3:02 PM, Pedro Alves wrote:
> The gdb.cp/ambiguous.exp testcase had been disabled for many years,
> but recently it was re-enabled. However, it is failing everywhere.
> That is because it is testing an old feature that is gone from GDB.
>
> The testcase is expecting to see an ambiguous field warning, like:
>
> # X is derived from A1 and A2; both A1 and A2 have a member 'x'
> send_gdb "print x.x\n"
> gdb_expect {
> -re "warning: x ambiguous; using X::A2::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
> pass "print x.x"
> }
> -re "warning: x ambiguous; using X::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
> pass "print x.x"
> }
> -re ".*$gdb_prompt $" { fail "print x.x" }
> timeout { fail "(timeout) print x.x" }
> }
>
> However, GDB just accesses one of the candidates without warning or
> error:
>
> print x.x
> $1 = 1431655296
> (gdb) FAIL: gdb.cp/ambiguous.exp: print x.x
>
> (The weird number is because the testcase does not initialize the
> variables.)
>
> The testcase come in originally with the big HP merge:
>
> +Sun Jan 10 23:44:11 1999 David Taylor <taylor@texas.cygnus.com>
> +
> +
> + The following files are part of the HP merge; some had longer
> + names at HP, but have been renamed to be no more than 14
> + characters in length.
>
> Looking at the tree back then, we find that warning:
>
> /* Helper function used by value_struct_elt to recurse through baseclasses.
> Look for a field NAME in ARG1. Adjust the address of ARG1 by OFFSET bytes,
> and search in it assuming it has (class) type TYPE.
> If found, return value, else return NULL.
>
> If LOOKING_FOR_BASECLASS, then instead of looking for struct fields,
> look for a baseclass named NAME. */
>
> static value_ptr
> search_struct_field (name, arg1, offset, type, looking_for_baseclass)
> char *name;
> register value_ptr arg1;
> int offset;
> register struct type *type;
> int looking_for_baseclass;
> {
> int found = 0;
> char found_class[1024];
> value_ptr v;
> struct type *vbase = NULL;
>
> found_class[0] = '\000';
>
> v = search_struct_field_aux (name, arg1, offset, type, looking_for_baseclass, &found, found_class, &vbase);
> if (found > 1)
> warning ("%s ambiguous; using %s::%s. Use a cast to disambiguate.",
> name, found_class, name);
>
> return v;
> }
>
>
> However, in current GDB, search_struct_field does not handle the
> ambiguous field case, nor is that warning found anywhere. Somehow it
> got lost over the years. That seems like a regression, because the
> compiler (as per language rules) rejects the ambiguous accesses as
> well. E.g.:
>
> gdb.cp/ambiguous.cc:98:5: error: request for member 'x' is ambiguous
> 98 | x.x = 1;
> | ^
> gdb.cp/ambiguous.cc:10:7: note: candidates are: 'int A2::x'
> 10 | int x;
> | ^
> gdb.cp/ambiguous.cc:4:7: note: 'int A1::x'
> 4 | int x;
> | ^
>
>
> This patch restores the feature, though implemented differently and
> with better user experience, IMHO. An ambiguous access is now an
> error instead of a warning, and also GDB shows you the all candidates,
> like:
>
> (gdb) print x.x
> Request for member 'x' is ambiguous in type 'X'. Candidates are:
> 'int A1::x' (X -> A1)
> 'int A2::x' (X -> A2)
> (gdb) print j.x
> Request for member 'x' is ambiguous in type 'J'. Candidates are:
> 'int A1::x' (J -> K -> A1)
> 'int A1::x' (J -> L -> A1)
Would it make sense to spare the users the touble of having to
re-type/cast things and, instead, display all the possible values with
an indication of what the fields are for each value?
That would be more intuitive to me, given the debugger is only
inspecting things and not enforcing a particular syntax for a source file.
Users tend to be lazy. They just want to see values, not be
syntactically correct. :-)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Reject ambiguous C++ field accesses
2020-08-27 18:45 ` Luis Machado
@ 2020-08-27 19:12 ` Pedro Alves
0 siblings, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2020-08-27 19:12 UTC (permalink / raw)
To: Luis Machado, gdb-patches
On 8/27/20 7:45 PM, Luis Machado wrote:
> On 8/27/20 3:02 PM, Pedro Alves wrote:
>> The gdb.cp/ambiguous.exp testcase had been disabled for many years,
>> but recently it was re-enabled. However, it is failing everywhere.
>> That is because it is testing an old feature that is gone from GDB.
>>
>> The testcase is expecting to see an ambiguous field warning, like:
>>
>> # X is derived from A1 and A2; both A1 and A2 have a member 'x'
>> send_gdb "print x.x\n"
>> gdb_expect {
>> -re "warning: x ambiguous; using X::A2::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
>> pass "print x.x"
>> }
>> -re "warning: x ambiguous; using X::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
>> pass "print x.x"
>> }
>> -re ".*$gdb_prompt $" { fail "print x.x" }
>> timeout { fail "(timeout) print x.x" }
>> }
>>
>> However, GDB just accesses one of the candidates without warning or
>> error:
>>
>> print x.x
>> $1 = 1431655296
>> (gdb) FAIL: gdb.cp/ambiguous.exp: print x.x
>>
>> (The weird number is because the testcase does not initialize the
>> variables.)
>>
>> The testcase come in originally with the big HP merge:
>>
>> +Sun Jan 10 23:44:11 1999 David Taylor <taylor@texas.cygnus.com>
>> +
>> +
>> + The following files are part of the HP merge; some had longer
>> + names at HP, but have been renamed to be no more than 14
>> + characters in length.
>>
>> Looking at the tree back then, we find that warning:
>>
>> /* Helper function used by value_struct_elt to recurse through baseclasses.
>> Look for a field NAME in ARG1. Adjust the address of ARG1 by OFFSET bytes,
>> and search in it assuming it has (class) type TYPE.
>> If found, return value, else return NULL.
>>
>> If LOOKING_FOR_BASECLASS, then instead of looking for struct fields,
>> look for a baseclass named NAME. */
>>
>> static value_ptr
>> search_struct_field (name, arg1, offset, type, looking_for_baseclass)
>> char *name;
>> register value_ptr arg1;
>> int offset;
>> register struct type *type;
>> int looking_for_baseclass;
>> {
>> int found = 0;
>> char found_class[1024];
>> value_ptr v;
>> struct type *vbase = NULL;
>>
>> found_class[0] = '\000';
>>
>> v = search_struct_field_aux (name, arg1, offset, type, looking_for_baseclass, &found, found_class, &vbase);
>> if (found > 1)
>> warning ("%s ambiguous; using %s::%s. Use a cast to disambiguate.",
>> name, found_class, name);
>>
>> return v;
>> }
>>
>>
>> However, in current GDB, search_struct_field does not handle the
>> ambiguous field case, nor is that warning found anywhere. Somehow it
>> got lost over the years. That seems like a regression, because the
>> compiler (as per language rules) rejects the ambiguous accesses as
>> well. E.g.:
>>
>> gdb.cp/ambiguous.cc:98:5: error: request for member 'x' is ambiguous
>> 98 | x.x = 1;
>> | ^
>> gdb.cp/ambiguous.cc:10:7: note: candidates are: 'int A2::x'
>> 10 | int x;
>> | ^
>> gdb.cp/ambiguous.cc:4:7: note: 'int A1::x'
>> 4 | int x;
>> | ^
>>
>>
>> This patch restores the feature, though implemented differently and
>> with better user experience, IMHO. An ambiguous access is now an
>> error instead of a warning, and also GDB shows you the all candidates,
>> like:
>>
>> (gdb) print x.x
>> Request for member 'x' is ambiguous in type 'X'. Candidates are:
>> 'int A1::x' (X -> A1)
>> 'int A2::x' (X -> A2)
>> (gdb) print j.x
>> Request for member 'x' is ambiguous in type 'J'. Candidates are:
>> 'int A1::x' (J -> K -> A1)
>> 'int A1::x' (J -> L -> A1)
>
> Would it make sense to spare the users the touble of having to re-type/cast things and, instead, display all the possible values with an indication of what the fields are for each value?
>
> That would be more intuitive to me, given the debugger is only inspecting things and not enforcing a particular syntax for a source file.
>
> Users tend to be lazy. They just want to see values, not be syntactically correct. :-)
>
No, that would not work with not-as-simple expressions. Like "p foo(x.x + 2)".
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Reject ambiguous C++ field accesses
2020-08-27 18:02 [PATCH] Reject ambiguous C++ field accesses Pedro Alves
2020-08-27 18:45 ` Luis Machado
@ 2020-08-27 19:58 ` Luis Machado
2020-08-28 14:35 ` Gary Benson
2020-08-28 20:12 ` [PATCH] " Pedro Alves
2020-08-28 14:38 ` Gary Benson
2 siblings, 2 replies; 10+ messages in thread
From: Luis Machado @ 2020-08-27 19:58 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 8/27/20 3:02 PM, Pedro Alves wrote:
> The gdb.cp/ambiguous.exp testcase had been disabled for many years,
> but recently it was re-enabled. However, it is failing everywhere.
> That is because it is testing an old feature that is gone from GDB.
>
> The testcase is expecting to see an ambiguous field warning, like:
>
> # X is derived from A1 and A2; both A1 and A2 have a member 'x'
> send_gdb "print x.x\n"
> gdb_expect {
> -re "warning: x ambiguous; using X::A2::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
> pass "print x.x"
> }
> -re "warning: x ambiguous; using X::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
> pass "print x.x"
> }
> -re ".*$gdb_prompt $" { fail "print x.x" }
> timeout { fail "(timeout) print x.x" }
> }
>
> However, GDB just accesses one of the candidates without warning or
> error:
>
> print x.x
> $1 = 1431655296
> (gdb) FAIL: gdb.cp/ambiguous.exp: print x.x
>
> (The weird number is because the testcase does not initialize the
> variables.)
>
> The testcase come in originally with the big HP merge:
>
> +Sun Jan 10 23:44:11 1999 David Taylor <taylor@texas.cygnus.com>
> +
> +
> + The following files are part of the HP merge; some had longer
> + names at HP, but have been renamed to be no more than 14
> + characters in length.
>
> Looking at the tree back then, we find that warning:
>
> /* Helper function used by value_struct_elt to recurse through baseclasses.
> Look for a field NAME in ARG1. Adjust the address of ARG1 by OFFSET bytes,
> and search in it assuming it has (class) type TYPE.
> If found, return value, else return NULL.
>
> If LOOKING_FOR_BASECLASS, then instead of looking for struct fields,
> look for a baseclass named NAME. */
>
> static value_ptr
> search_struct_field (name, arg1, offset, type, looking_for_baseclass)
> char *name;
> register value_ptr arg1;
> int offset;
> register struct type *type;
> int looking_for_baseclass;
> {
> int found = 0;
> char found_class[1024];
> value_ptr v;
> struct type *vbase = NULL;
>
> found_class[0] = '\000';
>
> v = search_struct_field_aux (name, arg1, offset, type, looking_for_baseclass, &found, found_class, &vbase);
> if (found > 1)
> warning ("%s ambiguous; using %s::%s. Use a cast to disambiguate.",
> name, found_class, name);
>
> return v;
> }
>
>
> However, in current GDB, search_struct_field does not handle the
> ambiguous field case, nor is that warning found anywhere. Somehow it
> got lost over the years. That seems like a regression, because the
> compiler (as per language rules) rejects the ambiguous accesses as
> well. E.g.:
>
> gdb.cp/ambiguous.cc:98:5: error: request for member 'x' is ambiguous
> 98 | x.x = 1;
> | ^
> gdb.cp/ambiguous.cc:10:7: note: candidates are: 'int A2::x'
> 10 | int x;
> | ^
> gdb.cp/ambiguous.cc:4:7: note: 'int A1::x'
> 4 | int x;
> | ^
>
>
> This patch restores the feature, though implemented differently and
> with better user experience, IMHO. An ambiguous access is now an
> error instead of a warning, and also GDB shows you the all candidates,
> like:
>
> (gdb) print x.x
> Request for member 'x' is ambiguous in type 'X'. Candidates are:
> 'int A1::x' (X -> A1)
> 'int A2::x' (X -> A2)
> (gdb) print j.x
> Request for member 'x' is ambiguous in type 'J'. Candidates are:
> 'int A1::x' (J -> K -> A1)
> 'int A1::x' (J -> L -> A1)
>
> Users can then fix their commands by casting or by specifying the
> baseclass explicitly, like:
>
> (gdb) p x.A1::x
> $1 = 1
> (gdb) p x.A2::x
> $2 = 2
> (gdb) p ((A1) x).x
> $3 = 1
> (gdb) p ((A2) x).x
> $4 = 2
> (gdb) p j.K::x
> $12 = 1
> (gdb) p j.L::x
> $13 = 2
> (gdb) p j.A1::x
> base class 'A1' is ambiguous in type 'J'
>
> The last error I've not touched; could be improved to also list the
> baseclass candidates.
>
> The showing the class "path" for each candidate was inspired by GCC's
> output when you try an ambiguous cast:
>
> gdb.cp/ambiguous.cc:161:8: error: ambiguous conversion from derived class 'const JVA1' to base class 'const A1':
> class JVA1 -> class KV -> class A1
> class JVA1 -> class A1
> (A1) jva1;
> ^~~~
>
> I did not include the "class" word as it seemed unnecessarily
> repetitive, but I can include it if people prefer it:
>
> (gdb) print j.x
> Request for member 'x' is ambiguous in type 'J'. Candidates are:
> 'int A1::x' (class J -> class K -> class A1)
> 'int A1::x' (class J -> class L -> class A1)
>
> The testcase is adjusted accordingly. I also took the chance to
> modernize it at the same time.
>
> Also, as mentioned above, the testcase doesn't currently initialize
> the tested variables. This patch inializes them all, giving each
> field a distinct value, so that we can be sure that GDB is accessing
> the right fields / offsets. The testcase is extended accordingly.
>
> Unfortunately, this exposes some bugs, not addressed in this patch.
> The bug is around a class that inherits from A1 directly and also
> inherits from with two other distinct base classes that inherit
> virtually from A1 in turn:
>
> print jva1.KV::x
> $51 = 1431665544
> (gdb) FAIL: gdb.cp/ambiguous.exp: all fields: print jva1.KV::x
> print jva1.KV::y
> $52 = 21845
> (gdb) FAIL: gdb.cp/ambiguous.exp: all fields: print jva1.KV::y
>
> This shows how GDB is clearly reading the confusing the offset of the
> _vptr.KV with the A1 offset:
>
> (gdb) print /x jva1
> $1 = {<KV> = {<A1> = {x = 0x2, y = 0x16}, _vptr.KV = 0x555555557b88 <vtable for JVA1+24>, i = 0x457}, ......}
> ^^^^^^^^^^^^^^
> (gdb) print /x jva1.KV::x
> $2 = 0x55557b88
> ^^^^^^^^^^
> (gdb) print /x jva1.KV::y
> $3 = 0x5555
> ^^^^^^
>
> (gdb) print /x (KV)jva1
> $4 = {<A1> = <invalid address>, _vptr.KV = 0x555555557b88 <vtable for JVA1+24>, i = 0x457}
> (gdb) print /x (A1)(KV)jva1
> Cannot access memory at address 0x0
>
> Since that's an orthogonal issue, I'm kfailing the tests that fail
> because of it. (I'll file a bug soon, unless someone can fix it
> quickly!)
>
> gdb/ChangeLog:
>
> * valops.c (struct struct_field_searcher): New.
> (update_search_result): Rename to ...
> (struct_field_searcher::update_result): ... this. Simplify
> prototype. Record all found fields.
> (do_search_struct_field): Rename to ...
> (struct_field_searcher::search): ... this. Simplify prototype.
> Maintain stack of visited baseclass path. Call update_result for
> fields too. Keep searching fields in baseclasses instead of
> stopping at the first found field.
> (search_struct_field): Use struct_field_searcher. When looking
> for fields, report ambiguous access attempts.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.cp/ambiguous.cc (marker1): Delete.
> (main): Initialize all the fields of the locals. Replace marker1
> call with a "set breakpoint here" marker.
> * gdb.cp/ambiguous.exp: Modernize. Use gdb_continue_to_breakpoint
> instead of running to marker1. Add tests printing all the
> variables and all the fields of the variables.
> (test_ambiguous): New proc, expecting the new GDB output when a
> field access is ambiguous. Change all "warning: X ambiguous"
> tests to use it.
> ---
> gdb/testsuite/gdb.cp/ambiguous.cc | 87 ++++++++--
> gdb/testsuite/gdb.cp/ambiguous.exp | 321 +++++++++++++++++++++----------------
> gdb/valops.c | 219 ++++++++++++++++++-------
> 3 files changed, 418 insertions(+), 209 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.cp/ambiguous.cc b/gdb/testsuite/gdb.cp/ambiguous.cc
> index 6ee7bc18ea9..a55686547f2 100644
> --- a/gdb/testsuite/gdb.cp/ambiguous.cc
> +++ b/gdb/testsuite/gdb.cp/ambiguous.cc
> @@ -1,9 +1,4 @@
>
> -void marker1()
> -{
> - return;
> -}
> -
> class A1 {
> public:
> int x;
> @@ -102,9 +97,81 @@ int main()
>
> i += k.i + m.w + a1.x + a2.x + a3.x + x.z + l.z + n.r + j.j;
>
> - marker1();
> -
> + /* Initialize all the fields. Keep the order the same as in the
> + .exp file. */
> +
> + a1.x = 1;
> + a1.y = 2;
> +
> + a2.x = 1;
> + a2.y = 2;
> +
> + a3.x = 1;
> + a3.y = 2;
> +
> + x.A1::x = 1;
> + x.A1::y = 2;
> + x.A2::x = 3;
> + x.A2::y = 4;
> + x.z = 5;
> +
> + l.x = 1;
> + l.y = 2;
> + l.z = 3;
> +
> + m.x = 1;
> + m.y = 2;
> + m.w = 3;
> +
> + n.A1::x = 1;
> + n.A1::y = 2;
> + n.A2::x = 3;
> + n.A2::y = 4;
> + n.w = 5;
> + n.r = 6;
> + n.z = 7;
> +
> + k.x = 1;
> + k.y = 2;
> + k.i = 3;
> +
> + j.K::x = 1;
> + j.K::y = 2;
> + j.L::x = 3;
> + j.L::y = 4;
> + j.i = 5;
> + j.z = 6;
> + j.j = 7;
> +
> + jv.x = 1;
> + jv.y = 2;
> + jv.i = 3;
> + jv.z = 4;
> + jv.jv = 5;
> +
> + jva1.KV::x = 1;
> + jva1.KV::y = 2;
> + jva1.LV::x = 3;
> + jva1.LV::y = 4;
> + jva1.z = 5;
> + jva1.i = 6;
> + jva1.jva1 = 7;
> +
> + jva2.KV::x = 1;
> + jva2.KV::y = 2;
> + jva2.LV::x = 3;
> + jva2.LV::y = 4;
> + jva2.A2::x = 5;
> + jva2.A2::y = 6;
> + jva2.z = 7;
> + jva2.i = 8;
> + jva2.jva2 = 9;
> +
> + jva1v.x = 1;
> + jva1v.y = 2;
> + jva1v.z = 3;
> + jva1v.i = 4;
> + jva1v.jva1v = 5;
> +
> + return 0; /* set breakpoint here */
> }
> -
> -
> -
> diff --git a/gdb/testsuite/gdb.cp/ambiguous.exp b/gdb/testsuite/gdb.cp/ambiguous.exp
> index 17fb29f5350..74783cd0725 100644
> --- a/gdb/testsuite/gdb.cp/ambiguous.exp
> +++ b/gdb/testsuite/gdb.cp/ambiguous.exp
> @@ -15,15 +15,9 @@
>
> # This file is part of the gdb testsuite
>
> -# tests relating to ambiguous class members
> -# Written by Satish Pai <pai@apollo.hp.com> 1997-07-28
> -
> -# This file is part of the gdb testsuite
> -
> -#
> -# test running programs
> -#
> -
> +# Print out various class objects' members and check that the error
> +# about the field or baseclass being ambiguious is emitted at the
> +# right times.
Typo, ambiguious.
>
> if { [skip_cplus_tests] } { continue }
>
> @@ -47,180 +41,225 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
> return -1
> }
>
> -#
> -# set it up at a breakpoint so we can play with the variable values
> -#
> +# Set it up at a breakpoint so we can play with the variable values.
> +
This old comment reads funny. Set what up? I'd rephrase a bit.
> diff --git a/gdb/valops.c b/gdb/valops.c
> index 0eb2b096211..0e030a03ecd 100644
> --- a/gdb/valops.c
> +++ b/gdb/valops.c
> @@ -1766,25 +1766,105 @@ typecmp (int staticp, int varargs, int nargs,
> return i + 1;
> }
>
> -/* Helper class for do_search_struct_field that updates *RESULT_PTR
> - and *LAST_BOFFSET, and possibly throws an exception if the field
> - search has yielded ambiguous results. */
> +/* Helper class for search_struct_field that keeps track of found
> + results. See search_struct_field for description of
> + LOOKING_FOR_BASECLASS. If LOOKING_FOR_BASECLASS is true, possibly
> + throws an exception if the base class search has yielded ambiguous
"throw" an exception instead?
> + results. If LOOKING_FOR_BASECLASS is false, found fields are
> + accumulated and the caller (search_struct_field takes) care of
"takes" outside the parenthesis.
> + throwing an error if the field search yielded ambiguous results.
> + The latter is done that way so that the error message can include a
> + list of all the found candidates. */
> +
> +struct struct_field_searcher
> +{
> + /* A found field. */
> + struct found_field
> + {
> + /* Patch to the structure where the field was found. */
Typo... Path.
> + std::vector<struct type *> path;
> +
> + /* The field found. */
> + struct value *field_value;
> + };
> +
> + struct_field_searcher (const char *name,
> + struct type *outermost_type,
> + bool looking_for_baseclass)
> + : m_name (name),
> + m_looking_for_baseclass (looking_for_baseclass),
> + m_outermost_type (outermost_type)
> + {
> + }
>
> -static void
> -update_search_result (struct value **result_ptr, struct value *v,
> - LONGEST *last_boffset, LONGEST boffset,
> - const char *name, struct type *type)
> + void search (struct value *arg1, LONGEST offset,
> + struct type *type);
> +
> + const std::vector<found_field> &fields ()
> + {
> + return m_fields;
> + }
> +
> + struct value *baseclass ()
> + {
> + return m_baseclass;
> + }
> +
> +private:
> + /* Update results to include V, a found field/baseclass. */
> + void update_result (struct value *v, LONGEST boffset);
> +
> + /* The name of the field/baseclass we're searching for. */
> + const char *m_name;
> +
> + /* Whether we're looking for a baseclass, or a field. */
> + const bool m_looking_for_baseclass;
> +
> + /* The offset of base class containing the field/baseclass we last
> + recorded. */
Maybe "offset into base class"?
> + LONGEST m_last_boffset = 0;
> +
> + /* If looking for a baseclass, then the result is stored here. */
> + struct value *m_baseclass = nullptr;
> +
> + /* When looking for fields, the found candidates are stored
> + here. */
> + std::vector<found_field> m_fields;
> +
> + /* The type of the initial type passed to search_struct_field; this
> + is used for error reporting when the lookup is ambiguous. */
> + struct type *m_outermost_type;
> +
> + /* The full path to the struct being inspected. E.g. for field 'x'
> + defined in class B inherited by class A, we have A and B pushed
> + on the path. */
> + std::vector <struct type *> m_struct_path;
> +};
> +
> +void
> +struct_field_searcher::update_result (struct value *v, LONGEST boffset)
> {
> if (v != NULL)
> {
> - if (*result_ptr != NULL
> - /* The result is not ambiguous if all the classes that are
> - found occupy the same space. */
> - && *last_boffset != boffset)
> - error (_("base class '%s' is ambiguous in type '%s'"),
> - name, TYPE_SAFE_NAME (type));
> - *result_ptr = v;
> - *last_boffset = boffset;
> + if (m_looking_for_baseclass)
> + {
> + if (m_baseclass != nullptr
> + /* The result is not ambiguous if all the classes that are
> + found occupy the same space. */
> + && m_last_boffset != boffset)
> + error (_("base class '%s' is ambiguous in type '%s'"),
> + m_name, TYPE_SAFE_NAME (m_outermost_type));
> +
> + m_baseclass = v;
> + m_last_boffset = boffset;
> + }
> + else
> + {
> + /* The field is not ambiguous if it occupies the same
> + space. */
> + if (m_fields.empty () || m_last_boffset != boffset)
> + m_fields.push_back ({m_struct_path, v});
> + }
> }
> }
>
> @@ -1795,25 +1875,25 @@ update_search_result (struct value **result_ptr, struct value *v,
> search_struct_field; this is used for error reporting when the
> lookup is ambiguous. */
>
> -static void
> -do_search_struct_field (const char *name, struct value *arg1, LONGEST offset,
> - struct type *type, int looking_for_baseclass,
> - struct value **result_ptr,
> - LONGEST *last_boffset,
> - struct type *outermost_type)
> +void
> +struct_field_searcher::search (struct value *arg1, LONGEST offset,
> + struct type *type)
> {
> int i;
> int nbases;
>
> + m_struct_path.push_back (type);
> + SCOPE_EXIT { m_struct_path.pop_back (); };
> +
> type = check_typedef (type);
> nbases = TYPE_N_BASECLASSES (type);
>
> - if (!looking_for_baseclass)
> + if (!m_looking_for_baseclass)
> for (i = type->num_fields () - 1; i >= nbases; i--)
> {
> const char *t_field_name = TYPE_FIELD_NAME (type, i);
>
> - if (t_field_name && (strcmp_iw (t_field_name, name) == 0))
> + if (t_field_name && (strcmp_iw (t_field_name, m_name) == 0))
> {
> struct value *v;
>
> @@ -1821,7 +1901,8 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset,
> v = value_static_field (type, i);
> else
> v = value_primitive_field (arg1, offset, i, type);
> - *result_ptr = v;
> +
> + update_result (v, offset);
> return;
> }
>
> @@ -1845,7 +1926,6 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset,
> represented as a struct, with a member for each
> <variant field>. */
>
> - struct value *v = NULL;
> LONGEST new_offset = offset;
>
> /* This is pretty gross. In G++, the offset in an
> @@ -1859,16 +1939,7 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset,
> && TYPE_FIELD_BITPOS (field_type, 0) == 0))
> new_offset += TYPE_FIELD_BITPOS (type, i) / 8;
>
> - do_search_struct_field (name, arg1, new_offset,
> - field_type,
> - looking_for_baseclass, &v,
> - last_boffset,
> - outermost_type);
> - if (v)
> - {
> - *result_ptr = v;
> - return;
> - }
> + search (arg1, new_offset, field_type);
> }
> }
> }
> @@ -1880,10 +1951,10 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset,
> /* If we are looking for baseclasses, this is what we get when
> we hit them. But it could happen that the base part's member
> name is not yet filled in. */
> - int found_baseclass = (looking_for_baseclass
> + int found_baseclass = (m_looking_for_baseclass
> && TYPE_BASECLASS_NAME (type, i) != NULL
> - && (strcmp_iw (name,
> - TYPE_BASECLASS_NAME (type,
> + && (strcmp_iw (m_name,
> + TYPE_BASECLASS_NAME (type,
> i)) == 0));
> LONGEST boffset = value_embedded_offset (arg1) + offset;
>
> @@ -1924,28 +1995,17 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset,
> if (found_baseclass)
> v = v2;
> else
> - {
> - do_search_struct_field (name, v2, 0,
> - TYPE_BASECLASS (type, i),
> - looking_for_baseclass,
> - result_ptr, last_boffset,
> - outermost_type);
> - }
> + search (v2, 0, TYPE_BASECLASS (type, i));
> }
> else if (found_baseclass)
> v = value_primitive_field (arg1, offset, i, type);
> else
> {
> - do_search_struct_field (name, arg1,
> - offset + TYPE_BASECLASS_BITPOS (type,
> - i) / 8,
> - basetype, looking_for_baseclass,
> - result_ptr, last_boffset,
> - outermost_type);
> + search (arg1, offset + TYPE_BASECLASS_BITPOS (type, i) / 8,
> + basetype);
> }
>
> - update_search_result (result_ptr, v, last_boffset,
> - boffset, name, outermost_type);
> + update_result (v, boffset);
> }
> }
>
> @@ -1960,12 +2020,55 @@ static struct value *
> search_struct_field (const char *name, struct value *arg1,
> struct type *type, int looking_for_baseclass)
> {
> - struct value *result = NULL;
> - LONGEST boffset = 0;
> + struct_field_searcher searcher (name, type, looking_for_baseclass);
>
> - do_search_struct_field (name, arg1, 0, type, looking_for_baseclass,
> - &result, &boffset, type);
> - return result;
> + searcher.search (arg1, 0, type);
> +
> + if (!looking_for_baseclass)
> + {
> + const auto &fields = searcher.fields ();
> +
> + if (fields.empty ())
> + return nullptr;
> + else if (fields.size () > 1)
> + {
> + std::string candidates;
> +
> + for (auto &&candidate : fields)
> + {
> + gdb_assert (!candidate.path.empty ());
> +
> + struct type *field_type = value_type (candidate.field_value);
> + struct type *struct_type = candidate.path.back ();
> +
> + std::string path;
> + bool first = true;
> + for (struct type *t : candidate.path)
> + {
> + if (first)
> + first = false;
> + else
> + path += " -> ";
> + path += t->name ();
> + }
> +
> + candidates += string_printf ("\n '%s %s::%s' (%s)",
> + TYPE_SAFE_NAME (field_type),
> + TYPE_SAFE_NAME (struct_type),
> + name,
> + path.c_str ());
> + }
> +
> + error (_("Request for member '%s' is ambiguous in type '%s'."
> + " Candidates are:%s"),
> + name, TYPE_SAFE_NAME (type),
> + candidates.c_str ());
> + }
> + else
> + return fields[0].field_value;
> + }
> + else
> + return searcher.baseclass ();
> }
>
> /* Helper function used by value_struct_elt to recurse through
>
> base-commit: 8c51f2f291a5459e1eabd000b2c52e5de52b4c56
>
Otherwise LGTM. I exercised this for aarch64-linux and see no more
failures and 10 KFAIL's.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Reject ambiguous C++ field accesses
2020-08-27 19:58 ` Luis Machado
@ 2020-08-28 14:35 ` Gary Benson
2020-08-28 20:22 ` [PATCH v2] " Pedro Alves
2020-08-28 20:12 ` [PATCH] " Pedro Alves
1 sibling, 1 reply; 10+ messages in thread
From: Gary Benson @ 2020-08-28 14:35 UTC (permalink / raw)
To: Luis Machado; +Cc: Pedro Alves, gdb-patches
Luis Machado wrote:
> On 8/27/20 3:02 PM, Pedro Alves wrote:
> > diff --git a/gdb/valops.c b/gdb/valops.c
> > index 0eb2b096211..0e030a03ecd 100644
> > --- a/gdb/valops.c
> > +++ b/gdb/valops.c
> > @@ -1766,25 +1766,105 @@ typecmp (int staticp, int varargs, int nargs,
> > return i + 1;
> > }
> > -/* Helper class for do_search_struct_field that updates *RESULT_PTR
> > - and *LAST_BOFFSET, and possibly throws an exception if the field
> > - search has yielded ambiguous results. */
> > +/* Helper class for search_struct_field that keeps track of found
> > + results. See search_struct_field for description of
> > + LOOKING_FOR_BASECLASS. If LOOKING_FOR_BASECLASS is true, possibly
> > + throws an exception if the base class search has yielded ambiguous
>
> "throw" an exception instead?
I think remove "possibly" and "has" also.
"If LOOKING_FOR_BASECLASS is true, throw an exception if the base
class search yielded ambiguous results."
Thanks,
Gary
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Reject ambiguous C++ field accesses
2020-08-27 18:02 [PATCH] Reject ambiguous C++ field accesses Pedro Alves
2020-08-27 18:45 ` Luis Machado
2020-08-27 19:58 ` Luis Machado
@ 2020-08-28 14:38 ` Gary Benson
2 siblings, 0 replies; 10+ messages in thread
From: Gary Benson @ 2020-08-28 14:38 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves wrote:
> An ambiguous access is now an error instead of a warning, and also
> GDB shows you the all candidates, like:
>
> (gdb) print x.x
> Request for member 'x' is ambiguous in type 'X'. Candidates are:
> 'int A1::x' (X -> A1)
> 'int A2::x' (X -> A2)
> (gdb) print j.x
> Request for member 'x' is ambiguous in type 'J'. Candidates are:
> 'int A1::x' (J -> K -> A1)
> 'int A1::x' (J -> L -> A1)
That's really nice, thanks for doing this!
I replied with a comment tweak in another message, but other than
that and the things Luis found, LGTM.
Thanks,
Gary
--
Gary Benson - he / him / his
Principal Software Engineer, Red Hat
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Reject ambiguous C++ field accesses
2020-08-27 19:58 ` Luis Machado
2020-08-28 14:35 ` Gary Benson
@ 2020-08-28 20:12 ` Pedro Alves
1 sibling, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2020-08-28 20:12 UTC (permalink / raw)
To: Luis Machado, gdb-patches
On 8/27/20 8:58 PM, Luis Machado wrote:
>> diff --git a/gdb/testsuite/gdb.cp/ambiguous.exp b/gdb/testsuite/gdb.cp/ambiguous.exp
>> index 17fb29f5350..74783cd0725 100644
>> --- a/gdb/testsuite/gdb.cp/ambiguous.exp
>> +++ b/gdb/testsuite/gdb.cp/ambiguous.exp
>> @@ -15,15 +15,9 @@
>> # This file is part of the gdb testsuite
>> -# tests relating to ambiguous class members
>> -# Written by Satish Pai <pai@apollo.hp.com> 1997-07-28
>> -
>> -# This file is part of the gdb testsuite
>> -
>> -#
>> -# test running programs
>> -#
>> -
>> +# Print out various class objects' members and check that the error
>> +# about the field or baseclass being ambiguious is emitted at the
>> +# right times.
>
> Typo, ambiguious.
Thanks, fixed.
>
>> if { [skip_cplus_tests] } { continue }
>> @@ -47,180 +41,225 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
>> return -1
>> }
>> -#
>> -# set it up at a breakpoint so we can play with the variable values
>> -#
>> +# Set it up at a breakpoint so we can play with the variable values.
>> +
>
> This old comment reads funny. Set what up? I'd rephrase a bit.
This comment is just before the starting up the inferior and
running to a breakpoint. It's talking about that. Setting
up the inferior ready for testing. I changed it to:
# Run to a breakpoint after the variables have been initialized so we
# can play with the variable values.
But I guess I could just remove it altogether, since it's
pretty obvious what we're doing.
>> diff --git a/gdb/valops.c b/gdb/valops.c
>> index 0eb2b096211..0e030a03ecd 100644
>> --- a/gdb/valops.c
>> +++ b/gdb/valops.c
>> @@ -1766,25 +1766,105 @@ typecmp (int staticp, int varargs, int nargs,
>> return i + 1;
>> }
>> -/* Helper class for do_search_struct_field that updates *RESULT_PTR
>> - and *LAST_BOFFSET, and possibly throws an exception if the field
>> - search has yielded ambiguous results. */
>> +/* Helper class for search_struct_field that keeps track of found
>> + results. See search_struct_field for description of
>> + LOOKING_FOR_BASECLASS. If LOOKING_FOR_BASECLASS is true, possibly
>> + throws an exception if the base class search has yielded ambiguous
>
> "throw" an exception instead?
I'll handle this one in response to Gary.
>
>> + results. If LOOKING_FOR_BASECLASS is false, found fields are
>> + accumulated and the caller (search_struct_field takes) care of
>
> "takes" outside the parenthesis.
Fixed.
>
>> + throwing an error if the field search yielded ambiguous results.
>> + The latter is done that way so that the error message can include a
>> + list of all the found candidates. */
>> +
>> +struct struct_field_searcher
>> +{
>> + /* A found field. */
>> + struct found_field
>> + {
>> + /* Patch to the structure where the field was found. */
>
> Typo... Path.
:-) Fixed.
>> + /* Whether we're looking for a baseclass, or a field. */
>> + const bool m_looking_for_baseclass;
>> +
>> + /* The offset of base class containing the field/baseclass we last
>> + recorded. */
>
> Maybe "offset into base class"?
Thanks. However, it's the offset of the class itself, not the field.
So I've changed it to:
/* The offset of the baseclass containing the field/baseclass we
last recorded. */
> Otherwise LGTM. I exercised this for aarch64-linux and see no more failures and 10 KFAIL's.
Great, thanks!
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] Reject ambiguous C++ field accesses
2020-08-28 14:35 ` Gary Benson
@ 2020-08-28 20:22 ` Pedro Alves
2020-10-12 17:12 ` Pedro Alves
0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2020-08-28 20:22 UTC (permalink / raw)
To: Gary Benson, Luis Machado; +Cc: gdb-patches
On 8/28/20 3:35 PM, Gary Benson wrote:
> Luis Machado wrote:
>> On 8/27/20 3:02 PM, Pedro Alves wrote:
>>> diff --git a/gdb/valops.c b/gdb/valops.c
>>> index 0eb2b096211..0e030a03ecd 100644
>>> --- a/gdb/valops.c
>>> +++ b/gdb/valops.c
>>> @@ -1766,25 +1766,105 @@ typecmp (int staticp, int varargs, int nargs,
>>> return i + 1;
>>> }
>>> -/* Helper class for do_search_struct_field that updates *RESULT_PTR
>>> - and *LAST_BOFFSET, and possibly throws an exception if the field
>>> - search has yielded ambiguous results. */
>>> +/* Helper class for search_struct_field that keeps track of found
>>> + results. See search_struct_field for description of
>>> + LOOKING_FOR_BASECLASS. If LOOKING_FOR_BASECLASS is true, possibly
>>> + throws an exception if the base class search has yielded ambiguous
>>
>> "throw" an exception instead?
>
> I think remove "possibly" and "has" also.
>
> "If LOOKING_FOR_BASECLASS is true, throw an exception if the base
> class search yielded ambiguous results."
I guess also yielded -> yields.
I don't think "throw" would be more correct, because it's intended as
"the helper class that throws, somewhere in some function".
"Helper class that keeps track .... And possibly throws."
I've now kept the first sentence closer to what it looked like before:
/* Helper class for search_struct_field that keeps track of found
results and possibly throws an exception if the search yields
ambiguous results. See search_struct_field for description of
LOOKING_FOR_BASECLASS. */
struct struct_field_searcher
{
I've kept the "possibly" here because whether an exception is thrown
depends on LOOKING_FOR_BASECLASS. And then I moved the detailed description
to the search method:
/* The search entry point. If LOOKING_FOR_BASECLASS is true and the
base class search yields ambiguous results, this throws an
exception. If LOOKING_FOR_BASECLASS is false, the found fields
are accumulated and the caller (search_struct_field) takes care
of throwing an error if the field search yields ambiguous
results. The latter is done that way so that the error message
can include a list of all the found candidates. */
void search (struct value *arg, LONGEST offset, struct type *type);
I hope it's clearer this way.
I've filed PR c++/26550 for the KFAILs now.
Here's the updated patch.
From 24cff92ab462aeeeb820c531a3f7b5525dc64c99 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Fri, 28 Aug 2020 21:10:59 +0100
Subject: [PATCH v2] Reject ambiguous C++ field accesses
The gdb.cp/ambiguous.exp testcase had been disabled for many years,
but recently it was re-enabled. However, it is failing everywhere.
That is because it is testing an old feature that is gone from GDB.
The testcase is expecting to see an ambiguous field warning, like:
# X is derived from A1 and A2; both A1 and A2 have a member 'x'
send_gdb "print x.x\n"
gdb_expect {
-re "warning: x ambiguous; using X::A2::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
pass "print x.x"
}
-re "warning: x ambiguous; using X::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
pass "print x.x"
}
-re ".*$gdb_prompt $" { fail "print x.x" }
timeout { fail "(timeout) print x.x" }
}
However, GDB just accesses one of the candidates without warning or
error:
print x.x
$1 = 1431655296
(gdb) FAIL: gdb.cp/ambiguous.exp: print x.x
(The weird number is because the testcase does not initialize the
variables.)
The testcase come in originally with the big HP merge:
+Sun Jan 10 23:44:11 1999 David Taylor <taylor@texas.cygnus.com>
+
+
+ The following files are part of the HP merge; some had longer
+ names at HP, but have been renamed to be no more than 14
+ characters in length.
Looking at the tree back then, we find that warning:
/* Helper function used by value_struct_elt to recurse through baseclasses.
Look for a field NAME in ARG1. Adjust the address of ARG1 by OFFSET bytes,
and search in it assuming it has (class) type TYPE.
If found, return value, else return NULL.
If LOOKING_FOR_BASECLASS, then instead of looking for struct fields,
look for a baseclass named NAME. */
static value_ptr
search_struct_field (name, arg1, offset, type, looking_for_baseclass)
char *name;
register value_ptr arg1;
int offset;
register struct type *type;
int looking_for_baseclass;
{
int found = 0;
char found_class[1024];
value_ptr v;
struct type *vbase = NULL;
found_class[0] = '\000';
v = search_struct_field_aux (name, arg1, offset, type, looking_for_baseclass, &found, found_class, &vbase);
if (found > 1)
warning ("%s ambiguous; using %s::%s. Use a cast to disambiguate.",
name, found_class, name);
return v;
}
However, in current GDB, search_struct_field does not handle the
ambiguous field case, nor is that warning found anywhere. Somehow it
got lost over the years. That seems like a regression, because the
compiler (as per language rules) rejects the ambiguous accesses as
well. E.g.:
gdb.cp/ambiguous.cc:98:5: error: request for member 'x' is ambiguous
98 | x.x = 1;
| ^
gdb.cp/ambiguous.cc:10:7: note: candidates are: 'int A2::x'
10 | int x;
| ^
gdb.cp/ambiguous.cc:4:7: note: 'int A1::x'
4 | int x;
| ^
This patch restores the feature, though implemented differently and
with better user experience, IMHO. An ambiguous access is now an
error instead of a warning, and also GDB shows you all the candidates,
like:
(gdb) print x.x
Request for member 'x' is ambiguous in type 'X'. Candidates are:
'int A1::x' (X -> A1)
'int A2::x' (X -> A2)
(gdb) print j.x
Request for member 'x' is ambiguous in type 'J'. Candidates are:
'int A1::x' (J -> K -> A1)
'int A1::x' (J -> L -> A1)
Users can then fix their commands by casting or by specifying the
baseclass explicitly, like:
(gdb) p x.A1::x
$1 = 1
(gdb) p x.A2::x
$2 = 2
(gdb) p ((A1) x).x
$3 = 1
(gdb) p ((A2) x).x
$4 = 2
(gdb) p j.K::x
$12 = 1
(gdb) p j.L::x
$13 = 2
(gdb) p j.A1::x
base class 'A1' is ambiguous in type 'J'
The last error I've not touched; could be improved to also list the
baseclass candidates.
The showing the class "path" for each candidate was inspired by GCC's
output when you try an ambiguous cast:
gdb.cp/ambiguous.cc:161:8: error: ambiguous conversion from derived class 'const JVA1' to base class 'const A1':
class JVA1 -> class KV -> class A1
class JVA1 -> class A1
(A1) jva1;
^~~~
I did not include the "class" word as it seemed unnecessarily
repetitive, but I can include it if people prefer it:
(gdb) print j.x
Request for member 'x' is ambiguous in type 'J'. Candidates are:
'int A1::x' (class J -> class K -> class A1)
'int A1::x' (class J -> class L -> class A1)
The testcase is adjusted accordingly. I also took the chance to
modernize it at the same time.
Also, as mentioned above, the testcase doesn't currently initialize
the tested variables. This patch inializes them all, giving each
field a distinct value, so that we can be sure that GDB is accessing
the right fields / offsets. The testcase is extended accordingly.
Unfortunately, this exposes a bug, not addressed in this patch. The
bug is around a class that inherits from A1 directly and also inherits
from two other distinct base classes that inherit virtually from A1 in
turn:
print jva1.KV::x
$51 = 1431665544
(gdb) FAIL: gdb.cp/ambiguous.exp: all fields: print jva1.KV::x
print jva1.KV::y
$52 = 21845
(gdb) FAIL: gdb.cp/ambiguous.exp: all fields: print jva1.KV::y
(gdb) print /x (KV)jva1
$4 = {<A1> = <invalid address>, _vptr.KV = 0x555555557b88 <vtable for JVA1+24>, i = 0x457}
(gdb) print /x (A1)(KV)jva1
Cannot access memory at address 0x0
Since that's an orthogonal issue, I filed PR c++/26550 and kfailed the
tests that fail because of it.
gdb/ChangeLog:
* valops.c (struct struct_field_searcher): New.
(update_search_result): Rename to ...
(struct_field_searcher::update_result): ... this. Simplify
prototype. Record all found fields.
(do_search_struct_field): Rename to ...
(struct_field_searcher::search): ... this. Simplify prototype.
Maintain stack of visited baseclass path. Call update_result for
fields too. Keep searching fields in baseclasses instead of
stopping at the first found field.
(search_struct_field): Use struct_field_searcher. When looking
for fields, report ambiguous access attempts.
gdb/testsuite/ChangeLog:
PR c++/26550
* gdb.cp/ambiguous.cc (marker1): Delete.
(main): Initialize all the fields of the locals. Replace marker1
call with a "set breakpoint here" marker.
* gdb.cp/ambiguous.exp: Modernize. Use gdb_continue_to_breakpoint
instead of running to marker1. Add tests printing all the
variables and all the fields of the variables.
(test_ambiguous): New proc, expecting the new GDB output when a
field access is ambiguous. Change all "warning: X ambiguous"
tests to use it.
---
gdb/testsuite/gdb.cp/ambiguous.cc | 87 ++++++++--
gdb/testsuite/gdb.cp/ambiguous.exp | 322 +++++++++++++++++++++----------------
gdb/valops.c | 227 ++++++++++++++++++--------
3 files changed, 422 insertions(+), 214 deletions(-)
diff --git a/gdb/testsuite/gdb.cp/ambiguous.cc b/gdb/testsuite/gdb.cp/ambiguous.cc
index 6ee7bc18ea9..a55686547f2 100644
--- a/gdb/testsuite/gdb.cp/ambiguous.cc
+++ b/gdb/testsuite/gdb.cp/ambiguous.cc
@@ -1,9 +1,4 @@
-void marker1()
-{
- return;
-}
-
class A1 {
public:
int x;
@@ -102,9 +97,81 @@ int main()
i += k.i + m.w + a1.x + a2.x + a3.x + x.z + l.z + n.r + j.j;
- marker1();
-
+ /* Initialize all the fields. Keep the order the same as in the
+ .exp file. */
+
+ a1.x = 1;
+ a1.y = 2;
+
+ a2.x = 1;
+ a2.y = 2;
+
+ a3.x = 1;
+ a3.y = 2;
+
+ x.A1::x = 1;
+ x.A1::y = 2;
+ x.A2::x = 3;
+ x.A2::y = 4;
+ x.z = 5;
+
+ l.x = 1;
+ l.y = 2;
+ l.z = 3;
+
+ m.x = 1;
+ m.y = 2;
+ m.w = 3;
+
+ n.A1::x = 1;
+ n.A1::y = 2;
+ n.A2::x = 3;
+ n.A2::y = 4;
+ n.w = 5;
+ n.r = 6;
+ n.z = 7;
+
+ k.x = 1;
+ k.y = 2;
+ k.i = 3;
+
+ j.K::x = 1;
+ j.K::y = 2;
+ j.L::x = 3;
+ j.L::y = 4;
+ j.i = 5;
+ j.z = 6;
+ j.j = 7;
+
+ jv.x = 1;
+ jv.y = 2;
+ jv.i = 3;
+ jv.z = 4;
+ jv.jv = 5;
+
+ jva1.KV::x = 1;
+ jva1.KV::y = 2;
+ jva1.LV::x = 3;
+ jva1.LV::y = 4;
+ jva1.z = 5;
+ jva1.i = 6;
+ jva1.jva1 = 7;
+
+ jva2.KV::x = 1;
+ jva2.KV::y = 2;
+ jva2.LV::x = 3;
+ jva2.LV::y = 4;
+ jva2.A2::x = 5;
+ jva2.A2::y = 6;
+ jva2.z = 7;
+ jva2.i = 8;
+ jva2.jva2 = 9;
+
+ jva1v.x = 1;
+ jva1v.y = 2;
+ jva1v.z = 3;
+ jva1v.i = 4;
+ jva1v.jva1v = 5;
+
+ return 0; /* set breakpoint here */
}
-
-
-
diff --git a/gdb/testsuite/gdb.cp/ambiguous.exp b/gdb/testsuite/gdb.cp/ambiguous.exp
index 17fb29f5350..b7fec1b2bb3 100644
--- a/gdb/testsuite/gdb.cp/ambiguous.exp
+++ b/gdb/testsuite/gdb.cp/ambiguous.exp
@@ -15,15 +15,9 @@
# This file is part of the gdb testsuite
-# tests relating to ambiguous class members
-# Written by Satish Pai <pai@apollo.hp.com> 1997-07-28
-
-# This file is part of the gdb testsuite
-
-#
-# test running programs
-#
-
+# Print out various class objects' members and check that the error
+# about the field or baseclass being ambiguous is emitted at the right
+# times.
if { [skip_cplus_tests] } { continue }
@@ -47,180 +41,226 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
return -1
}
-#
-# set it up at a breakpoint so we can play with the variable values
-#
if ![runto_main] then {
perror "couldn't run to breakpoint"
continue
}
-send_gdb "break marker1\n" ; gdb_expect -re ".*$gdb_prompt $"
- send_gdb "cont\n"
- gdb_expect {
- -re "Break.* marker1 \\(\\) at .*:$decimal.*$gdb_prompt $" {
- send_gdb "up\n"
- gdb_expect {
- -re ".*$gdb_prompt $" { pass "up from marker1" }
- timeout { fail "up from marker1" }
- }
- }
- -re "$gdb_prompt $" { fail "continue to marker1" }
- timeout { fail "(timeout) continue to marker1" }
+# Run to a breakpoint after the variables have been initialized so we
+# can play with the variable values.
+
+set lineno [gdb_get_line_number "set breakpoint here"]
+
+gdb_breakpoint $lineno
+gdb_continue_to_breakpoint "breakpoint here"
+
+set number -?$decimal
+
+with_test_prefix "all vars" {
+ gdb_test "print a1" \
+ " = \{x = 1, y = 2\}"
+
+ gdb_test "print a2" \
+ " = \{x = 1, y = 2\}"
+
+ gdb_test "print a3" \
+ " = \{x = 1, y = 2\}"
+
+ gdb_test "print x" \
+ " = \{<A1> = \{x = 1, y = 2\}, <A2> = \{x = 3, y = 4\}, z = 5\}"
+
+ gdb_test "print l" \
+ " = \{<A1> = \{x = 1, y = 2\}, z = 3\}"
+
+ gdb_test "print m" \
+ " = \{<A2> = \{x = 1, y = 2\}, w = 3\}"
+
+ gdb_test "print n" \
+ " = \{<L> = \{<A1> = \{x = 1, y = 2\}, z = 7\}, <M> = \{<A2> = \{x = 3, y = 4\}, w = 5\}, r = 6\}"
+
+ gdb_test "print k" \
+ " = \{<A1> = \{x = 1, y = 2\}, i = 3\}"
+
+ gdb_test "print j" \
+ " = {<K> = {<A1> = {x = 1, y = 2}, i = 5}, <L> = {<A1> = {x = 3, y = 4}, z = 6}, j = 7}"
+
+ gdb_test "print jv" \
+ " = \{<KV> = \{<A1> = \{x = 1, y = 2\}, _vptr.KV = $hex <vtable for JV.*>, i = 3\}, <LV> = \{_vptr.LV = $hex <VTT for JV>, z = 4\}, jv = 5\}"
+
+ # No way to initialize one of the A1's, so just take any number there.
+ gdb_test "print jva1" \
+ " = \{<KV> = \{<A1> = \{x = 3, y = 4\}, _vptr.KV = $hex <vtable for JVA1.*>, i = 6\}, <LV> = \{_vptr.LV = $hex <VTT for JVA1>, z = 5\}, <A1> = \{x = $number, y = $number\}, jva1 = 7\}"
+
+ gdb_test "print jva2" \
+ " = \{<KV> = \{<A1> = \{x = 3, y = 4\}, _vptr.KV = $hex <vtable for JVA2.*>, i = 8\}, <LV> = \{_vptr.LV = $hex <VTT for JVA2>, z = 7\}, <A2> = \{x = 5, y = 6\}, jva2 = 9\}"
+
+ gdb_test "print jva1v" \
+ " = \{<KV> = \{<A1> = \{x = 1, y = 2\}, _vptr.KV = $hex <vtable for JVA1V+.*>, i = 4\}, <LV> = \{_vptr.LV = $hex <VTT for JVA1V>, z = 3\}, jva1v = 5\}"
+}
+
+# Check that we can access all the fields correctly, using the same
+# syntax as used in the .cc file. Keep the order here in sync with
+# the .cc file.
+with_test_prefix "all fields" {
+ gdb_test "print a1.x" " = 1"
+ gdb_test "print a1.y" " = 2"
+
+ gdb_test "print a2.x" " = 1"
+ gdb_test "print a2.y" " = 2"
+
+ gdb_test "print a3.x" " = 1"
+ gdb_test "print a3.y" " = 2"
+
+ gdb_test "print x.A1::x" " = 1"
+ gdb_test "print x.A1::y" " = 2"
+ gdb_test "print x.A2::x" " = 3"
+ gdb_test "print x.A2::y" " = 4"
+ gdb_test "print x.z" " = 5"
+
+ gdb_test "print l.x" " = 1"
+ gdb_test "print l.y" " = 2"
+ gdb_test "print l.z" " = 3"
+
+ gdb_test "print m.x" " = 1"
+ gdb_test "print m.y" " = 2"
+ gdb_test "print m.w" " = 3"
+
+ gdb_test "print n.A1::x" " = 1"
+ gdb_test "print n.A1::y" " = 2"
+ gdb_test "print n.A2::x" " = 3"
+ gdb_test "print n.A2::y" " = 4"
+ gdb_test "print n.w" " = 5"
+ gdb_test "print n.r" " = 6"
+ gdb_test "print n.z" " = 7"
+
+ gdb_test "print k.x" " = 1"
+ gdb_test "print k.y" " = 2"
+ gdb_test "print k.i" " = 3"
+
+ gdb_test "print j.K::x" " = 1"
+ gdb_test "print j.K::y" " = 2"
+ gdb_test "print j.L::x" " = 3"
+ gdb_test "print j.L::y" " = 4"
+ gdb_test "print j.i" " = 5"
+ gdb_test "print j.z" " = 6"
+ gdb_test "print j.j" " = 7"
+
+ gdb_test "print jv.x" " = 1"
+ gdb_test "print jv.y" " = 2"
+ gdb_test "print jv.i" " = 3"
+ gdb_test "print jv.z" " = 4"
+ gdb_test "print jv.jv" " = 5"
+
+ setup_kfail "c++/26550" *-*-*
+ gdb_test "print jva1.KV::x" " = 1"
+ setup_kfail "c++/26550" *-*-*
+ gdb_test "print jva1.KV::y" " = 2"
+ setup_kfail "c++/26550" *-*-*
+ gdb_test "print jva1.LV::x" " = 3"
+ setup_kfail "c++/26550" *-*-*
+ gdb_test "print jva1.LV::y" " = 4"
+ gdb_test "print jva1.z" " = 5"
+ gdb_test "print jva1.i" " = 6"
+ gdb_test "print jva1.jva1" "= 7"
+
+ setup_kfail "c++/26550" *-*-*
+ gdb_test "print jva2.KV::x" " = 1"
+ setup_kfail "c++/26550" *-*-*
+ gdb_test "print jva2.KV::y" " = 2"
+ setup_kfail "c++/26550" *-*-*
+ gdb_test "print jva2.LV::x" " = 3"
+ setup_kfail "c++/26550" *-*-*
+ gdb_test "print jva2.LV::y" " = 4"
+ gdb_test "print jva2.A2::x" " = 5"
+ gdb_test "print jva2.A2::y" " = 6"
+ gdb_test "print jva2.z" " = 7"
+ gdb_test "print jva2.i" " = 8"
+ gdb_test "print jva2.jva2" "= 9"
+
+ gdb_test "print jva1v.x" " = 1"
+ gdb_test "print jva1v.y" " = 2"
+ gdb_test "print jva1v.z" " = 3"
+ gdb_test "print jva1v.i" " = 4"
+ gdb_test "print jva1v.jva1v" " = 5"
+}
+
+# Test that printing WHAT reports an error about FIELD being ambiguous
+# in TYPE, and that the candidates are CANDIDATES.
+proc test_ambiguous {what field type candidates} {
+ set msg "Request for member '$field' is ambiguous in type '$type'. Candidates are:"
+
+ foreach c $candidates {
+ set c_re [string_to_regexp $c]
+ append msg "\r\n $c_re"
}
-# print out various class objects' members. The values aren't
-# important, just check that the warning is emitted at the
-# right times.
+ gdb_test "print $what" $msg
+}
# X is derived from A1 and A2; both A1 and A2 have a member 'x'
-send_gdb "print x.x\n"
-gdb_expect {
- -re "warning: x ambiguous; using X::A2::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print x.x"
- }
- -re "warning: x ambiguous; using X::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print x.x"
- }
- -re ".*$gdb_prompt $" { fail "print x.x" }
- timeout { fail "(timeout) print x.x" }
+test_ambiguous "x.x" "x" "X" {
+ "'int A1::x' (X -> A1)"
+ "'int A2::x' (X -> A2)"
}
-
# N is derived from A1 and A2, but not immediately -- two steps
# up in the hierarchy. Both A1 and A2 have a member 'x'.
-send_gdb "print n.x\n"
-gdb_expect {
- -re "warning: x ambiguous; using N::M::A2::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print n.x"
- }
- -re "warning: x ambiguous; using N::L::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print n.x"
- }
- -re ".*$gdb_prompt $" { fail "print n.x" }
- timeout { fail "(timeout) print n.x" }
+test_ambiguous "n.x" "x" "N" {
+ "'int A1::x' (N -> L -> A1)"
+ "'int A2::x' (N -> M -> A2)"
}
-# J is derived from A1 twice. A1 has a member x.
-send_gdb "print j.x\n"
-gdb_expect {
- -re "warning: x ambiguous; using J::L::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print j.x"
- }
- -re "warning: x ambiguous; using J::K::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print j.x"
- }
- -re ".*$gdb_prompt $" { fail "print j.x" }
- timeout { fail "(timeout) print j.x" }
+# J is derived from A1 twice. A1 has a member x.
+test_ambiguous "j.x" "x" "J" {
+ "'int A1::x' (J -> K -> A1)"
+ "'int A1::x' (J -> L -> A1)"
}
# JV is derived from A1 but A1 is a virtual base. Should not
-# report an ambiguity in this case.
-send_gdb "print jv.x\n"
-gdb_expect {
- -re "warning: x ambiguous.*Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- fail "print jv.x (ambiguity reported)"
- }
- -re "\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" { pass "print jv.x" }
- -re ".*$gdb_prompt $" { fail "print jv.x (??)" }
- timeout { fail "(timeout) print jv.x" }
-}
+# report an ambiguity in this case.
+gdb_test "print jv.x" " = 1"
# JVA1 is derived from A1; A1 occurs as a virtual base in two
# ancestors, and as a non-virtual immediate base. Ambiguity must
-# be reported.
-send_gdb "print jva1.x\n"
-gdb_expect {
- -re "warning: x ambiguous; using JVA1::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print jva1.x"
- }
- -re "warning: x ambiguous; using JVA1::KV::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print jva1.x"
- }
- -re ".*$gdb_prompt $" { fail "print jva1.x" }
- timeout { fail "(timeout) print jva1.x" }
+# be reported.
+test_ambiguous "jva1.x" "x" "JVA1" {
+ "'int A1::x' (JVA1 -> KV -> A1)"
+ "'int A1::x' (JVA1 -> A1)"
}
# JVA2 is derived from A1 & A2; A1 occurs as a virtual base in two
# ancestors, and A2 is a non-virtual immediate base. Ambiguity must
# be reported as A1 and A2 both have a member 'x'.
-send_gdb "print jva2.x\n"
-gdb_expect {
- -re "warning: x ambiguous; using JVA2::A2::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print jva2.x"
- }
- -re "warning: x ambiguous; using JVA2::KV::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- pass "print jva2.x"
- }
- -re ".*$gdb_prompt $" { fail "print jva2.x" }
- timeout { fail "(timeout) print jva2.x" }
+test_ambiguous "jva2.x" "x" "JVA2" {
+ "'int A1::x' (JVA2 -> KV -> A1)"
+ "'int A2::x' (JVA2 -> A2)"
}
# JVA1V is derived from A1; A1 occurs as a virtual base in two
# ancestors, and also as a virtual immediate base. Ambiguity must
# not be reported.
-send_gdb "print jva1v.x\n"
-gdb_expect {
- -re "warning: x ambiguous.*Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" {
- fail "print jva1v.x (ambiguity reported)"
- }
- -re "\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" { pass "print jva1v.x" }
- -re ".*$gdb_prompt $" { fail "print jva1v.x (??)" }
- timeout { fail "(timeout) print jva1v.x" }
-}
+gdb_test "print jva1v.x" " = 1"
# Now check for ambiguous bases.
# J is derived from A1 twice; report ambiguity if a J is
# cast to an A1.
-send_gdb "print (A1)j\n"
-gdb_expect {
- -re "warning: A1 ambiguous; using J::L::A1. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \{x = \[-\]*\[0-9\]*, y = \[-\]*\[0-9\]*\}\r\n$gdb_prompt $" {
- pass "print (A1)j"
- }
- -re "warning: A1 ambiguous; using J::K::A1. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \{x = \[-\]*\[0-9\]*, y = \[-\]*\[0-9\]*\}\r\n$gdb_prompt $" {
- pass "print (A1)j"
- }
- -re ".*$gdb_prompt $" { fail "print (A1)j" }
- timeout { fail "(timeout) print (A1)j" }
-}
+gdb_test "print (A1)j" "base class 'A1' is ambiguous in type 'J'"
# JV is derived from A1 twice, but A1 is a virtual base; should
# not report ambiguity when a JV is cast to an A1.
-send_gdb "print (A1)jv\n"
-gdb_expect {
- -re "warning: A1 ambiguous.*Use a cast to disambiguate.\r\n\\$\[0-9\]* = \{x = \[-\]*\[0-9\]*, y = \[-\]*\[0-9\]*\}\r\n$gdb_prompt $" {
- fail "print (A1)jv (ambiguity reported)"
- }
- -re "\\$\[0-9\]* = \{x = \[-\]*\[0-9\]*, y = \[-\]*\[0-9\]*\}\r\n$gdb_prompt $" { pass "print (A1)jv" }
- -re ".*$gdb_prompt $" { fail "print (A1)jv (??)" }
- timeout { fail "(timeout) print (A1)jv" }
-}
+gdb_test "print (A1)jv" " = {x = 1, y = 2}"
# JVA1 is derived from A1; A1 is a virtual base and also a
# non-virtual base. Must report ambiguity if a JVA1 is cast to an A1.
-send_gdb "print (A1)jva1\n"
-gdb_expect {
- -re "warning: A1 ambiguous; using JVA1::A1. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \{x = \[-\]*\[0-9\]*, y = \[-\]*\[0-9\]*\}\r\n$gdb_prompt $" {
- pass "print (A1)jva1"
- }
- -re "warning: A1 ambiguous; using JVA1::KV::A1. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \{x = \[-\]*\[0-9\]*, y = \[-\]*\[0-9\]*\}\r\n$gdb_prompt $" {
- pass "print (A1)jva1"
- }
- -re ".*$gdb_prompt $" { fail "print (A1)jva1" }
- timeout { fail "(timeout) print (A1)jva1" }
-}
+gdb_test "print (A1)jva1" "base class 'A1' is ambiguous in type 'JVA1'"
+
+# Add an intermediate cast to KV, and it should work.
+setup_kfail "c++/26550" *-*-*
+gdb_test "print (KV)jva1" " = \{<A1> = \{x = 3, y = 4\}, _vptr.KV = $hex <VTT for KV>, i = 6\}"
+setup_kfail "c++/26550" *-*-*
+gdb_test "print (A1)(KV)jva1" " = \{x = 3, y = 4\}"
# JVA1V is derived from A1; A1 is a virtual base indirectly
# and also directly; must not report ambiguity when a JVA1V is cast to an A1.
-send_gdb "print (A1)jva1v\n"
-gdb_expect {
- -re "warning: A1 ambiguous.*Use a cast to disambiguate.\r\n\\$\[0-9\]* = \{x = \[-\]*\[0-9\]*, y = \[-\]*\[0-9\]*\}\r\n$gdb_prompt $" {
- fail "print (A1)jva1v (ambiguity reported)"
- }
- -re "\\$\[0-9\]* = \{x = \[-\]*\[0-9\]*, y = \[-\]*\[0-9\]*\}\r\n$gdb_prompt $" { pass "print (A1)jva1v"
- }
- -re ".*$gdb_prompt $" { fail "print (A1)jva1v (??)" }
- timeout { fail "(timeout) print (A1)jva1v" }
-}
-
+gdb_test "print (A1)jva1v" " = {x = 1, y = 2}"
diff --git a/gdb/valops.c b/gdb/valops.c
index 0eb2b096211..c9c11ed8650 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -1766,54 +1766,132 @@ typecmp (int staticp, int varargs, int nargs,
return i + 1;
}
-/* Helper class for do_search_struct_field that updates *RESULT_PTR
- and *LAST_BOFFSET, and possibly throws an exception if the field
- search has yielded ambiguous results. */
+/* Helper class for search_struct_field that keeps track of found
+ results and possibly throws an exception if the search yields
+ ambiguous results. See search_struct_field for description of
+ LOOKING_FOR_BASECLASS. */
-static void
-update_search_result (struct value **result_ptr, struct value *v,
- LONGEST *last_boffset, LONGEST boffset,
- const char *name, struct type *type)
+struct struct_field_searcher
+{
+ /* A found field. */
+ struct found_field
+ {
+ /* Path to the structure where the field was found. */
+ std::vector<struct type *> path;
+
+ /* The field found. */
+ struct value *field_value;
+ };
+
+ /* See corresponding fields for description of parameters. */
+ struct_field_searcher (const char *name,
+ struct type *outermost_type,
+ bool looking_for_baseclass)
+ : m_name (name),
+ m_looking_for_baseclass (looking_for_baseclass),
+ m_outermost_type (outermost_type)
+ {
+ }
+
+ /* The search entry point. If LOOKING_FOR_BASECLASS is true and the
+ base class search yields ambiguous results, this throws an
+ exception. If LOOKING_FOR_BASECLASS is false, the found fields
+ are accumulated and the caller (search_struct_field) takes care
+ of throwing an error if the field search yields ambiguous
+ results. The latter is done that way so that the error message
+ can include a list of all the found candidates. */
+ void search (struct value *arg, LONGEST offset, struct type *type);
+
+ const std::vector<found_field> &fields ()
+ {
+ return m_fields;
+ }
+
+ struct value *baseclass ()
+ {
+ return m_baseclass;
+ }
+
+private:
+ /* Update results to include V, a found field/baseclass. */
+ void update_result (struct value *v, LONGEST boffset);
+
+ /* The name of the field/baseclass we're searching for. */
+ const char *m_name;
+
+ /* Whether we're looking for a baseclass, or a field. */
+ const bool m_looking_for_baseclass;
+
+ /* The offset of the baseclass containing the field/baseclass we
+ last recorded. */
+ LONGEST m_last_boffset = 0;
+
+ /* If looking for a baseclass, then the result is stored here. */
+ struct value *m_baseclass = nullptr;
+
+ /* When looking for fields, the found candidates are stored
+ here. */
+ std::vector<found_field> m_fields;
+
+ /* The type of the initial type passed to search_struct_field; this
+ is used for error reporting when the lookup is ambiguous. */
+ struct type *m_outermost_type;
+
+ /* The full path to the struct being inspected. E.g. for field 'x'
+ defined in class B inherited by class A, we have A and B pushed
+ on the path. */
+ std::vector <struct type *> m_struct_path;
+};
+
+void
+struct_field_searcher::update_result (struct value *v, LONGEST boffset)
{
if (v != NULL)
{
- if (*result_ptr != NULL
- /* The result is not ambiguous if all the classes that are
- found occupy the same space. */
- && *last_boffset != boffset)
- error (_("base class '%s' is ambiguous in type '%s'"),
- name, TYPE_SAFE_NAME (type));
- *result_ptr = v;
- *last_boffset = boffset;
+ if (m_looking_for_baseclass)
+ {
+ if (m_baseclass != nullptr
+ /* The result is not ambiguous if all the classes that are
+ found occupy the same space. */
+ && m_last_boffset != boffset)
+ error (_("base class '%s' is ambiguous in type '%s'"),
+ m_name, TYPE_SAFE_NAME (m_outermost_type));
+
+ m_baseclass = v;
+ m_last_boffset = boffset;
+ }
+ else
+ {
+ /* The field is not ambiguous if it occupies the same
+ space. */
+ if (m_fields.empty () || m_last_boffset != boffset)
+ m_fields.push_back ({m_struct_path, v});
+ }
}
}
/* A helper for search_struct_field. This does all the work; most
- arguments are as passed to search_struct_field. The result is
- stored in *RESULT_PTR, which must be initialized to NULL.
- OUTERMOST_TYPE is the type of the initial type passed to
- search_struct_field; this is used for error reporting when the
- lookup is ambiguous. */
+ arguments are as passed to search_struct_field. */
-static void
-do_search_struct_field (const char *name, struct value *arg1, LONGEST offset,
- struct type *type, int looking_for_baseclass,
- struct value **result_ptr,
- LONGEST *last_boffset,
- struct type *outermost_type)
+void
+struct_field_searcher::search (struct value *arg1, LONGEST offset,
+ struct type *type)
{
int i;
int nbases;
+ m_struct_path.push_back (type);
+ SCOPE_EXIT { m_struct_path.pop_back (); };
+
type = check_typedef (type);
nbases = TYPE_N_BASECLASSES (type);
- if (!looking_for_baseclass)
+ if (!m_looking_for_baseclass)
for (i = type->num_fields () - 1; i >= nbases; i--)
{
const char *t_field_name = TYPE_FIELD_NAME (type, i);
- if (t_field_name && (strcmp_iw (t_field_name, name) == 0))
+ if (t_field_name && (strcmp_iw (t_field_name, m_name) == 0))
{
struct value *v;
@@ -1821,7 +1899,8 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset,
v = value_static_field (type, i);
else
v = value_primitive_field (arg1, offset, i, type);
- *result_ptr = v;
+
+ update_result (v, offset);
return;
}
@@ -1845,7 +1924,6 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset,
represented as a struct, with a member for each
<variant field>. */
- struct value *v = NULL;
LONGEST new_offset = offset;
/* This is pretty gross. In G++, the offset in an
@@ -1859,16 +1937,7 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset,
&& TYPE_FIELD_BITPOS (field_type, 0) == 0))
new_offset += TYPE_FIELD_BITPOS (type, i) / 8;
- do_search_struct_field (name, arg1, new_offset,
- field_type,
- looking_for_baseclass, &v,
- last_boffset,
- outermost_type);
- if (v)
- {
- *result_ptr = v;
- return;
- }
+ search (arg1, new_offset, field_type);
}
}
}
@@ -1880,10 +1949,10 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset,
/* If we are looking for baseclasses, this is what we get when
we hit them. But it could happen that the base part's member
name is not yet filled in. */
- int found_baseclass = (looking_for_baseclass
+ int found_baseclass = (m_looking_for_baseclass
&& TYPE_BASECLASS_NAME (type, i) != NULL
- && (strcmp_iw (name,
- TYPE_BASECLASS_NAME (type,
+ && (strcmp_iw (m_name,
+ TYPE_BASECLASS_NAME (type,
i)) == 0));
LONGEST boffset = value_embedded_offset (arg1) + offset;
@@ -1924,28 +1993,17 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset,
if (found_baseclass)
v = v2;
else
- {
- do_search_struct_field (name, v2, 0,
- TYPE_BASECLASS (type, i),
- looking_for_baseclass,
- result_ptr, last_boffset,
- outermost_type);
- }
+ search (v2, 0, TYPE_BASECLASS (type, i));
}
else if (found_baseclass)
v = value_primitive_field (arg1, offset, i, type);
else
{
- do_search_struct_field (name, arg1,
- offset + TYPE_BASECLASS_BITPOS (type,
- i) / 8,
- basetype, looking_for_baseclass,
- result_ptr, last_boffset,
- outermost_type);
+ search (arg1, offset + TYPE_BASECLASS_BITPOS (type, i) / 8,
+ basetype);
}
- update_search_result (result_ptr, v, last_boffset,
- boffset, name, outermost_type);
+ update_result (v, boffset);
}
}
@@ -1960,12 +2018,55 @@ static struct value *
search_struct_field (const char *name, struct value *arg1,
struct type *type, int looking_for_baseclass)
{
- struct value *result = NULL;
- LONGEST boffset = 0;
+ struct_field_searcher searcher (name, type, looking_for_baseclass);
- do_search_struct_field (name, arg1, 0, type, looking_for_baseclass,
- &result, &boffset, type);
- return result;
+ searcher.search (arg1, 0, type);
+
+ if (!looking_for_baseclass)
+ {
+ const auto &fields = searcher.fields ();
+
+ if (fields.empty ())
+ return nullptr;
+ else if (fields.size () == 1)
+ return fields[0].field_value;
+ else
+ {
+ std::string candidates;
+
+ for (auto &&candidate : fields)
+ {
+ gdb_assert (!candidate.path.empty ());
+
+ struct type *field_type = value_type (candidate.field_value);
+ struct type *struct_type = candidate.path.back ();
+
+ std::string path;
+ bool first = true;
+ for (struct type *t : candidate.path)
+ {
+ if (first)
+ first = false;
+ else
+ path += " -> ";
+ path += t->name ();
+ }
+
+ candidates += string_printf ("\n '%s %s::%s' (%s)",
+ TYPE_SAFE_NAME (field_type),
+ TYPE_SAFE_NAME (struct_type),
+ name,
+ path.c_str ());
+ }
+
+ error (_("Request for member '%s' is ambiguous in type '%s'."
+ " Candidates are:%s"),
+ name, TYPE_SAFE_NAME (type),
+ candidates.c_str ());
+ }
+ }
+ else
+ return searcher.baseclass ();
}
/* Helper function used by value_struct_elt to recurse through
base-commit: 8f57f343104b8d3632e65ac1fbb12ee69891ef5f
--
2.14.5
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] Reject ambiguous C++ field accesses
2020-08-28 20:22 ` [PATCH v2] " Pedro Alves
@ 2020-10-12 17:12 ` Pedro Alves
2020-10-13 8:47 ` Gary Benson
0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2020-10-12 17:12 UTC (permalink / raw)
To: Gary Benson, Luis Machado; +Cc: gdb-patches
On 8/28/20 9:22 PM, Pedro Alves wrote:
> gdb/ChangeLog:
>
> * valops.c (struct struct_field_searcher): New.
> (update_search_result): Rename to ...
> (struct_field_searcher::update_result): ... this. Simplify
> prototype. Record all found fields.
> (do_search_struct_field): Rename to ...
> (struct_field_searcher::search): ... this. Simplify prototype.
> Maintain stack of visited baseclass path. Call update_result for
> fields too. Keep searching fields in baseclasses instead of
> stopping at the first found field.
> (search_struct_field): Use struct_field_searcher. When looking
> for fields, report ambiguous access attempts.
>
> gdb/testsuite/ChangeLog:
>
> PR c++/26550
> * gdb.cp/ambiguous.cc (marker1): Delete.
> (main): Initialize all the fields of the locals. Replace marker1
> call with a "set breakpoint here" marker.
> * gdb.cp/ambiguous.exp: Modernize. Use gdb_continue_to_breakpoint
> instead of running to marker1. Add tests printing all the
> variables and all the fields of the variables.
> (test_ambiguous): New proc, expecting the new GDB output when a
> field access is ambiguous. Change all "warning: X ambiguous"
> tests to use it.
I've merged this now.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] Reject ambiguous C++ field accesses
2020-10-12 17:12 ` Pedro Alves
@ 2020-10-13 8:47 ` Gary Benson
0 siblings, 0 replies; 10+ messages in thread
From: Gary Benson @ 2020-10-13 8:47 UTC (permalink / raw)
To: Pedro Alves; +Cc: Luis Machado, gdb-patches
Pedro Alves wrote:
> On 8/28/20 9:22 PM, Pedro Alves wrote:
> > gdb/ChangeLog:
> >
> > * valops.c (struct struct_field_searcher): New.
> > (update_search_result): Rename to ...
> > (struct_field_searcher::update_result): ... this. Simplify
> > prototype. Record all found fields.
> > (do_search_struct_field): Rename to ...
> > (struct_field_searcher::search): ... this. Simplify prototype.
> > Maintain stack of visited baseclass path. Call update_result for
> > fields too. Keep searching fields in baseclasses instead of
> > stopping at the first found field.
> > (search_struct_field): Use struct_field_searcher. When looking
> > for fields, report ambiguous access attempts.
> >
> > gdb/testsuite/ChangeLog:
> >
> > PR c++/26550
> > * gdb.cp/ambiguous.cc (marker1): Delete.
> > (main): Initialize all the fields of the locals. Replace marker1
> > call with a "set breakpoint here" marker.
> > * gdb.cp/ambiguous.exp: Modernize. Use gdb_continue_to_breakpoint
> > instead of running to marker1. Add tests printing all the
> > variables and all the fields of the variables.
> > (test_ambiguous): New proc, expecting the new GDB output when a
> > field access is ambiguous. Change all "warning: X ambiguous"
> > tests to use it.
>
> I've merged this now.
Thanks Pedro.
Cheers,
Gary
--
Gary Benson - he / him / his
Principal Software Engineer, Red Hat
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-10-13 8:47 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-27 18:02 [PATCH] Reject ambiguous C++ field accesses Pedro Alves
2020-08-27 18:45 ` Luis Machado
2020-08-27 19:12 ` Pedro Alves
2020-08-27 19:58 ` Luis Machado
2020-08-28 14:35 ` Gary Benson
2020-08-28 20:22 ` [PATCH v2] " Pedro Alves
2020-10-12 17:12 ` Pedro Alves
2020-10-13 8:47 ` Gary Benson
2020-08-28 20:12 ` [PATCH] " Pedro Alves
2020-08-28 14:38 ` Gary Benson
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).