From patchwork Mon Nov 17 06:08:53 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: 3775 Received: (qmail 3777 invoked by alias); 17 Nov 2014 06:08:59 -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 3766 invoked by uid 89); 17 Nov 2014 06:08:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.9 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-oi0-f51.google.com Received: from mail-oi0-f51.google.com (HELO mail-oi0-f51.google.com) (209.85.218.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 17 Nov 2014 06:08:56 +0000 Received: by mail-oi0-f51.google.com with SMTP id e131so1621442oig.38 for ; Sun, 16 Nov 2014 22:08:54 -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=tKRFANUAXMS8v+KEOneoi5eYWvGPruNiW8Szi2y8XUI=; b=A5izDBAHndKhPrih2evGZ5B7leVh5atD1XEKIEdONo/2NK+wm5U0aL+miilOAg5uU2 1mASfJQXOkMwYUxP+5XrHBz9EufdORK4Wkto5X25TzF21JycnBfO7tK4ChVkZ3pQ6/D5 JNXbSTvBDYXa4gXwPUJEBG7vxUdf1KwG1oED1N0YdD6s4j8VWxiidTtticMdmLgoIU56 sK5FskdW5Jbm9DzJZGhm0TT6r+oVqXzZCP+KLJQT5E0AOodr9P4KUrSx4gX+BSr3Gcva b1xUc6Ir3fybg5RSHC3yiJ2A6qCjWYcf9Mgcd+X5FV5TM2EbMqw/yESn4BESccoEpjW7 D9Gw== X-Gm-Message-State: ALoCoQnnQjlgZpgpoNoiyzRo0qPk6vbuiRyX52/LPc6fIrYw/LxmoRSp4aZBRAvKbKCMWZfMRL0L MIME-Version: 1.0 X-Received: by 10.182.76.2 with SMTP id g2mr17343361obw.8.1416204533976; Sun, 16 Nov 2014 22:08:53 -0800 (PST) Received: by 10.202.137.1 with HTTP; Sun, 16 Nov 2014 22:08:53 -0800 (PST) Date: Sun, 16 Nov 2014 22:08:53 -0800 Message-ID: Subject: [PATCH v5] Make chained function calls in expressions work From: Siva Chandra To: gdb-patches Cc: Ulrich Weigand X-IsSubscribed: yes Link to v4: https://sourceware.org/ml/gdb-patches/2014-10/msg00697.html I have made all the changes suggested by Ulrich Weigand in his comments for v4. gdb/ChangeLog: 2014-11-16 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) (skip_thread_stack_temporaries) (value_in_thread_stack_temporaries): Declare. * gdbtypes.c (class_or_union_p): New function. * gdbtypes.h (class_or_union_p): Declare. * infcall.c (get_return_value_from_memory): New function. (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) (skip_thread_stack_temporaries): Likewise. (value_in_thread_stack_temporaries): Likewise. (struct ptid_data): New type. * value.c (value_force_lval): New function. * value.h (value_force_lval): Declare. gdb/testsuite/ChangeLog: 2014-11-16 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..be1c6ec 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 && inferior_ptid.pid != 0 + && 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..2bf577c 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 CORE_ADDR skip_thread_stack_temporaries (ptid_t, CORE_ADDR, int); + +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 8e44b7c..1334e7e 100644 --- a/gdb/gdbtypes.c +++ b/gdb/gdbtypes.c @@ -2501,6 +2501,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..d7ab1f9 100644 --- a/gdb/infcall.c +++ b/gdb/infcall.c @@ -455,6 +455,33 @@ cleanup_delete_std_terminate_breakpoint (void *ignore) delete_std_terminate_breakpoint (); } +/* Reads a value returned by an inferior function for those return + values whose address is passed as the hidden first argument. + TYPE is the type of value. ADDR is the address from where to read it. + If STACK_TEMPORARIES is non-zero, then the returned value will be on + lval_memory type and will be stored as a temporary on the thread's + stack. */ + +static struct value * +get_return_value_from_memory (struct type *type, CORE_ADDR addr, + int stack_temporaries) +{ + struct value *retval; + if (stack_temporaries) + { + retval = value_from_contents_and_address (type, NULL, addr); + push_thread_stack_temporary (inferior_ptid, retval); + } + else + { + retval = allocate_value (type); + read_value_memory (retval, 0, 1, addr, value_contents_raw (retval), + TYPE_LENGTH (type)); + } + + return retval; +} + /* All this stuff with a dummy frame may seem unnecessarily complicated (why not just save registers in GDB?). The purpose of pushing a dummy frame which looks just like a real frame is so that if you call a @@ -495,6 +522,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 +621,16 @@ 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) + { + if (gdbarch_inner_than (gdbarch, 1, 2)) + sp = skip_thread_stack_temporaries (inferior_ptid, sp, 1); + else + sp = skip_thread_stack_temporaries (inferior_ptid, sp, 0); + } } funaddr = find_function_addr (function, &values_type); @@ -719,9 +757,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. + + 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) + if (struct_return || hidden_first_param_p + || (stack_temporaries && class_or_union_p (values_type))) { if (gdbarch_inner_than (gdbarch, 1, 2)) { @@ -1059,31 +1109,27 @@ 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) + retval = get_return_value_from_memory (values_type, struct_addr, + stack_temporaries); + else { - /* If the function returns void, don't bother fetching the - return value. */ - switch (gdbarch_return_value (gdbarch, function, target_values_type, - NULL, NULL, NULL)) + 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)) { - 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: - read_value_memory (retval, 0, 1, struct_addr, - value_contents_raw (retval), - TYPE_LENGTH (values_type)); - break; + /* 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..4a7a26e 100644 --- a/gdb/thread.c +++ b/gdb/thread.c @@ -636,6 +636,131 @@ prune_threads (void) } } +/* A simple struct to pass ptid data to a cleanup function. */ + +struct ptid_data +{ + ptid_t ptid; +}; + +/* Disable storing stack temporaries for the thread whose id is + stored in DATA. */ + +static void +disable_thread_stack_temporaries (void *data) +{ + struct ptid_data *pd = data; + struct thread_info *tp = find_thread_ptid (pd->ptid); + + if (tp != NULL) + tp->stack_temporaries_enabled = 0; + + 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); + struct ptid_data *data; + struct cleanup *c; + + gdb_assert (tp != NULL); + + tp->stack_temporaries_enabled = 1; + tp->stack_temporaries = NULL; + data = (struct ptid_data *) xmalloc (sizeof (struct ptid_data)); + data->ptid = ptid; + c = make_cleanup (disable_thread_stack_temporaries, data); + make_cleanup (VEC_cleanup (value_ptr), &tp->stack_temporaries); + + 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. */ + +CORE_ADDR +skip_thread_stack_temporaries (ptid_t ptid, CORE_ADDR sp, int downward) +{ + CORE_ADDR addr = sp; + struct thread_info *tp = find_thread_ptid (ptid); + + gdb_assert (tp != NULL); + if (!VEC_empty (value_ptr, tp->stack_temporaries)) + { + struct value *v = VEC_last (value_ptr, tp->stack_temporaries); + CORE_ADDR val_addr = value_address (v); + + if (downward) + { + gdb_assert (sp >= val_addr); + addr = val_addr; + } + else + { + struct type *type; + + gdb_assert (sp <= val_addr); + type = value_type (v); + addr = val_addr + TYPE_LENGTH (type); + } + } + + return addr; +} + void thread_change_ptid (ptid_t old_ptid, ptid_t new_ptid) { diff --git a/gdb/value.c b/gdb/value.c index ecfb154..b02713d 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 */