From patchwork Thu Jul 23 11:07:34 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 7811 Received: (qmail 53636 invoked by alias); 23 Jul 2015 11:07: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 53545 invoked by uid 89); 23 Jul 2015 11:07:51 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 X-HELO: mail-wi0-f179.google.com Received: from mail-wi0-f179.google.com (HELO mail-wi0-f179.google.com) (209.85.212.179) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 23 Jul 2015 11:07:47 +0000 Received: by wibud3 with SMTP id ud3so213420884wib.0 for ; Thu, 23 Jul 2015 04:07:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:in-reply-to:references; bh=WbHs3UiwhpU10XaBn5YL7FPHqjW27HKmBIvlHWgsfyc=; b=QDdLW0o5hB80Br+3uA/27seG3AlM7MwTvaqL+vw7s9WoVr/zsIxFqeJmXyjz6LQ8rK 51R62hELfqJ0Nha0cfhSsFyIq5fxheQTXhdRlIU2Jbmn0kcnwRPU3i8JrLVquZja4DSq ws9nMUEJnYLyTA6UZuZrSRSPL4pHm2w7jNvJAXTA1XZMmPU28qPpNo8I5Sb4IYt1Tz0/ gDFOJe2OHkJBxFMf13ozosu5xynLPOjVR2TeTLqTXswWdvKaSJyXdbp2LjN7bi1ZlFrH AsoglNDXytkql6fzLxbVLbnXefItokVjqkpZPEXAsq0ggLL1yasSkuVio4zfqAAjSjQg CSNg== X-Gm-Message-State: ALoCoQmATKLhoCgcpfgUetdnyrZJHZP+SoaTHYCRz2vGsO3pqBzOd6NP0al6n/WyuKGqfQPlYMVa X-Received: by 10.194.171.129 with SMTP id au1mr15658709wjc.115.1437649664158; Thu, 23 Jul 2015 04:07:44 -0700 (PDT) Received: from localhost (host86-141-20-226.range86-141.btcentralplus.com. [86.141.20.226]) by smtp.gmail.com with ESMTPSA id ck7sm6932046wjc.43.2015.07.23.04.07.42 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 23 Jul 2015 04:07:43 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 2/2] gdb: Select a frame for frame_info. Date: Thu, 23 Jul 2015 12:07:34 +0100 Message-Id: <8092a27a3fe5289afc010fc325f10ea7394f0596.1437649195.git.andrew.burgess@embecosm.com> In-Reply-To: References: In-Reply-To: References: <1436373998-6060-1-git-send-email-andrew.burgess@embecosm.com> X-IsSubscribed: yes Within the 'info frame' command (implemented in the frame_info function) gdb will create register values, which are then displayed to the user. These register values will be created as lazy value. As part of fetching the value of a lazy register value we must find the frame in which the value was created based on the frame id, which is stored in the value, this is done with a call to frame_find_by_id. In a previous commit I already addressed the problem that frame_find_by_id did not used to consider the selected frame when trying to find a frame. This is now fixed. However, if in frame_info the user passed a frame specification, then it is possible that a brand new frame will have been created, a frame outside of the current backtrace, and also not the currently selected frame. So, usually, something like, 'info frame 4' will cause gdb to crash with an assertion failure. This can be especially annoying if the user accidentally types 'info frame 4' intending to ask about the fourth stack frame, but, there happens to only be 3 stack frames at present. The solution I propose here is that frame_info should temporarily select the frame identified by the user. The previously selected frame will be restored at the end of the function. gdb/ChangeLog: * frame.h (make_restore_selected_frame_cleanup): Declare new function. * frame.c (do_restore_selected_frame): New function. (make_restore_selected_frame_cleanup): Define new function. * stack.c (frame_info): Temporarily select frame for duration of this function. gdb/testsuite/ChangeLog: * gdb.base/create-frame.exp: Add test for 'info frame' with specific address. --- gdb/ChangeLog | 9 +++++++++ gdb/frame.c | 21 +++++++++++++++++++++ gdb/frame.h | 5 +++++ gdb/stack.c | 10 ++++++++++ gdb/testsuite/ChangeLog | 5 +++++ gdb/testsuite/gdb.base/create-frame.exp | 8 +++++++- 6 files changed, 57 insertions(+), 1 deletion(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index b4aa365..747a7c7 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,14 @@ 2015-07-23 Andrew Burgess + * frame.h (make_restore_selected_frame_cleanup): Declare new + function. + * frame.c (do_restore_selected_frame): New function. + (make_restore_selected_frame_cleanup): Define new function. + * stack.c (frame_info): Temporarily select frame for duration of + this function. + +2015-07-23 Andrew Burgess + * frame.c (frame_find_by_id): Also check the selected frame. 2015-07-23 Yao Qi diff --git a/gdb/frame.c b/gdb/frame.c index b442592..6d7fb82 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -2711,6 +2711,27 @@ frame_prepare_for_sniffer (struct frame_info *frame, return make_cleanup (frame_cleanup_after_sniffer, frame); } +/* Used as a clean-up to restore the selected frame. */ + +static void +do_restore_selected_frame (void *arg) +{ + struct frame_info *fi = (struct frame_info *) arg; + + select_frame (fi); +} + +/* See frame.h. */ + +struct cleanup * +make_restore_selected_frame_cleanup (void) +{ + struct frame_info *fi; + + fi = get_selected_frame_if_set (); + return make_cleanup (do_restore_selected_frame, fi); +} + extern initialize_file_ftype _initialize_frame; /* -Wmissing-prototypes */ static struct cmd_list_element *set_backtrace_cmdlist; diff --git a/gdb/frame.h b/gdb/frame.h index be64c57..d90a196 100644 --- a/gdb/frame.h +++ b/gdb/frame.h @@ -814,4 +814,9 @@ extern struct frame_info *create_new_frame (CORE_ADDR base, CORE_ADDR pc); extern int frame_unwinder_is (struct frame_info *fi, const struct frame_unwind *unwinder); +/* Create a clean-up to restore the selected frame. This includes + restoring the selected frame to NULL if that is its current value. */ + +extern struct cleanup * make_restore_selected_frame_cleanup (void); + #endif /* !defined (FRAME_H) */ diff --git a/gdb/stack.c b/gdb/stack.c index 4878825..9453fea 100644 --- a/gdb/stack.c +++ b/gdb/stack.c @@ -1434,6 +1434,16 @@ frame_info (char *addr_exp, int from_tty) fi = parse_frame_specification_1 (addr_exp, "No stack.", &selected_frame_p); gdbarch = get_frame_arch (fi); + /* During the following value will be created and then displayed. + Displaying a value can require using frame_find_by_id, so we must make + sure that the frame FI is findable using frame_find_by_id. As it is + possible that the frame specification caused a new frame to be + created, one outside of the current stack trace, and different to the + currently selected frame, then the only way to ensure that we can find + this frame again later is if we temporarily select this new frame. */ + make_restore_selected_frame_cleanup (); + select_frame (fi); + /* Name of the value returned by get_frame_pc(). Per comments, "pc" is not a good name. */ if (gdbarch_pc_regnum (gdbarch) >= 0) diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 51d5937..a6e70bb 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,5 +1,10 @@ 2015-07-23 Andrew Burgess + * gdb.base/create-frame.exp: Add test for 'info frame' with + specific address. + +2015-07-23 Andrew Burgess + * gdb.mi/mi-var-frame.c: New file. * gdb.mi/mi-var-frame.exp: New file. * gdb.base/create-frame.c: New file. diff --git a/gdb/testsuite/gdb.base/create-frame.exp b/gdb/testsuite/gdb.base/create-frame.exp index 2518463..e285a4e 100644 --- a/gdb/testsuite/gdb.base/create-frame.exp +++ b/gdb/testsuite/gdb.base/create-frame.exp @@ -27,4 +27,10 @@ gdb_test "bt" "#0.*#1.*#2.*" "backtrace at breakpoint" gdb_test_no_output "select-frame 3" gdb_test "info frame" "Stack level 0, frame at 0x3:.*" \ - "info frame for created frame" + "info frame for currently selected frame, at 0x3" + +gdb_test "info frame 4" "Stack frame at 0x4:.*" \ + "info frame for specific frame, created at 0x4" + +gdb_test "info frame" "Stack level 0, frame at 0x3:.*" \ + "info frame for currently selected frame (again), at 0x3"