Siva Chandra writes:
> Ping.
>
> On Wed, Jul 16, 2014 at 6:34 AM, Siva Chandra <sivachandra@google.com> wrote:
> > Ping.
> >
> > On Wed, Jul 9, 2014 at 11:52 AM, Siva Chandra <sivachandra@google.com> wrote:
> >> The bug with examples where it manifests is detailed on the bug page.
> >> To summarize here:
> >>
> >> When expressions involving calls to virtual methods are evaluated
> >> under EVAL_AVOID_SIDE_EFFECTS, the intermediate objects could be
> >> zeroed values. In which case, when find_overload_match tries to
> >> (indirectly) lookup the method for the most derived type of the object
> >> via its vptr, GDB trips with "Cannot access memory at address 0x0."
> >> This patch adds an additional argument NOSIDE to find_overload_match
> >> which serves as a flag to indicate whether method resolution should be
> >> performed by looking up the most derived object via the vptr. When
> >> NOSIDE is EVAL_AVOID_SIDE_EFFECTS, the method for the compile time
> >> type (instead of the most derived type) of the object is returned by
> >> find_overload_match. This will not affect the functionality as all
> >> that is important when evaluating under EVAL_AVOID_SIDE_EFFECTS is the
> >> return type of the method.
> >>
> >> Regression tested testsuite/gdb.cp. But, am I simplifying the problem somewhere?
> >>
> >> ChangeLog
> >> 2014-07-09 Siva Chandra Reddy <sivachandra@google.com>
> >>
> >> gdb/
> >>
> >> * eval.c: Update all calls to find_overload_match.
> >> * valarith.c: Update calls to find_overload_match.
> >> (value_user_defined_cpp_op, value_user_defined_op): New
> >> argument NOSIDE. Update all callers.
> >> * valops.c (find_overload_match): New argument NOSIDE.
> >> * value.h (find_overload_match): Update signature.
> >>
> >> gdb/testsuite
> >>
> >> * gdb.cp/pr17132.cc: New file.
> >> * gdb.cp/pr17132.exp: New file.
Hi.
I'm not confident I know c++ well enough to say for certain that there
isn't a problem somewhere, but I'm ok with the patch, modulo a few
comment changes.
@@ -2767,9 +2771,20 @@ find_overload_match (struct value **args, int nargs,
{
if (src_method_oload_champ >= 0)
{
- if (TYPE_FN_FIELD_VIRTUAL_P (fns_ptr, method_oload_champ))
- *valp = value_virtual_fn_field (&temp, fns_ptr, method_oload_champ,
- basetype, boffset);
+ /* If the best matching method is virtual, then picking the correct
+ implementation of the method will require reading the vtable of
+ the object via the vtable pointer. Hence, do not do this if
+ NOSIDE is EVAL_AVOID_SIDE_EFFECTS. It will not lead to any
+ errors because of this as all that is of interest when evaluating
+ expressions under EVAL_AVOID_SIDE_EFFECTS is the return type of
+ the method. */
+ if (TYPE_FN_FIELD_VIRTUAL_P (fns_ptr, method_oload_champ)
+ && noside != EVAL_AVOID_SIDE_EFFECTS)
+ {
+ *valp = value_virtual_fn_field (&temp, fns_ptr,
+ method_oload_champ, basetype,
+ boffset);
+ }
else
*valp = value_fn_field (&temp, fns_ptr, method_oload_champ,
basetype, boffset);
It doesn't feel right to encode in find_overload_match any assumptions
about why it might be called with EVAL_AVOID_SIDE_EFFECTS. At least
not explicitly. Instead it should say what it does when passed
EVAL_AVOID_SIDE_EFFECTS and let the caller decide.
Thus I would change this comment:
+ If NOSIDE is EVAL_AVOID_SIDE_EFFECTS, then OBJP's memory will not be
+ read while picking the best overload match.
to this (or something like it):
+ If NOSIDE is EVAL_AVOID_SIDE_EFFECTS, then OBJP's memory cannot be
+ read while picking the best overload match (it may be all zeroes and thus
+ not have a vtable pointer), in which case skip virtual function lookup.
+ This is ok as typically EVAL_AVOID_SIDE_EFFECTS is only used to determine
+ the result type.
and then I'd be ok with just removing this comment:
+ /* If the best matching method is virtual, then picking the correct
+ implementation of the method will require reading the vtable of
+ the object via the vtable pointer. Hence, do not do this if
+ NOSIDE is EVAL_AVOID_SIDE_EFFECTS. It will not lead to any
+ errors because of this as all that is of interest when evaluating
+ expressions under EVAL_AVOID_SIDE_EFFECTS is the return type of
+ the method. */
@@ -1601,7 +1601,7 @@ evaluate_subexp_standard (struct type *expect_type,
NON_METHOD, /* not method */
NULL, NULL, /* pass NULL symbol since
symbol is unknown */
- NULL, &symp, NULL, 0);
+ NULL, &symp, NULL, 0, noside);
/* Now fix the expression being evaluated. */
exp->elts[save_pos1 + 2].symbol = symp;
@@ -1637,7 +1637,7 @@ evaluate_subexp_standard (struct type *expect_type,
METHOD, /* method */
&arg2, /* the object */
NULL, &valp, NULL,
- &static_memfuncp, 0);
+ &static_memfuncp, 0, noside);
if (op == OP_SCOPE && !static_memfuncp)
{
@@ -1708,7 +1708,7 @@ evaluate_subexp_standard (struct type *expect_type,
NULL, /* no need for name */
NON_METHOD, /* not method */
NULL, function, /* the function */
- NULL, &symp, NULL, no_adl);
+ NULL, &symp, NULL, no_adl, noside);
if (op == OP_VAR_VALUE)
{
new file mode 100644
@@ -0,0 +1,61 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2014 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/>. */
+
+
+/* One could use unique_ptr instead, but that requires a GCC which can
+ support "-std=c++11". */
+
+template <typename T>
+class smart_ptr
+{
+public:
+ smart_ptr (T *obj) : _obj (obj) { }
+ ~smart_ptr () { delete _obj; }
+
+ T *operator-> ();
+
+private:
+ T *_obj;
+};
+
+template <typename T>
+T *
+smart_ptr<T>::operator-> ()
+{
+ return _obj;
+}
+
+class A
+{
+public:
+ virtual int func ();
+};
+
+int
+A::func ()
+{
+ return 3;
+}
+
+int
+main ()
+{
+ A *a_ptr = 0;
+ smart_ptr<A> a (new A);
+
+ return a->func(); /* Break here */
+}
new file mode 100644
@@ -0,0 +1,40 @@
+# Copyright 2014 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/>.
+
+# This file is part of the gdb testsuite
+
+if {[skip_cplus_tests]} { continue }
+
+standard_testfile .cc
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}]} {
+ return -1
+}
+
+if {![runto_main]} {
+ return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "Break here"]
+gdb_continue_to_breakpoint "Break here"
+
+gdb_test "ptype a_ptr->func ()" ".* = int" "ptype a_ptr->func ()"
+gdb_test "ptype a->func ()" ".* = int" "ptype a->func ()"
+gdb_test "p sizeof (a_ptr->func()) == sizeof (int)" ".* = true" \
+ "p sizeof (a_ptr->func())"
+gdb_test "p sizeof (a->func()) == sizeof (int)" ".* = true" \
+ "p sizeof (a->func())"
+gdb_test "p 1 && a->func()" ".* = true" "p 1 && a->func()"
+gdb_test "p 0 || a->func()" ".* = true" "p 0 || a->func()"
@@ -287,7 +287,7 @@ unop_user_defined_p (enum exp_opcode op, struct value *arg1)
static struct value *
value_user_defined_cpp_op (struct value **args, int nargs, char *operator,
- int *static_memfuncp)
+ int *static_memfuncp, enum noside noside)
{
struct symbol *symp = NULL;
@@ -296,7 +296,7 @@ value_user_defined_cpp_op (struct value **args, int nargs, char *operator,
find_overload_match (args, nargs, operator, BOTH /* could be method */,
&args[0] /* objp */,
NULL /* pass NULL symbol since symbol is unknown */,
- &valp, &symp, static_memfuncp, 0);
+ &valp, &symp, static_memfuncp, 0, noside);
if (valp)
return valp;
@@ -318,12 +318,15 @@ value_user_defined_cpp_op (struct value **args, int nargs, char *operator,
static struct value *
value_user_defined_op (struct value **argp, struct value **args, char *name,
- int *static_memfuncp, int nargs)
+ int *static_memfuncp, int nargs, enum noside noside)
{
struct value *result = NULL;
if (current_language->la_language == language_cplus)
- result = value_user_defined_cpp_op (args, nargs, name, static_memfuncp);
+ {
+ result = value_user_defined_cpp_op (args, nargs, name, static_memfuncp,
+ noside);
+ }
else
result = value_struct_elt (argp, args, name, static_memfuncp,
"structure");
@@ -472,7 +475,7 @@ value_x_binop (struct value *arg1, struct value *arg2, enum exp_opcode op,
}
argvec[0] = value_user_defined_op (&arg1, argvec + 1, tstr,
- &static_memfuncp, 2);
+ &static_memfuncp, 2, noside);
if (argvec[0])
{
@@ -583,7 +586,7 @@ value_x_unop (struct value *arg1, enum exp_opcode op, enum noside noside)
}
argvec[0] = value_user_defined_op (&arg1, argvec + 1, tstr,
- &static_memfuncp, nargs);
+ &static_memfuncp, nargs, noside);
if (argvec[0])
{
@@ -2452,6 +2452,9 @@ value_find_oload_method_list (struct value **argp, const char *method,
ADL overload candidates when performing overload resolution for a fully
qualified name.
+ If NOSIDE is EVAL_AVOID_SIDE_EFFECTS, then OBJP's memory will not be
+ read while picking the best overload match.
+
Note: This function does *not* check the value of
overload_resolution. Caller must check it to see whether overload
resolution is permitted. */
@@ -2461,7 +2464,8 @@ find_overload_match (struct value **args, int nargs,
const char *name, enum oload_search_type method,
struct value **objp, struct symbol *fsym,
struct value **valp, struct symbol **symp,
- int *staticp, const int no_adl)
+ int *staticp, const int no_adl,
+ const enum noside noside)
{
struct value *obj = (objp ? *objp : NULL);
struct type *obj_type = obj ? value_type (obj) : NULL;
@@ -2767,9 +2771,20 @@ find_overload_match (struct value **args, int nargs,
{
if (src_method_oload_champ >= 0)
{
- if (TYPE_FN_FIELD_VIRTUAL_P (fns_ptr, method_oload_champ))
- *valp = value_virtual_fn_field (&temp, fns_ptr, method_oload_champ,
- basetype, boffset);
+ /* If the best matching method is virtual, then picking the correct
+ implementation of the method will require reading the vtable of
+ the object via the vtable pointer. Hence, do not do this if
+ NOSIDE is EVAL_AVOID_SIDE_EFFECTS. It will not lead to any
+ errors because of this as all that is of interest when evaluating
+ expressions under EVAL_AVOID_SIDE_EFFECTS is the return type of
+ the method. */
+ if (TYPE_FN_FIELD_VIRTUAL_P (fns_ptr, method_oload_champ)
+ && noside != EVAL_AVOID_SIDE_EFFECTS)
+ {
+ *valp = value_virtual_fn_field (&temp, fns_ptr,
+ method_oload_champ, basetype,
+ boffset);
+ }
else
*valp = value_fn_field (&temp, fns_ptr, method_oload_champ,
basetype, boffset);
@@ -694,7 +694,8 @@ extern int find_overload_match (struct value **args, int nargs,
enum oload_search_type method,
struct value **objp, struct symbol *fsym,
struct value **valp, struct symbol **symp,
- int *staticp, const int no_adl);
+ int *staticp, const int no_adl,
+ enum noside noside);
extern struct value *value_field (struct value *arg1, int fieldno);