This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch 2/2] Fix overload resolution of int* vs void*
On 10/08/2010 06:54 PM, Tom Tromey wrote:
"Sami" == sami wagiaalla<swagiaal@redhat.com> writes:
Sami> @@ -1886,6 +1886,14 @@ do_is_ancestor (struct type *base, struct type *dclass, int public)
Sami> if (class_types_same_p (base, dclass))
Sami> return 1;
Sami> + if ((TYPE_CODE (base) == TYPE_CODE_PTR
Sami> +&& TYPE_CODE (dclass) == TYPE_CODE_PTR)
Sami> + || (TYPE_CODE (base) == TYPE_CODE_REF
Sami> + && TYPE_CODE (dclass) == TYPE_CODE_REF))
Sami> + return do_is_ancestor (TYPE_TARGET_TYPE (base),
Sami> + TYPE_TARGET_TYPE (dclass),
Sami> + public);
This seems strange to me. It ought to only be applicable at the
outermost call. But in that case, I think a wrapper function would be
more appropriate, so that do_is_ancestor has a single purpose.
Also, is this the right thing for the caller when there are multiple
pointers (like "Class ***")? Or do you intend to only strip a single
pointer?
I understood that one could convert 'Class ***' to 'BaseClass ***', but
it turns out that is in correct. I should not have assumed that without
testing, and there is nothing in the spec that should have made me think
so. Since that is the case, the calling function can just deference the
pointers.
Sami> + /* Resolve typedefs */
Sami> + if (TYPE_CODE (a) == TYPE_CODE_TYPEDEF)
Sami> + a = check_typedef (a);
Sami> + if (TYPE_CODE (b) == TYPE_CODE_TYPEDEF)
Sami> + b = check_typedef (b);
It is ok to call check_typedef unconditionally, but I see this is just
copied from rank_one_type.
Yeah I wondered about that when I copied the code but could not think of
a reason not to do so.
I added a test case for the typedef scenario.
Sami> + /* (b) pointer to ancestor-pointer conversion. */
Sami> + if (is_public_ancestor (parm, arg))
Sami> + return BASE_PTR_CONVERSION_BADNESS;
I am curious why you didn't just give POINTER_CONVERSION_BADNESS a new
value and instead introduced BASE_PTR_CONVERSION_BADNESS.
But then, I also don't understand the existing code that returns
POINTER_CONVERSION_BADNESS...
base pointer conversion (BASE_PTR_CONVERSION_BADNESS) is meant to be a
slightly better option than generic (POINTER_CONVERSION_BADNESS)
Also, why specifically is_public_ancestor and not is_ancestor?
You can convert a pointer to B to a pointer to A only if A is an
accessible ancestor of B.
I suppose this is an incremental patch -- still not truly correct with
regards to the distance-to-base problem, but an improvement over what
gdb currently does.
Yes. I am working on a second patch to solve the distance to base
problem. I will post it in reply to my previous attempt at that problem.
This is fine, even great, but it is handy for
reviewers if you note that sort of thing in the patch explanation.
Cool, will do in the future. Incidentally, I should mention that is
there is also the conversion of pointer to other data types and ranking
within them that needs to be covered.
Sami> case TYPE_CODE_ARRAY:
Sami> - return rank_one_type (TYPE_TARGET_TYPE (parm),
Sami> - TYPE_TARGET_TYPE (arg));
Sami> + if (types_equal (TYPE_TARGET_TYPE (parm),
Sami> + TYPE_TARGET_TYPE (arg)))
Sami> + return 0;
Sami> + return INCOMPATIBLE_TYPE_BADNESS;
Do the new tests cover this case? I could not immediately tell.
Yes, take a look at foo1_2 foo2_2, and foo2_4. Let me know if the tests
need elaborations it is easy to miss cases when you are so close to the
code.
Revised patch attached.
Fixed void* vs int* overload issue (PR C++/10343).
2010-10-12 Sami Wagiaalla <swagiaal@redhat.com>
* gdbtypes.h: Create BASE_PTR_CONVERSION_BADNESS.
* gdbtypes.c (rank_one_type): Move type comparison code out of here
to...
(types_equal): ...here. And changed it as follows:
Outside of typedefs type must be of the same TYPE_CODE.
When compairing two pointers or references they are equal if their
targets are equal.
Correct pointer conversions.
2010-10-12 Sami Wagiaalla <swagiaal@redhat.com>
* gdb.cp/converts.cc: New test program.
* gdb.cp/converts.exp: New test.
* gdb.cp/overload.exp: Added test for void* vs int*.
* gdb.cp/overload.exp: Ditto.
* gdb.cp/oranking.exp: Removed related kfail.
* gdb.cp/virtfunc2.cc (Obj2): Change inheritance to public.
Add calls to the function calls being tested.
* gdb.cp/derivation.cc (G): Changed inheritance to public.
Add calls to the function calls being tested.
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 9c3152d..5af7491 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2104,6 +2104,56 @@ integer_types_same_name_p (const char *first, const char *second)
return 1;
}
+/* Compares type A to type B returns 1 if the represent the same type
+ 0 otherwise. */
+
+static int
+types_equal (struct type *a, struct type *b)
+{
+ /* Identical type pointers. */
+ /* However, this still doesn't catch all cases of same type for b
+ and a. The reason is that builtin types are different from
+ the same ones constructed from the object. */
+ if (a == b)
+ return 1;
+
+ /* Resolve typedefs */
+ if (TYPE_CODE (a) == TYPE_CODE_TYPEDEF)
+ a = check_typedef (a);
+ if (TYPE_CODE (b) == TYPE_CODE_TYPEDEF)
+ b = check_typedef (b);
+
+ /* If after resolving typedefs a and b are not of the same type
+ code then they are not equal. */
+ if (TYPE_CODE (a) != TYPE_CODE (b))
+ return 0;
+
+ /* If a and b are both pointers types or both reference types then
+ they are equal of the same type iff the objects they refer to are
+ of the same type. */
+ if (TYPE_CODE (a) == TYPE_CODE_PTR
+ || TYPE_CODE (a) == TYPE_CODE_REF)
+ return types_equal (TYPE_TARGET_TYPE (a),
+ TYPE_TARGET_TYPE (b));
+
+ /*
+ Well, damnit, if the names are exactly the same, I'll say they
+ are exactly the same. This happens when we generate method
+ stubs. The types won't point to the same address, but they
+ really are the same.
+ */
+
+ if (TYPE_NAME (a) && TYPE_NAME (b)
+ && strcmp (TYPE_NAME (a), TYPE_NAME (b)) == 0)
+ return 1;
+
+ /* Check if identical after resolving typedefs. */
+ if (a == b)
+ return 1;
+
+ return 0;
+}
+
/* Compare one type (PARM) for compatibility with another (ARG).
* PARM is intended to be the parameter type of a function; and
* ARG is the supplied argument's type. This function tests if
@@ -2117,11 +2167,8 @@ integer_types_same_name_p (const char *first, const char *second)
int
rank_one_type (struct type *parm, struct type *arg)
{
- /* Identical type pointers. */
- /* However, this still doesn't catch all cases of same type for arg
- and param. The reason is that builtin types are different from
- the same ones constructed from the object. */
- if (parm == arg)
+
+ if (types_equal (parm, arg))
return 0;
/* Resolve typedefs */
@@ -2130,21 +2177,6 @@ rank_one_type (struct type *parm, struct type *arg)
if (TYPE_CODE (arg) == TYPE_CODE_TYPEDEF)
arg = check_typedef (arg);
- /*
- Well, damnit, if the names are exactly the same, I'll say they
- are exactly the same. This happens when we generate method
- stubs. The types won't point to the same address, but they
- really are the same.
- */
-
- if (TYPE_NAME (parm) && TYPE_NAME (arg)
- && !strcmp (TYPE_NAME (parm), TYPE_NAME (arg)))
- return 0;
-
- /* Check if identical after resolving typedefs. */
- if (parm == arg)
- return 0;
-
/* See through references, since we can almost make non-references
references. */
if (TYPE_CODE (arg) == TYPE_CODE_REF)
@@ -2168,15 +2200,23 @@ rank_one_type (struct type *parm, struct type *arg)
switch (TYPE_CODE (arg))
{
case TYPE_CODE_PTR:
- if (TYPE_CODE (TYPE_TARGET_TYPE (parm)) == TYPE_CODE_VOID
- && TYPE_CODE (TYPE_TARGET_TYPE (arg)) != TYPE_CODE_VOID)
+
+ /* Allowed pointer conversions are:
+ (a) pointer to void-pointer conversion. */
+ if (TYPE_CODE (TYPE_TARGET_TYPE (parm)) == TYPE_CODE_VOID)
return VOID_PTR_CONVERSION_BADNESS;
- else
- return rank_one_type (TYPE_TARGET_TYPE (parm),
- TYPE_TARGET_TYPE (arg));
+
+ /* (b) pointer to ancestor-pointer conversion. */
+ if (is_public_ancestor (TYPE_TARGET_TYPE (parm),
+ TYPE_TARGET_TYPE (arg)))
+ return BASE_PTR_CONVERSION_BADNESS;
+
+ return INCOMPATIBLE_TYPE_BADNESS;
case TYPE_CODE_ARRAY:
- return rank_one_type (TYPE_TARGET_TYPE (parm),
- TYPE_TARGET_TYPE (arg));
+ if (types_equal (TYPE_TARGET_TYPE (parm),
+ TYPE_TARGET_TYPE (arg)))
+ return 0;
+ return INCOMPATIBLE_TYPE_BADNESS;
case TYPE_CODE_FUNC:
return rank_one_type (TYPE_TARGET_TYPE (parm), arg);
case TYPE_CODE_INT:
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index dba7284..62ade5f 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1400,6 +1400,9 @@ extern int is_unique_ancestor (struct type *, struct value *);
#define INTEGER_PROMOTION_BADNESS 1
/* Badness of floating promotion */
#define FLOAT_PROMOTION_BADNESS 1
+/* Badness of converting a derived class pointer
+ to a base class pointer. */
+#define BASE_PTR_CONVERSION_BADNESS 1
/* Badness of integral conversion */
#define INTEGER_CONVERSION_BADNESS 2
/* Badness of floating conversion */
diff --git a/gdb/testsuite/gdb.cp/converts.cc b/gdb/testsuite/gdb.cp/converts.cc
new file mode 100644
index 0000000..435ed13
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/converts.cc
@@ -0,0 +1,49 @@
+class A {};
+class B : public A {};
+
+typedef A TA1;
+typedef A TA2;
+typedef TA2 TA3;
+
+int foo0_1 (TA1) { return 1; }
+int foo0_2 (TA3) { return 2; }
+
+int foo1_1 (char *) {return 11;}
+int foo1_2 (char[]) {return 12;}
+int foo1_3 (int*) {return 13;}
+int foo1_4 (A*) {return 14;}
+int foo1_5 (void*) {return 15;}
+int foo1_6 (void**) {return 15;}
+
+int foo2_1 (char** ) {return 21;}
+int foo2_2 (char[][1]) {return 22;}
+int foo2_3 (char *[]) {return 23;}
+int foo2_4 (int *[]) {return 24;}
+
+int main()
+{
+
+ TA2 ta; // typedef to..
+ foo0_1 (ta); // ..another typedef
+ foo0_2 (ta); // ..typedef of a typedef
+
+ char *a; // pointer to..
+ B *bp;
+ foo1_1 (a); // ..pointer
+ foo1_2 (a); // ..array
+ foo1_3 ((int*)a); // ..pointer of wrong type
+ foo1_3 ((int*)bp); // ..pointer of wrong type
+ foo1_4 (bp); // ..ancestor pointer
+ foo1_4 (bp); // ..ancestor pointer
+ foo1_5 (bp); // ..void pointer
+ foo1_6 ((void**)bp); // ..void pointer
+
+ char **b; // pointer pointer to..
+ char ba[1][1];
+ foo1_5 (b); // ..void pointer
+ foo2_1 (b); // ..pointer pointer
+ foo2_2 (ba); // ..array of arrays
+ foo2_3 (b); // ..array of pointers
+ foo2_4 ((int**)b); // ..array of wrong pointers
+ return 0; // end of main
+}
diff --git a/gdb/testsuite/gdb.cp/converts.exp b/gdb/testsuite/gdb.cp/converts.exp
new file mode 100644
index 0000000..894f1cc
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/converts.exp
@@ -0,0 +1,46 @@
+# Copyright 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+set testfile converts
+set srcfile ${testfile}.cc
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug c++}] } {
+ return -1
+}
+
+############################################
+
+if ![runto_main] then {
+ perror "couldn't run to breakpoint main"
+ continue
+}
+
+gdb_breakpoint [gdb_get_line_number "end of main"]
+gdb_continue_to_breakpoint "end of main"
+
+gdb_test "p foo0_1 (ta)" "= 1"
+gdb_test "p foo0_2 (ta)" "= 2"
+
+gdb_test "p foo1_1 (a)" "= 11"
+gdb_test "p foo1_2 (a)" "= 12"
+gdb_test "p foo1_3 (a)" "Cannot resolve function.*"
+gdb_test "p foo1_3 (bp)" "Cannot resolve function.*"
+gdb_test "p foo1_4 (bp)" "= 14"
+gdb_test "p foo1_5 (bp)" "= 15"
+
+gdb_test "p foo1_5 (b)" "= 15"
+gdb_test "p foo2_1 (b)" "= 21"
+gdb_test "p foo2_2 (b)" "Cannot resolve function.*"
+gdb_test "p foo2_3 (b)" "= 23"
+gdb_test "p foo2_4 (b)" "Cannot resolve function.*"
diff --git a/gdb/testsuite/gdb.cp/derivation.cc b/gdb/testsuite/gdb.cp/derivation.cc
index f6d42e7..02999c1 100644
--- a/gdb/testsuite/gdb.cp/derivation.cc
+++ b/gdb/testsuite/gdb.cp/derivation.cc
@@ -96,7 +96,7 @@ public:
};
-class G : private A, public B, protected C {
+class G : public A, public B, public C {
public:
int g;
int gg;
@@ -207,7 +207,10 @@ int main(void)
E e_instance;
F f_instance;
G g_instance;
-
+
+ g_instance.afoo();
+ g_instance.cfoo();
+
#ifdef usestubs
set_debug_traps();
breakpoint();
diff --git a/gdb/testsuite/gdb.cp/oranking.exp b/gdb/testsuite/gdb.cp/oranking.exp
index abe8252..f06933a 100644
--- a/gdb/testsuite/gdb.cp/oranking.exp
+++ b/gdb/testsuite/gdb.cp/oranking.exp
@@ -56,7 +56,6 @@ setup_kfail "gdb/12098" *-*-*
gdb_test "p foo5(c)" "26"
gdb_test "p test6()" "28"
-setup_kfail "gdb/10343" *-*-*
gdb_test "p foo6(bp)" "28"
gdb_test "p test7()" "210"
diff --git a/gdb/testsuite/gdb.cp/overload.cc b/gdb/testsuite/gdb.cp/overload.cc
index 78fae14..dc117fb 100644
--- a/gdb/testsuite/gdb.cp/overload.cc
+++ b/gdb/testsuite/gdb.cp/overload.cc
@@ -24,6 +24,9 @@ int overload1arg (unsigned long);
int overload1arg (float);
int overload1arg (double);
+int overload1arg (int*);
+int overload1arg (void*);
+
int overloadfnarg (void);
int overloadfnarg (int);
int overloadfnarg (int, int (*) (int));
@@ -99,6 +102,8 @@ int main ()
unsigned long arg10 =10;
float arg11 =100.0;
double arg12 = 200.0;
+ int arg13 = 200.0;
+ char arg14 = 'a';
char *str = (char *) "A";
foo foo_instance1(111);
@@ -150,6 +155,8 @@ int foo::overload1arg (long arg) { arg = 0; return 9;}
int foo::overload1arg (unsigned long arg) { arg = 0; return 10;}
int foo::overload1arg (float arg) { arg = 0; return 11;}
int foo::overload1arg (double arg) { arg = 0; return 12;}
+int foo::overload1arg (int* arg) { arg = 0; return 13;}
+int foo::overload1arg (void* arg) { arg = 0; return 14;}
/* Test to see that we can explicitly request overloaded functions
with function pointers in the prototype. */
diff --git a/gdb/testsuite/gdb.cp/overload.exp b/gdb/testsuite/gdb.cp/overload.exp
index f05cc23..25aeb07 100644
--- a/gdb/testsuite/gdb.cp/overload.exp
+++ b/gdb/testsuite/gdb.cp/overload.exp
@@ -80,6 +80,8 @@ set re_methods "${re_methods}${ws}int overload1arg\\(long( int)?\\);"
set re_methods "${re_methods}${ws}int overload1arg\\((unsigned long|long unsigned)( int)?\\);"
set re_methods "${re_methods}${ws}int overload1arg\\(float\\);"
set re_methods "${re_methods}${ws}int overload1arg\\(double\\);"
+set re_methods "${re_methods}${ws}int overload1arg\\(int \\*\\);"
+set re_methods "${re_methods}${ws}int overload1arg\\(void \\*\\);"
set re_methods "${re_methods}${ws}int overloadfnarg\\((void|)\\);"
set re_methods "${re_methods}${ws}int overloadfnarg\\(int\\);"
set re_methods "${re_methods}${ws}int overloadfnarg\\(int, int ?\\(\\*\\) ?\\(int\\)\\);"
@@ -256,6 +258,14 @@ gdb_test "print foo_instance1.overload1arg((double)arg12)" \
"\\$\[0-9\]+ = 12" \
"print call overloaded func double arg"
+gdb_test "print foo_instance1.overload1arg(&arg13)" \
+ "\\$\[0-9\]+ = 13" \
+ "print call overloaded func int\\* arg"
+
+gdb_test "print foo_instance1.overload1arg(&arg14)" \
+ "\\$\[0-9\]+ = 14" \
+ "print call overloaded func char\\* arg"
+
# ---
# List overloaded functions.
diff --git a/gdb/testsuite/gdb.cp/virtfunc2.cc b/gdb/testsuite/gdb.cp/virtfunc2.cc
index 90f3eda..666a495 100644
--- a/gdb/testsuite/gdb.cp/virtfunc2.cc
+++ b/gdb/testsuite/gdb.cp/virtfunc2.cc
@@ -27,7 +27,7 @@ public:
virtual int do_print() { return 123456; }
};
-class Obj2 : Obj, virtual public interface
+class Obj2 : public Obj, virtual public interface
{
virtual int do_print2() { return 654321; }
};
@@ -35,5 +35,7 @@ class Obj2 : Obj, virtual public interface
int main(int argc, char** argv) {
Obj o;
Obj2 o2;
+ o2.do_print();
+
return 0; // marker 1
}