PR c++/17494 - Fix evaluation of method calls under EVAL_SKIP
Commit Message
When evaluating method calls under EVAL_SKIP, the "object" and the
arguments to the method should also be evaluated under EVAL_SKIP,
instead of skipping to evaluate them. Getting this right fixes PR
c++/17494.
gdb/ChangeLog:
2014-10-20 Siva Chandra Reddy <sivachandra@google.com>
PR c++/17494
* eval.c (evaluate_subexp_standard): Evaluate the "object" and
the method args also under EVAL_SKIP when evaluating method
calls under EVAL_SKIP.
gdb/testsuite/ChangeLog:
2014-10-20 Siva Chandra Reddy <sivachandra@google.com>
PR c++/17494
* gdb.cp/pr17494.cc: New file.
* gdb.cp/pr17494.exp: New file.
Comments
On Mon, Oct 20, 2014 at 12:11 PM, Siva Chandra <sivachandra@google.com> wrote:
> When evaluating method calls under EVAL_SKIP, the "object" and the
> arguments to the method should also be evaluated under EVAL_SKIP,
> instead of skipping to evaluate them. Getting this right fixes PR
> c++/17494.
>
> gdb/ChangeLog:
>
> 2014-10-20 Siva Chandra Reddy <sivachandra@google.com>
>
> PR c++/17494
> * eval.c (evaluate_subexp_standard): Evaluate the "object" and
> the method args also under EVAL_SKIP when evaluating method
> calls under EVAL_SKIP.
>
> gdb/testsuite/ChangeLog:
>
> 2014-10-20 Siva Chandra Reddy <sivachandra@google.com>
>
> PR c++/17494
> * gdb.cp/pr17494.cc: New file.
> * gdb.cp/pr17494.exp: New file.
Ping.
On Mon, Oct 20, 2014 at 12:11 PM, Siva Chandra <sivachandra@google.com> wrote:
> When evaluating method calls under EVAL_SKIP, the "object" and the
> arguments to the method should also be evaluated under EVAL_SKIP,
> instead of skipping to evaluate them. Getting this right fixes PR
> c++/17494.
>
> gdb/ChangeLog:
>
> 2014-10-20 Siva Chandra Reddy <sivachandra@google.com>
>
> PR c++/17494
> * eval.c (evaluate_subexp_standard): Evaluate the "object" and
> the method args also under EVAL_SKIP when evaluating method
> calls under EVAL_SKIP.
>
> gdb/testsuite/ChangeLog:
>
> 2014-10-20 Siva Chandra Reddy <sivachandra@google.com>
>
> PR c++/17494
> * gdb.cp/pr17494.cc: New file.
> * gdb.cp/pr17494.exp: New file.
Ping.
On Mon, Nov 3, 2014 at 7:08 AM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
>>gdb/ChangeLog:
>>
>>2014-10-20 Siva Chandra Reddy <sivachandra@google.com>
>>
>> PR c++/17494
>> * eval.c (evaluate_subexp_standard): Evaluate the "object" and
>> the method args also under EVAL_SKIP when evaluating method
>> calls under EVAL_SKIP.
>>
>>gdb/testsuite/ChangeLog:
>>
>>2014-10-20 Siva Chandra Reddy <sivachandra@google.com>
>>
>> PR c++/17494
>> * gdb.cp/pr17494.cc: New file.
>> * gdb.cp/pr17494.exp: New file.
>
> This is OK.
Thanks for the review. Pushed: e0f52461c2467b6610391681fa27cd9b3c5def57
On Mon, Nov 03, 2014 at 06:08:15PM -0800, Siva Chandra wrote:
> >> PR c++/17494
> >> * eval.c (evaluate_subexp_standard): Evaluate the "object" and
> >> the method args also under EVAL_SKIP when evaluating method
> >> calls under EVAL_SKIP.
>
> Thanks for the review. Pushed: e0f52461c2467b6610391681fa27cd9b3c5def57
After this patch, when using gcc-4.9.1 and gcc-5.0 (20140911) I see
what looks to me to be nonsense errors (ie. I'd suspect a compiler
bug). Does anyone else see the same? Nothing special in configure
options, x86_64-linux.
/src/binutils-gdb/gdb/eval.c: In function ‘evaluate_subexp_standard’:
/src/binutils-gdb/gdb/eval.c:745:16: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
struct value *ret = NULL;
^
In file included from /src/binutils-gdb/gdb/common/common-defs.h:49:0,
from /src/binutils-gdb/gdb/defs.h:28,
from /src/binutils-gdb/gdb/eval.c:20:
/src/binutils-gdb/gdb/common/common-exceptions.h:148:20: error: ‘buf’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
SIGJMP_BUF *buf = \
^
/src/binutils-gdb/gdb/common/common-exceptions.h:148:20: note: ‘buf’ was declared here
SIGJMP_BUF *buf = \
^
/src/binutils-gdb/gdb/eval.c:747:2: note: in expansion of macro ‘TRY_CATCH’
TRY_CATCH (ex, RETURN_MASK_ERROR)
^
/src/binutils-gdb/gdb/eval.c:1429:19: error: ‘value’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
struct value *value = NULL;
^
In file included from /src/binutils-gdb/gdb/common/common-defs.h:49:0,
from /src/binutils-gdb/gdb/defs.h:28,
from /src/binutils-gdb/gdb/eval.c:20:
/src/binutils-gdb/gdb/common/common-exceptions.h:148:20: error: ‘buf’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
SIGJMP_BUF *buf = \
^
/src/binutils-gdb/gdb/common/common-exceptions.h:148:20: note: ‘buf’ was declared here
SIGJMP_BUF *buf = \
^
/src/binutils-gdb/gdb/eval.c:1430:5: note: in expansion of macro ‘TRY_CATCH’
TRY_CATCH (except, RETURN_MASK_ERROR)
^
/src/binutils-gdb/gdb/eval.c:1846:18: error: ‘value’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
struct value *value = NULL;
^
In file included from /src/binutils-gdb/gdb/common/common-defs.h:49:0,
from /src/binutils-gdb/gdb/defs.h:28,
from /src/binutils-gdb/gdb/eval.c:20:
/src/binutils-gdb/gdb/common/common-exceptions.h:148:20: error: ‘buf’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
SIGJMP_BUF *buf = \
^
/src/binutils-gdb/gdb/common/common-exceptions.h:148:20: note: ‘buf’ was declared here
SIGJMP_BUF *buf = \
^
/src/binutils-gdb/gdb/eval.c:1847:4: note: in expansion of macro ‘TRY_CATCH’
TRY_CATCH (except, RETURN_MASK_ERROR)
^
cc1: all warnings being treated as errors
Sorry if I am at fault. My compiler is gcc-4.8.2 and I do not see
these errors. I will dig further as soon as I can (which is not likely
until tomorrow).
On Tue, Nov 4, 2014 at 4:24 PM, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Nov 03, 2014 at 06:08:15PM -0800, Siva Chandra wrote:
>> >> PR c++/17494
>> >> * eval.c (evaluate_subexp_standard): Evaluate the "object" and
>> >> the method args also under EVAL_SKIP when evaluating method
>> >> calls under EVAL_SKIP.
>>
>> Thanks for the review. Pushed: e0f52461c2467b6610391681fa27cd9b3c5def57
>
> After this patch, when using gcc-4.9.1 and gcc-5.0 (20140911) I see
> what looks to me to be nonsense errors (ie. I'd suspect a compiler
> bug). Does anyone else see the same? Nothing special in configure
> options, x86_64-linux.
>
> /src/binutils-gdb/gdb/eval.c: In function ‘evaluate_subexp_standard’:
> /src/binutils-gdb/gdb/eval.c:745:16: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> struct value *ret = NULL;
> ^
> In file included from /src/binutils-gdb/gdb/common/common-defs.h:49:0,
> from /src/binutils-gdb/gdb/defs.h:28,
> from /src/binutils-gdb/gdb/eval.c:20:
> /src/binutils-gdb/gdb/common/common-exceptions.h:148:20: error: ‘buf’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> SIGJMP_BUF *buf = \
> ^
> /src/binutils-gdb/gdb/common/common-exceptions.h:148:20: note: ‘buf’ was declared here
> SIGJMP_BUF *buf = \
> ^
> /src/binutils-gdb/gdb/eval.c:747:2: note: in expansion of macro ‘TRY_CATCH’
> TRY_CATCH (ex, RETURN_MASK_ERROR)
> ^
> /src/binutils-gdb/gdb/eval.c:1429:19: error: ‘value’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> struct value *value = NULL;
> ^
> In file included from /src/binutils-gdb/gdb/common/common-defs.h:49:0,
> from /src/binutils-gdb/gdb/defs.h:28,
> from /src/binutils-gdb/gdb/eval.c:20:
> /src/binutils-gdb/gdb/common/common-exceptions.h:148:20: error: ‘buf’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> SIGJMP_BUF *buf = \
> ^
> /src/binutils-gdb/gdb/common/common-exceptions.h:148:20: note: ‘buf’ was declared here
> SIGJMP_BUF *buf = \
> ^
> /src/binutils-gdb/gdb/eval.c:1430:5: note: in expansion of macro ‘TRY_CATCH’
> TRY_CATCH (except, RETURN_MASK_ERROR)
> ^
> /src/binutils-gdb/gdb/eval.c:1846:18: error: ‘value’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> struct value *value = NULL;
> ^
> In file included from /src/binutils-gdb/gdb/common/common-defs.h:49:0,
> from /src/binutils-gdb/gdb/defs.h:28,
> from /src/binutils-gdb/gdb/eval.c:20:
> /src/binutils-gdb/gdb/common/common-exceptions.h:148:20: error: ‘buf’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> SIGJMP_BUF *buf = \
> ^
> /src/binutils-gdb/gdb/common/common-exceptions.h:148:20: note: ‘buf’ was declared here
> SIGJMP_BUF *buf = \
> ^
> /src/binutils-gdb/gdb/eval.c:1847:4: note: in expansion of macro ‘TRY_CATCH’
> TRY_CATCH (except, RETURN_MASK_ERROR)
> ^
> cc1: all warnings being treated as errors
>
>
> --
> Alan Modra
> Australia Development Lab, IBM
On Wed, Nov 05, 2014 at 12:24:16AM +0000, Alan Modra wrote:
> On Mon, Nov 03, 2014 at 06:08:15PM -0800, Siva Chandra wrote:
> > >> PR c++/17494
> > >> * eval.c (evaluate_subexp_standard): Evaluate the "object" and
> > >> the method args also under EVAL_SKIP when evaluating method
> > >> calls under EVAL_SKIP.
> >
> > Thanks for the review. Pushed: e0f52461c2467b6610391681fa27cd9b3c5def57
>
> After this patch, when using gcc-4.9.1 and gcc-5.0 (20140911) I see
> what looks to me to be nonsense errors (ie. I'd suspect a compiler
> bug). Does anyone else see the same? Nothing special in configure
> options, x86_64-linux.
For what it is worth, I'm seeing the same with my GCC 4.9.1 (x84_64-linux),
again nothing special in the configure options for GCC:
../gcc-4.9.1/configure --enable-checking=release --with-cpu=native
Thanks,
James
> /src/binutils-gdb/gdb/eval.c: In function ‘evaluate_subexp_standard’:
> /src/binutils-gdb/gdb/eval.c:745:16: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> struct value *ret = NULL;
> ^
> In file included from /src/binutils-gdb/gdb/common/common-defs.h:49:0,
> from /src/binutils-gdb/gdb/defs.h:28,
> from /src/binutils-gdb/gdb/eval.c:20:
> /src/binutils-gdb/gdb/common/common-exceptions.h:148:20: error: ‘buf’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> SIGJMP_BUF *buf = \
> ^
> /src/binutils-gdb/gdb/common/common-exceptions.h:148:20: note: ‘buf’ was declared here
> SIGJMP_BUF *buf = \
> ^
> /src/binutils-gdb/gdb/eval.c:747:2: note: in expansion of macro ‘TRY_CATCH’
> TRY_CATCH (ex, RETURN_MASK_ERROR)
> ^
> /src/binutils-gdb/gdb/eval.c:1429:19: error: ‘value’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> struct value *value = NULL;
> ^
> In file included from /src/binutils-gdb/gdb/common/common-defs.h:49:0,
> from /src/binutils-gdb/gdb/defs.h:28,
> from /src/binutils-gdb/gdb/eval.c:20:
> /src/binutils-gdb/gdb/common/common-exceptions.h:148:20: error: ‘buf’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> SIGJMP_BUF *buf = \
> ^
> /src/binutils-gdb/gdb/common/common-exceptions.h:148:20: note: ‘buf’ was declared here
> SIGJMP_BUF *buf = \
> ^
> /src/binutils-gdb/gdb/eval.c:1430:5: note: in expansion of macro ‘TRY_CATCH’
> TRY_CATCH (except, RETURN_MASK_ERROR)
> ^
> /src/binutils-gdb/gdb/eval.c:1846:18: error: ‘value’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> struct value *value = NULL;
> ^
> In file included from /src/binutils-gdb/gdb/common/common-defs.h:49:0,
> from /src/binutils-gdb/gdb/defs.h:28,
> from /src/binutils-gdb/gdb/eval.c:20:
> /src/binutils-gdb/gdb/common/common-exceptions.h:148:20: error: ‘buf’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> SIGJMP_BUF *buf = \
> ^
> /src/binutils-gdb/gdb/common/common-exceptions.h:148:20: note: ‘buf’ was declared here
> SIGJMP_BUF *buf = \
> ^
> /src/binutils-gdb/gdb/eval.c:1847:4: note: in expansion of macro ‘TRY_CATCH’
> TRY_CATCH (except, RETURN_MASK_ERROR)
> ^
> cc1: all warnings being treated as errors
>
>
> --
> Alan Modra
> Australia Development Lab, IBM
>
On Wed, Nov 05, 2014 at 11:00:26AM +0000, James Greenhalgh wrote:
> For what it is worth, I'm seeing the same with my GCC 4.9.1 (x84_64-linux),
> again nothing special in the configure options for GCC:
Thanks for the confirmation. I've opened a gcc bugzilla.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63748
@@ -1332,9 +1332,6 @@ evaluate_subexp_standard (struct type *expect_type,
/* First, evaluate the structure into arg2. */
pc2 = (*pos)++;
- if (noside == EVAL_SKIP)
- goto nosideret;
-
if (op == STRUCTOP_MEMBER)
{
arg2 = evaluate_subexp_for_address (exp, pos, noside);
@@ -1353,7 +1350,10 @@ evaluate_subexp_standard (struct type *expect_type,
arg1 = evaluate_subexp (NULL_TYPE, exp, pos, noside);
type = check_typedef (value_type (arg1));
- if (TYPE_CODE (type) == TYPE_CODE_METHODPTR)
+ if (noside == EVAL_SKIP)
+ tem = 1; /* Set it to the right arg index so that all arguments
+ can also be skipped. */
+ else if (TYPE_CODE (type) == TYPE_CODE_METHODPTR)
{
if (noside == EVAL_AVOID_SIDE_EFFECTS)
arg1 = value_zero (TYPE_TARGET_TYPE (type), not_lval);
@@ -1396,8 +1396,6 @@ evaluate_subexp_standard (struct type *expect_type,
pc2 = (*pos)++;
tem2 = longest_to_int (exp->elts[pc2 + 1].longconst);
*pos += 3 + BYTES_TO_EXP_ELEM (tem2 + 1);
- if (noside == EVAL_SKIP)
- goto nosideret;
if (op == STRUCTOP_STRUCT)
{
@@ -1546,6 +1544,9 @@ evaluate_subexp_standard (struct type *expect_type,
/* Signal end of arglist. */
argvec[tem] = 0;
+ if (noside == EVAL_SKIP)
+ goto nosideret;
+
if (op == OP_ADL_FUNC)
{
struct symbol *symp;
new file mode 100644
@@ -0,0 +1,63 @@
+/* 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". */
+
+int
+func (int i, int j)
+{
+ return i + j;
+}
+
+class A
+{
+public:
+ A () { a = 12345; f = &func; }
+ int geta ();
+ int adda (int i);
+
+ int a;
+ int (*f) (int, int);
+};
+
+int
+A::geta ()
+{
+ return a;
+}
+
+int
+A::adda (int i)
+{
+ return a + i;
+}
+
+int
+main ()
+{
+ A a;
+ A *a_ptr = &a;
+ int (A::*m1) ();
+ int (A::*m2) (int);
+
+ m1 = &A::geta;
+ m2 = &A::adda;
+
+ return (a.*m1) () + (a.*m2) (12) + (a.*(&A::f)) (1, 2); /* Break here */
+}
new file mode 100644
@@ -0,0 +1,57 @@
+# 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 "p a.geta()" ".* = 12345" "a.geta()"
+gdb_test "p a_ptr->geta()" ".* = 12345" "a.geta()"
+
+gdb_test "p false ? a.geta() : 123" ".* = 123" "ternop 1"
+gdb_test "p false ? a_ptr->geta() : 123" ".* = 123" "ternop 2"
+gdb_test "p (true ? 123 : a.geta()) + 123" ".* = 246" "ternop 3"
+gdb_test "p (true ? 123 : a_ptr->geta()) + 123" ".* = 246" "ternop 4"
+gdb_test "p false ? (a.*m1)() : 123" ".* = 123" "ternop 5"
+gdb_test "p false ? (a_ptr->*m1)() : 123" ".* = 123" "ternop 6"
+gdb_test "p (true ? 123 : (a.*m1)()) + 123" ".* = 246" "ternop 7"
+gdb_test "p (true ? 123 : (a_ptr->*m1)()) + 123" ".* = 246" "ternop 8"
+
+gdb_test "p false ? a.adda(456) : 123" ".* = 123" "ternop 9"
+gdb_test "p false ? a_ptr->adda(456) : 123" ".* = 123" "ternop 10"
+gdb_test "p (true ? 123 : a.adda(456)) + 123" ".* = 246" "ternop 11"
+gdb_test "p (true ? 123 : a_ptr->adda(456)) + 123" ".* = 246" "ternop 12"
+gdb_test "p false ? (a.*m2)(123) : 123" ".* = 123" "ternop 13"
+gdb_test "p false ? (a_ptr->*m2)(123) : 123" ".* = 123" "ternop 14"
+gdb_test "p (true ? 123 : (a.*m2)(123)) + 123" ".* = 246" "ternop 15"
+gdb_test "p (true ? 123 : (a_ptr->*m2)(123)) + 123" ".* = 246" "ternop 16"
+
+gdb_test "p false ? (a.*(&A::f))(1, 2) : 123" ".* = 123" "ternop 17"
+gdb_test "p false ? (a_ptr->*(&A::f))(1, 2) : 123" ".* = 123" "ternop 18"
+gdb_test "p (true ? 123 : (a.*(&A::f))(1, 2)) + 123" ".* = 246" "ternop 19"
+gdb_test "p (true ? 123 : (a_ptr->*(&A::f))(1, 2)) + 123" ".* = 246" "ternop 20"