From patchwork Fri Sep 11 18:49:35 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 8648 Received: (qmail 54567 invoked by alias); 11 Sep 2015 18:49: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 54533 invoked by uid 89); 11 Sep 2015 18:49:59 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.1 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-f173.google.com Received: from mail-wi0-f173.google.com (HELO mail-wi0-f173.google.com) (209.85.212.173) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 11 Sep 2015 18:49:52 +0000 Received: by wiclk2 with SMTP id lk2so74637347wic.0 for ; Fri, 11 Sep 2015 11:49:48 -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=jG1d2hDQuRb6rYln1rchOoUm79JjmOKB4aF/iUjUwNk=; b=gQbViXlSIRPF7mBD618dOGQ+BZ1stMFIZE3tBsEObPzJwQbp0aeEzSS83QVU4DZCDn s8Ptgnh3lK5+Mug7XLBDpwhfaeJTiqTBdXSn0G//AY7pB2LeOBDluFoM68vXXf2mlyZV 3LpkuiVzBAXwpkCMwbVjonE3S1NpQq0YxqltoK0Rhah5jkkGuYRrZAJzEhDlS4YXtVpZ d/3m2F1x1VMejm/Fwyprr2WELTNPLPKIpo3LJVtZP1jzSlby2cepP/4OBBr6VNKypy3m Lh0dEg1Wly+CXBGr2pAE2hiXYIvue0JTo3UmodqRN7oiDwtVVSp5RAs1VO5JU3n6gy9+ FZVQ== X-Gm-Message-State: ALoCoQk2d6oUsRnPP9PbLVR90YJEiCIcAOyUX8gR86DKTN9o9oP3g+nvz2U4rb878vOAZmMUrnrG X-Received: by 10.194.22.135 with SMTP id d7mr483996wjf.78.1441997388481; Fri, 11 Sep 2015 11:49:48 -0700 (PDT) Received: from localhost (host81-131-206-173.range81-131.btcentralplus.com. [81.131.206.173]) by smtp.gmail.com with ESMTPSA id fn8sm479503wib.2.2015.09.11.11.49.47 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 11 Sep 2015 11:49:47 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 1/9] gdb: Check the selected-frame in frame_find_by_id. Date: Fri, 11 Sep 2015 19:49:35 +0100 Message-Id: <3544270eb01ddce088cccf4a2f708af4ec4dbc77.1441996064.git.andrew.burgess@embecosm.com> In-Reply-To: References: In-Reply-To: References: X-IsSubscribed: yes In gdb we track two different things, the current-frame, and the selected-frame. The current-frame represents the frame in which we are currently stopped. If gdb stops at a breakpoint and the user asks for a backtrace then all the frames in the backtrace will be the ancestors of the the current-frame. Also the current-frame does not change until the inferior is set running again (though it is possible for the user to alter the contents of the current-frame this is not the same as changing the current-frame). There is also the selected-frame. This is the frame that the user has "selected" using the frame selection commands 'frame', 'up', or 'down' being the most common. Usually the selected-frame is always an ancestor of the current-frame, however, there is a feature of the 'frame' command that allows arbitrary frames to be created, in this case the selected-frame might not be an ancestor of the current-frame. The problem this causes is that lazy register values hold the frame-id for the frame in which the real register value can be obtained. When gdb needs to fetch the actual register value we try to find the frame using frame_find_by_id, which currently looks at the current-frame, and all the ancestors of the current-frame. When the selected-frame is not an ancestor of the current-frame then frame_find_by_id will fail to find the required frame, and gdb will fail with an assertion while trying to fetch the lazy value (the assertion is that the frame will always be found). This patch extends frame_find_by_id to also check the selected-frame as a separate comparison as well as the current-frame and its ancestors. This resolves the issue. There are also two new tests added that show this issue. The first is an mi test using var objects, which is where I first saw this issue. The second is a slightly simpler test, just creating a new frame then calling 'info frame' is enough to trigger the assertion. gdb/ChangeLog: * frame.c (frame_find_by_id): Also check the selected frame. gdb/testsuite/ChangeLog: * gdb.mi/mi-var-frame.c: New file. * gdb.mi/mi-var-frame.exp: New file. * gdb.base/frame-cmds.c: New file. * gdb.base/frame-cmds.exp: New file. --- gdb/ChangeLog | 4 ++ gdb/frame.c | 18 +++++++-- gdb/testsuite/ChangeLog | 7 ++++ gdb/testsuite/gdb.base/frame-cmds.c | 34 +++++++++++++++++ gdb/testsuite/gdb.base/frame-cmds.exp | 30 +++++++++++++++ gdb/testsuite/gdb.mi/mi-var-frame.c | 34 +++++++++++++++++ gdb/testsuite/gdb.mi/mi-var-frame.exp | 70 +++++++++++++++++++++++++++++++++++ 7 files changed, 194 insertions(+), 3 deletions(-) create mode 100644 gdb/testsuite/gdb.base/frame-cmds.c create mode 100644 gdb/testsuite/gdb.base/frame-cmds.exp create mode 100644 gdb/testsuite/gdb.mi/mi-var-frame.c create mode 100644 gdb/testsuite/gdb.mi/mi-var-frame.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index d7a8d31..b4fe9cc 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,7 @@ +2015-09-11 Andrew Burgess + + * frame.c (frame_find_by_id): Also check the selected frame. + 2015-09-11 Pierre Langlois * aarch64-tdep.c (decode_cb): Move up comment describing the diff --git a/gdb/frame.c b/gdb/frame.c index 745e007..114fa6a 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -732,7 +732,7 @@ frame_id_inner (struct gdbarch *gdbarch, struct frame_id l, struct frame_id r) struct frame_info * frame_find_by_id (struct frame_id id) { - struct frame_info *frame, *prev_frame; + struct frame_info *frame, *prev_frame, *selected_frame; /* ZERO denotes the null frame, let the caller decide what to do about it. Should it instead return get_current_frame()? */ @@ -761,7 +761,7 @@ frame_find_by_id (struct frame_id id) prev_frame = get_prev_frame (frame); if (!prev_frame) - return NULL; + break; /* As a safety net to avoid unnecessary backtracing while trying to find an invalid ID, we check for a common situation where @@ -772,8 +772,20 @@ frame_find_by_id (struct frame_id id) && !frame_id_inner (get_frame_arch (frame), id, self) && frame_id_inner (get_frame_arch (prev_frame), id, get_frame_id (prev_frame))) - return NULL; + break; + } + + /* We check the selected frame specifically here to handle the case where + the selected frame is not an ancestor of the current frame. */ + selected_frame = get_selected_frame_if_set (); + if (selected_frame) + { + struct frame_id selected_frame_id = get_frame_id (selected_frame); + + if (frame_id_eq (id, selected_frame_id)) + return frame; } + return NULL; } diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 1538d15..cf83b3a 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2015-09-11 Andrew Burgess + + * gdb.mi/mi-var-frame.c: New file. + * gdb.mi/mi-var-frame.exp: New file. + * gdb.base/frame-cmds.c: New file. + * gdb.base/frame-cmds.exp: New file. + 2015-09-09 Doug Evans * gdb.python/py-prettyprint.exp: Check result of run_lang_tests. diff --git a/gdb/testsuite/gdb.base/frame-cmds.c b/gdb/testsuite/gdb.base/frame-cmds.c new file mode 100644 index 0000000..53b222d --- /dev/null +++ b/gdb/testsuite/gdb.base/frame-cmds.c @@ -0,0 +1,34 @@ +/* Copyright 2015 Free Software Foundation, Inc. + + This file is part of GDB. + + 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 . */ + +int +frame_2 (void) +{ + return 0; +} + +int +frame_1 (void) +{ + return frame_2 (); +} + +int +main (void) +{ + return frame_1 (); +} diff --git a/gdb/testsuite/gdb.base/frame-cmds.exp b/gdb/testsuite/gdb.base/frame-cmds.exp new file mode 100644 index 0000000..2518463 --- /dev/null +++ b/gdb/testsuite/gdb.base/frame-cmds.exp @@ -0,0 +1,30 @@ +# Copyright 2015 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 . + +if { [prepare_for_testing create-frame.exp "create-frame" {create-frame.c} {debug nowarnings}] } { + return -1 +} +set srcfile create-frame.c + +runto_main +gdb_breakpoint frame_2 +gdb_continue_to_breakpoint frame_2 + +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" diff --git a/gdb/testsuite/gdb.mi/mi-var-frame.c b/gdb/testsuite/gdb.mi/mi-var-frame.c new file mode 100644 index 0000000..53b222d --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-var-frame.c @@ -0,0 +1,34 @@ +/* Copyright 2015 Free Software Foundation, Inc. + + This file is part of GDB. + + 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 . */ + +int +frame_2 (void) +{ + return 0; +} + +int +frame_1 (void) +{ + return frame_2 (); +} + +int +main (void) +{ + return frame_1 (); +} diff --git a/gdb/testsuite/gdb.mi/mi-var-frame.exp b/gdb/testsuite/gdb.mi/mi-var-frame.exp new file mode 100644 index 0000000..5c3f196 --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-var-frame.exp @@ -0,0 +1,70 @@ +# Copyright 2015 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 tests an issue relating to updating var objects in user created +# frames. + +# This test requires a register name for creating a var object. +# Currently only x86/x86_64 like targets are supported. +if {![istarget "x86_64-*-*"] && ![istarget "i?86-*-*"]} { + return 0 +} + +load_lib mi-support.exp +set MIFLAGS "-i=mi" + +gdb_exit +if [mi_gdb_start] { + continue +} + +standard_testfile mi-var-frame.c + +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { + untested mi-var-frame.exp + return -1 +} + +mi_delete_breakpoints +mi_gdb_reinitialize_dir $srcdir/$subdir +mi_gdb_load ${binfile} + +mi_create_breakpoint "frame_2" \ + "break-insert into frame_2" \ + -number 1 -func frame_2 -file ".*${srcfile}" + +mi_run_cmd +mi_expect_stop "breakpoint-hit" "frame_2" "" ".*${srcfile}" \ + ".*" { "" "disp=\"keep\"" } "run to breakpoint" + +mi_gdb_test "-stack-info-depth" \ + "\\^done,depth=\"3\"" \ + "stack info-depth" + +set register "\$eax" + +mi_gdb_test "-var-create R * $register" \ + "\\^done,name=\"R\",numchild=\"0\",value=\".*\",type=\".*\",has_more=\"0\"" \ + "create varobj 'R' for register '$register'" + +for {set i 0} {$i < 5} {incr i} { + mi_gdb_test "-stack-select-frame $i" \ + {\^done} \ + "-stack-select-frame $i" + + mi_gdb_test "-var-update R" \ + "\\^done,changelist=\\\[\\\]" \ + "update 'R' in frame $i: no change" +}