| Message ID | CAGyQ6gxnfw6yRmMzGG2T5acqO2od7WL7-8F1wVRTp9mfNvg9Sg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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
diff --git a/gdb/eval.c b/gdb/eval.c index 5906744..ab431c6 100644 --- a/gdb/eval.c +++ b/gdb/eval.c @@ -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; diff --git a/gdb/testsuite/gdb.cp/pr17494.cc b/gdb/testsuite/gdb.cp/pr17494.cc new file mode 100644 index 0000000..77653e1 --- /dev/null +++ b/gdb/testsuite/gdb.cp/pr17494.cc @@ -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 */ +} diff --git a/gdb/testsuite/gdb.cp/pr17494.exp b/gdb/testsuite/gdb.cp/pr17494.exp new file mode 100644 index 0000000..7f2e0cf --- /dev/null +++ b/gdb/testsuite/gdb.cp/pr17494.exp @@ -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"
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.