From patchwork Thu Nov 27 06:55:44 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Siva Chandra Reddy X-Patchwork-Id: 3966 Received: (qmail 27223 invoked by alias); 27 Nov 2014 06:55:51 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 27210 invoked by uid 89); 27 Nov 2014 06:55:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.8 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mail-ob0-f176.google.com Received: from mail-ob0-f176.google.com (HELO mail-ob0-f176.google.com) (209.85.214.176) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 27 Nov 2014 06:55:46 +0000 Received: by mail-ob0-f176.google.com with SMTP id vb8so3298007obc.7 for ; Wed, 26 Nov 2014 22:55:45 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:date:message-id:subject:from:to:cc :content-type; bh=DFTQ/YO60XQfPGY/KK5Ml8Kb+IhRBBGbqSyfMfZQCxY=; b=WhySIFwk/gMytRxbEliucmHc8LoEvjHKTQ/mQSVmhsEpNglcItNU0fn9utKvnurnGR kQRNHvl9/TVaf4enSK+rEtSwDckjThfl0Hq8LuYHRc8Gp4RtPz3aWCp2oJyj+NsEcsDU w8GEhel/cehDRVFu6p+SSwOZL0PLBn0OQ16VHVMrGpwE9cOhpOf8tzv5SnPrSyDMw4Ni 70EkxC62YRKeBI2RGHVXFBk5ZsMu9lzkm3r4JotnQ521kOKqSKcRGHc/7cp1DdYMfESf N0n7lD1iZdHA/jD//4VkrD3VhD9P8KAOjX5oy0/TPZr268aJpJrlVi2CQ2zalL3MvYnt hC5Q== X-Gm-Message-State: ALoCoQkj/lvDDUCvBNijSrHT0ZNRiDGlbqQqtMScv5IBw9rDv80CD0p28+utk3VM/cXBNPIgYWLq MIME-Version: 1.0 X-Received: by 10.182.213.2 with SMTP id no2mr21583996obc.76.1417071344900; Wed, 26 Nov 2014 22:55:44 -0800 (PST) Received: by 10.202.225.68 with HTTP; Wed, 26 Nov 2014 22:55:44 -0800 (PST) Date: Wed, 26 Nov 2014 22:55:44 -0800 Message-ID: Subject: [PATCH v6] Make chained function calls in expressions work From: Siva Chandra To: gdb-patches Cc: Ulrich Weigand X-IsSubscribed: yes Link to v5: https://sourceware.org/ml/gdb-patches/2014-11/msg00394.html I have made all the changes suggested by Ulrich Weigand in his comments for v5. gdb/ChangeLog: 2014-11-27 Siva Chandra Reddy * eval.c: Include gdbthread.h. (evaluate_subexp): Enable thread stack temporaries before evaluating a complete expression and clean them up after the evaluation is complete. * gdbthread.h: Include common/vec.h. (value_ptr): New typedef. (VEC (value_ptr)): New vector type. (value_vec): New typedef. (struct thread_info): Add new fields stack_temporaries_enabled and stack_temporaries. (enable_thread_stack_temporaries) (thread_stack_temporaries_enabled_p, push_thread_stack_temporary) (get_last_thread_stack_temporary) (value_in_thread_stack_temporaries): Declare. * gdbtypes.c (class_or_union_p): New function. * gdbtypes.h (class_or_union_p): Declare. * infcall.c (call_function_by_hand): Store return values of class type as temporaries on stack. * thread.c (enable_thread_stack_temporaries): New function. (thread_stack_temporaries_enabled_p, push_thread_stack_temporary) (get_last_thread_stack_temporary): Likewise. (value_in_thread_stack_temporaries): Likewise. * value.c (value_force_lval): New function. * value.h (value_force_lval): Declare. gdb/testsuite/ChangeLog: 2014-11-27 Siva Chandra Reddy * gdb.cp/chained-calls.cc: New file. * gdb.cp/chained-calls.exp: New file. * gdb.cp/smartp.exp: Remove KFAIL for "p c2->inta". diff --git a/gdb/eval.c b/gdb/eval.c index 655ea22..a13793c 100644 --- a/gdb/eval.c +++ b/gdb/eval.c @@ -24,6 +24,7 @@ #include "expression.h" #include "target.h" #include "frame.h" +#include "gdbthread.h" #include "language.h" /* For CAST_IS_CONVERSION. */ #include "f-lang.h" /* For array bound stuff. */ #include "cp-abi.h" @@ -63,8 +64,28 @@ struct value * evaluate_subexp (struct type *expect_type, struct expression *exp, int *pos, enum noside noside) { - return (*exp->language_defn->la_exp_desc->evaluate_exp) + struct cleanup *cleanups; + struct value *retval; + int cleanup_temps = 0; + + if (*pos == 0 && target_has_execution + && exp->language_defn->la_language == language_cplus) + { + cleanups = enable_thread_stack_temporaries (inferior_ptid); + cleanup_temps = 1; + } + + retval = (*exp->language_defn->la_exp_desc->evaluate_exp) (expect_type, exp, pos, noside); + + if (cleanup_temps) + { + if (value_in_thread_stack_temporaries (retval, inferior_ptid)) + retval = value_non_lval (retval); + do_cleanups (cleanups); + } + + return retval; } /* Parse the string EXP as a C expression, evaluate it, diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h index 4fd5f69..c7e258f 100644 --- a/gdb/gdbthread.h +++ b/gdb/gdbthread.h @@ -28,6 +28,7 @@ struct symtab; #include "ui-out.h" #include "inferior.h" #include "btrace.h" +#include "common/vec.h" /* Frontend view of the thread state. Possible extensions: stepping, finishing, until(ling),... */ @@ -152,6 +153,10 @@ struct thread_suspend_state enum gdb_signal stop_signal; }; +typedef struct value *value_ptr; +DEF_VEC_P (value_ptr); +typedef VEC (value_ptr) value_vec; + struct thread_info { struct thread_info *next; @@ -264,6 +269,14 @@ struct thread_info /* Branch trace information for this thread. */ struct btrace_thread_info btrace; + + /* Flag which indicates that the stack temporaries should be stored while + evaluating expressions. */ + int stack_temporaries_enabled; + + /* Values that are stored as temporaries on stack while evaluating + expressions. */ + value_vec *stack_temporaries; }; /* Create an empty thread list, or empty the existing one. */ @@ -465,6 +478,16 @@ extern void prune_threads (void); int pc_in_thread_step_range (CORE_ADDR pc, struct thread_info *thread); +extern struct cleanup *enable_thread_stack_temporaries (ptid_t ptid); + +extern int thread_stack_temporaries_enabled_p (ptid_t ptid); + +extern void push_thread_stack_temporary (ptid_t ptid, struct value *v); + +extern struct value *get_last_thread_stack_temporary (ptid_t); + +extern int value_in_thread_stack_temporaries (struct value *, ptid_t); + extern struct thread_info *thread_list; #endif /* GDBTHREAD_H */ diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c index 61d259d..611a0e7 100644 --- a/gdb/gdbtypes.c +++ b/gdb/gdbtypes.c @@ -2508,6 +2508,15 @@ is_scalar_type_recursive (struct type *t) return 0; } +/* Return true is T is a class or a union. False otherwise. */ + +int +class_or_union_p (const struct type *t) +{ + return (TYPE_CODE (t) == TYPE_CODE_STRUCT + || TYPE_CODE (t) == TYPE_CODE_UNION); +} + /* A helper function which returns true if types A and B represent the "same" class type. This is true if the types have the same main type, or the same name. */ diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h index 14a1f08..a56f543 100644 --- a/gdb/gdbtypes.h +++ b/gdb/gdbtypes.h @@ -1823,6 +1823,8 @@ extern int is_integral_type (struct type *); extern int is_scalar_type_recursive (struct type *); +extern int class_or_union_p (const struct type *); + extern void maintenance_print_type (char *, int); extern htab_t create_copied_types_hash (struct objfile *objfile); diff --git a/gdb/infcall.c b/gdb/infcall.c index bbac693..718393c 100644 --- a/gdb/infcall.c +++ b/gdb/infcall.c @@ -495,6 +495,7 @@ call_function_by_hand (struct value *function, int nargs, struct value **args) ptid_t call_thread_ptid; struct gdb_exception e; char name_buf[RAW_FUNCTION_ADDRESS_SIZE]; + int stack_temporaries = thread_stack_temporaries_enabled_p (inferior_ptid); if (TYPE_CODE (ftype) == TYPE_CODE_PTR) ftype = check_typedef (TYPE_TARGET_TYPE (ftype)); @@ -593,6 +594,33 @@ call_function_by_hand (struct value *function, int nargs, struct value **args) If the ABI specifies a "Red Zone" (see the doco) the code below will quietly trash it. */ sp = old_sp; + + /* Skip over the stack temporaries that might have been generated during + the evaluation of an expression. */ + if (stack_temporaries) + { + struct value *lastval; + + lastval = get_last_thread_stack_temporary (inferior_ptid); + if (lastval != NULL) + { + CORE_ADDR lastval_addr = value_address (lastval); + + if (gdbarch_inner_than (gdbarch, 1, 2)) + { + gdb_assert (sp >= lastval_addr); + sp = lastval_addr; + } + else + { + gdb_assert (sp <= lastval_addr); + sp = lastval_addr + TYPE_LENGTH (value_type (lastval)); + } + + if (gdbarch_frame_align_p (gdbarch)) + sp = gdbarch_frame_align (gdbarch, sp); + } + } } funaddr = find_function_addr (function, &values_type); @@ -719,9 +747,21 @@ call_function_by_hand (struct value *function, int nargs, struct value **args) /* Reserve space for the return structure to be written on the stack, if necessary. Make certain that the value is correctly - aligned. */ + aligned. + + While evaluating expressions, we reserve space on the stack for + return values of class type even if the language ABI and the target + ABI do not require that the return value be passed as a hidden first + argument. This is because we want to store the return value as an + on-stack temporary while the expression is being evaluated. This + enables us to have chained function calls in expressions. - if (struct_return || hidden_first_param_p) + Keeping the return values as on-stack temporaries while the expression + is being evaluated is OK because the thread is stopped until the + expression is completely evaluated. */ + + if (struct_return || hidden_first_param_p + || (stack_temporaries && class_or_union_p (values_type))) { if (gdbarch_inner_than (gdbarch, 1, 2)) { @@ -1059,31 +1099,40 @@ When the function is done executing, GDB will silently stop."), At this stage, leave the RETBUF alone. */ restore_infcall_control_state (inf_status); - /* Figure out the value returned by the function. */ - retval = allocate_value (values_type); - - if (hidden_first_param_p) - read_value_memory (retval, 0, 1, struct_addr, - value_contents_raw (retval), - TYPE_LENGTH (values_type)); - else if (TYPE_CODE (target_values_type) != TYPE_CODE_VOID) + if (TYPE_CODE (values_type) == TYPE_CODE_VOID) + retval = allocate_value (values_type); + else if (struct_return || hidden_first_param_p) { - /* If the function returns void, don't bother fetching the - return value. */ - switch (gdbarch_return_value (gdbarch, function, target_values_type, - NULL, NULL, NULL)) + if (stack_temporaries) + { + retval = value_from_contents_and_address (values_type, NULL, + struct_addr); + push_thread_stack_temporary (inferior_ptid, retval); + } + else { - case RETURN_VALUE_REGISTER_CONVENTION: - case RETURN_VALUE_ABI_RETURNS_ADDRESS: - case RETURN_VALUE_ABI_PRESERVES_ADDRESS: - gdbarch_return_value (gdbarch, function, values_type, - retbuf, value_contents_raw (retval), NULL); - break; - case RETURN_VALUE_STRUCT_CONVENTION: + retval = allocate_value (values_type); read_value_memory (retval, 0, 1, struct_addr, value_contents_raw (retval), TYPE_LENGTH (values_type)); - break; + } + } + else + { + retval = allocate_value (values_type); + gdbarch_return_value (gdbarch, function, values_type, + retbuf, value_contents_raw (retval), NULL); + if (stack_temporaries && class_or_union_p (values_type)) + { + /* Values of class type returned in registers are copied onto + the stack and their lval_type set to lval_memory. This is + required because further evaluation of the expression + could potentially invoke methods on the return value + requiring GDB to evaluate the "this" pointer. To evaluate + the this pointer, GDB needs the memory address of the + value. */ + value_force_lval (retval, struct_addr); + push_thread_stack_temporary (inferior_ptid, retval); } } diff --git a/gdb/testsuite/gdb.cp/chained-calls.cc b/gdb/testsuite/gdb.cp/chained-calls.cc new file mode 100644 index 0000000..a30ec3b --- /dev/null +++ b/gdb/testsuite/gdb.cp/chained-calls.cc @@ -0,0 +1,203 @@ +/* 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 . */ + +class S +{ +public: + S () { } + S (S &obj); + + S operator+ (const S &s); + + int a; +}; + +S::S (S &obj) +{ + a = obj.a; +} + +S +S::operator+ (const S &s) +{ + S res; + + res.a = a + s.a; + + return res; +} + +S +f (int i) +{ + S s; + + s.a = i; + + return s; +} + +int +g (const S &s) +{ + return s.a; +} + +class A +{ +public: + A operator+ (const A &); + int a; +}; + +A +A::operator+ (const A &obj) +{ + A n; + + n.a = a + obj.a; + + return n; +} + +A +p () +{ + A a; + a.a = 12345678; + return a; +} + +A +r () +{ + A a; + a.a = 10000000; + return a; +} + +A +q (const A &a) +{ + return a; +} + +class B +{ +public: + int b[1024]; +}; + +B +makeb () +{ + B b; + int i; + + for (i = 0; i < 1024; i++) + b.b[i] = i; + + return b; +} + +int +getb (const B &b, int i) +{ + return b.b[i]; +} + +class C +{ +public: + C (); + ~C (); + + A operator* (); + + A *a_ptr; +}; + +C::C () +{ + a_ptr = new A; + a_ptr->a = 5678; +} + +C::~C () +{ + delete a_ptr; +} + +A +C::operator* () +{ + return *a_ptr; +} + +#define TYPE_INDEX 1 + +enum type +{ + INT, + CHAR +}; + +union U +{ +public: + U (type t); + type get_type (); + + int a; + char c; + type tp[2]; +}; + +U::U (type t) +{ + tp[TYPE_INDEX] = t; +} + +U +make_int () +{ + return U (INT); +} + +U +make_char () +{ + return U (CHAR); +} + +type +U::get_type () +{ + return tp[TYPE_INDEX]; +} + +int +main () +{ + int i = g(f(0)); + A a = q(p() + r()); + + B b = makeb (); + C c; + + return i + getb(b, 0); /* Break here */ +} diff --git a/gdb/testsuite/gdb.cp/chained-calls.exp b/gdb/testsuite/gdb.cp/chained-calls.exp new file mode 100644 index 0000000..c903bea --- /dev/null +++ b/gdb/testsuite/gdb.cp/chained-calls.exp @@ -0,0 +1,44 @@ +# 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 . + +# 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 g(f(12345))" ".* = 12345" "g(f())" +gdb_test "p q(p())" ".* = {a = 12345678}" "q(p())" +gdb_test "p p() + r()" ".* = {a = 22345678}" "p() + r()" +gdb_test "p q(p() + r())" ".* = {a = 22345678}" "q(p() + r())" +gdb_test "p g(f(6700) + f(89))" ".* = 6789" "g(f() + f())" +gdb_test "p g(f(g(f(300) + f(40))) + f(5))" ".* = 345" \ + "g(f(g(f() + f())) + f())" +gdb_test "p getb(makeb(), 789)" ".* = 789" "getb(makeb(), ...)" +gdb_test "p *c" ".* = {a = 5678}" "*c" +gdb_test "p *c + *c" ".* = {a = 11356}" "*c + *c" +gdb_test "P q(*c + *c)" ".* = {a = 11356}" "q(*c + *c)" +gdb_test "p make_int().get_type ()" ".* = INT" "make_int().get_type ()" diff --git a/gdb/testsuite/gdb.cp/smartp.exp b/gdb/testsuite/gdb.cp/smartp.exp index 2a1028a..e3d271f 100644 --- a/gdb/testsuite/gdb.cp/smartp.exp +++ b/gdb/testsuite/gdb.cp/smartp.exp @@ -72,6 +72,5 @@ gdb_test "p b->foo()" "= 66" gdb_test "p c->foo()" "= 66" gdb_test "p c->inta" "= 77" -setup_kfail "gdb/11606" "*-*-*" gdb_test "p c2->inta" "= 77" diff --git a/gdb/thread.c b/gdb/thread.c index 5c94264..a55a47a 100644 --- a/gdb/thread.c +++ b/gdb/thread.c @@ -636,6 +636,109 @@ prune_threads (void) } } +/* Disable storing stack temporaries for the thread whose id is + stored in DATA. */ + +static void +disable_thread_stack_temporaries (void *data) +{ + ptid_t *pd = data; + struct thread_info *tp = find_thread_ptid (*pd); + + if (tp != NULL) + { + tp->stack_temporaries_enabled = 0; + VEC_free (value_ptr, tp->stack_temporaries); + } + + xfree (pd); +} + +/* Enable storing stack temporaries for thread with id PTID and return a + cleanup which can disable and clear the stack temporaries. */ + +struct cleanup * +enable_thread_stack_temporaries (ptid_t ptid) +{ + struct thread_info *tp = find_thread_ptid (ptid); + ptid_t *data; + struct cleanup *c; + + gdb_assert (tp != NULL); + + tp->stack_temporaries_enabled = 1; + tp->stack_temporaries = NULL; + data = (ptid_t *) xmalloc (sizeof (ptid_t)); + *data = ptid; + c = make_cleanup (disable_thread_stack_temporaries, data); + + return c; +} + +/* Return non-zero value if stack temporaies are enabled for the thread + with id PTID. */ + +int +thread_stack_temporaries_enabled_p (ptid_t ptid) +{ + struct thread_info *tp = find_thread_ptid (ptid); + + if (tp == NULL) + return 0; + else + return tp->stack_temporaries_enabled; +} + +/* Push V on to the stack temporaries of the thread with id PTID. */ + +void +push_thread_stack_temporary (ptid_t ptid, struct value *v) +{ + struct thread_info *tp = find_thread_ptid (ptid); + + gdb_assert (tp != NULL && tp->stack_temporaries_enabled); + VEC_safe_push (value_ptr, tp->stack_temporaries, v); +} + +/* Return 1 if VAL is among the stack temporaries of the thread + with id PTID. Return 0 otherwise. */ + +int +value_in_thread_stack_temporaries (struct value *val, ptid_t ptid) +{ + struct thread_info *tp = find_thread_ptid (ptid); + + gdb_assert (tp != NULL && tp->stack_temporaries_enabled); + if (!VEC_empty (value_ptr, tp->stack_temporaries)) + { + struct value *v; + int i; + + for (i = 0; VEC_iterate (value_ptr, tp->stack_temporaries, i, v); i++) + if (v == val) + return 1; + } + + return 0; +} + +/* Return an address after skipping over the current stack temporaries + of thread with id PTID. SP is the current stack frame pointer. Non-zero + DOWNWARD indicates that the stack grows downwards/backwards. */ + +struct value * +get_last_thread_stack_temporary (ptid_t ptid) +{ + struct value *lastval = NULL; + struct thread_info *tp = find_thread_ptid (ptid); + + gdb_assert (tp != NULL); + if (!VEC_empty (value_ptr, tp->stack_temporaries)) + lastval = VEC_last (value_ptr, tp->stack_temporaries); + + return lastval; +} + void thread_change_ptid (ptid_t old_ptid, ptid_t new_ptid) { diff --git a/gdb/value.c b/gdb/value.c index ecfb154..3a9c3a9 100644 --- a/gdb/value.c +++ b/gdb/value.c @@ -1724,6 +1724,18 @@ value_non_lval (struct value *arg) return arg; } +/* Write contents of V at ADDR and set its lval type to be LVAL_MEMORY. */ + +void +value_force_lval (struct value *v, CORE_ADDR addr) +{ + gdb_assert (VALUE_LVAL (v) == not_lval); + + write_memory (addr, value_contents_raw (v), TYPE_LENGTH (value_type (v))); + v->lval = lval_memory; + v->location.address = addr; +} + void set_value_component_location (struct value *component, const struct value *whole) diff --git a/gdb/value.h b/gdb/value.h index e3603c3..a8c33fb 100644 --- a/gdb/value.h +++ b/gdb/value.h @@ -1038,6 +1038,8 @@ extern struct value *value_copy (struct value *); extern struct value *value_non_lval (struct value *); +extern void value_force_lval (struct value *, CORE_ADDR); + extern void preserve_one_value (struct value *, struct objfile *, htab_t); /* From valops.c */