From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31109 invoked by alias); 17 Aug 2012 02:21:23 -0000 Received: (qmail 30800 invoked by uid 22791); 17 Aug 2012 02:21:21 -0000 X-SWARE-Spam-Status: No, hits=-3.6 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,TW_EG X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 17 Aug 2012 02:21:06 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1T2CBN-00018b-BQ from Yao_Qi@mentor.com for gdb-patches@sourceware.org; Thu, 16 Aug 2012 19:21:05 -0700 Received: from SVR-ORW-FEM-03.mgc.mentorg.com ([147.34.97.39]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Thu, 16 Aug 2012 19:21:05 -0700 Received: from qiyao.dyndns.org.dyndns.org (147.34.91.1) by svr-orw-fem-03.mgc.mentorg.com (147.34.97.39) with Microsoft SMTP Server id 14.1.289.1; Thu, 16 Aug 2012 19:21:03 -0700 From: Yao Qi To: Subject: [RFC] Don't allow setting register in non-innermost frame Date: Fri, 17 Aug 2012 02:21:00 -0000 Message-ID: <1345170040-25959-1-git-send-email-yao@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-08/txt/msg00469.txt.bz2 Hi, When playing with 'set $reg=0xXXX', I am thinking about the expected behaviour of setting register when the frame is not innermost. Current GDB will invalidate regcaches and reinit frache_caches for any register changes (on innermost and non-innermost frame). However, the semantics of 'setting register on non-innermost frame' is tricky. On innermost frame, 'setting register' means writing something to register in cpu, while on non-innermost frame, it means writing something to either real register (regcache) or memory (frame). On the other head, I am trying to figure out a situation that user has to set register on non-innermost frame. Why does this user have to do that? Literally, IMO, 'set $reg=0xXXXX' means 'setting the cpu register $reg to value 0xXXX', which is simple and clear. I propose to restrict setting register in innermost frame only. WDYT? Regression tested on x86_64-linux. Surprisingly, we don't have a test case for 'set register', so I add one. In the future, I plan to add more tests here for 'set register', such as 'set some bits of a register' and 'set multiple registers in one time', to cover the different paths in value_assign for 'lval_register'. gdb: * valops.c (value_assign): Emit error if get_next_frame returns non-NULL. gdb/testsuite: * gdb.base/set-reg.exp: New. --- gdb/testsuite/gdb.base/set-reg.c | 40 +++++++++++++ gdb/testsuite/gdb.base/set-reg.exp | 108 ++++++++++++++++++++++++++++++++++++ gdb/valops.c | 12 ++++ 3 files changed, 160 insertions(+), 0 deletions(-) create mode 100644 gdb/testsuite/gdb.base/set-reg.c create mode 100644 gdb/testsuite/gdb.base/set-reg.exp diff --git a/gdb/testsuite/gdb.base/set-reg.c b/gdb/testsuite/gdb.base/set-reg.c new file mode 100644 index 0000000..7ce350d --- /dev/null +++ b/gdb/testsuite/gdb.base/set-reg.c @@ -0,0 +1,40 @@ +/* This test is part of GDB, the GNU debugger. + + Copyright 2012 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 . */ + +int +inner (int x) +{ + return x + 2; +} + +static short +middle (int x) +{ + return 2 * inner (x); +} + +short +top (int x) +{ + return 2 * middle (x); +} + +int +main (int argc, char **argv) +{ + return top (argc); +} diff --git a/gdb/testsuite/gdb.base/set-reg.exp b/gdb/testsuite/gdb.base/set-reg.exp new file mode 100644 index 0000000..abe7764 --- /dev/null +++ b/gdb/testsuite/gdb.base/set-reg.exp @@ -0,0 +1,108 @@ +# Copyright 2012 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 . + +standard_testfile +set executable $testfile + +if { [gdb_compile $srcdir/$subdir/$srcfile $binfile executable {debug}] != "" } { + untested "Failed to compile" + return -1 +} + +# A value in register in previous frame. +set reg_prev_frame_in_reg "unknown" +# A value in memory in previous frame. +set reg_prev_frame_in_memory "unknown" + +if [is_amd64_regs_target] { + set reg_prev_frame_in_reg "rax" + set reg_prev_frame_in_memory "rbp" +} elseif [is_x86_like_target] { + set reg_prev_frame_in_reg "eax" + set reg_prev_frame_in_memory "ebp" +} else { + # set reg_prev_frame_in_reg for other arch. + +} + +# Test setting register on innermost frame and non-innermost frame. + +proc test_set_reg_on_frame {} {with_test_prefix "on frame" { + global executable + global reg_prev_frame_in_reg + global reg_prev_frame_in_memory + global decimal + global hex + global expect_out + global gdb_prompt + + clean_restart $executable + + if ![runto_main] { + return -1 + } + + # Name of register is not set, skip the rest of test. + if { [string equal $reg_prev_frame_in_reg "unknown"] } { + return 1 + } + + gdb_test "break inner" "Breakpoint.*at.* file .*, line.*" + gdb_test "continue" "Continuing.\r\n\r\nBreakpoint.*inner.*at.*" \ + "continue to inner" + + gdb_test_sequence "bt" "bt before setting reg" { + "\[\r\n\]#0 .* inner \\(x=1\\) at " + "\[\r\n\]#1 .* middle \\(x=1\\) at " + "\[\r\n\]#2 .* top \\(x=1\\) at " + "\[\r\n\]#3 .* main \\(.*\\) at" + } + + set reg_val "" + gdb_test_multiple "p/x \$${reg_prev_frame_in_reg}" "print register ${reg_prev_frame_in_reg}" { + -re ".*${decimal} = ($hex).*$gdb_prompt $" { + set reg_val $expect_out(1,string) + } + } + + gdb_test_no_output "set \$${reg_prev_frame_in_reg}=${reg_val}" \ + "set register ${reg_prev_frame_in_reg} on inner frame" + + # Stack is unchanged. + gdb_test_sequence "bt" "bt after setting reg" { + "\[\r\n\]#0 .* inner \\(x=1\\) at " + "\[\r\n\]#1 .* middle \\(x=1\\) at " + "\[\r\n\]#2 .* top \\(x=1\\) at " + "\[\r\n\]#3 .* main \\(.*\\) at" + } + + gdb_test "up" + gdb_test "set \$${reg_prev_frame_in_reg}=${reg_val}" \ + "Frame in which this value is assigned to is not innermost.*" \ + "set register ${reg_prev_frame_in_reg} on non-inner frame" + + gdb_test "set \$${reg_prev_frame_in_memory}=${reg_val}" \ + "Frame in which this value is assigned to is not innermost.*" \ + "set register ${reg_prev_frame_in_memory} on non-inner frame" + + gdb_test_sequence "bt" "bt after up" { + "\[\r\n\]#0 .* inner \\(x=1\\) at " + "\[\r\n\]#1 .* middle \\(x=1\\) at " + "\[\r\n\]#2 .* top \\(x=1\\) at " + "\[\r\n\]#3 .* main \\(.*\\) at" + } +}} + +test_set_reg_on_frame diff --git a/gdb/valops.c b/gdb/valops.c index 8167cd4..14540b8 100644 --- a/gdb/valops.c +++ b/gdb/valops.c @@ -1241,6 +1241,18 @@ value_assign (struct value *toval, struct value *fromval) and then restore the new frame afterwards. */ old_frame = get_frame_id (deprecated_safe_get_selected_frame ()); + /* TOVAL is a register, although VALUE_LVAL(TOVAL) may not be + lval_register. */ + if (VALUE_REGNUM (toval) >= 0) + { + /* Figure out which frame this is in currently. */ + struct frame_info *frame = frame_find_by_id (VALUE_FRAME_ID (toval)); + + if (get_next_frame (frame) != NULL) + error (_("Frame in which this value is assigned to is" + " not innermost.")); + } + switch (VALUE_LVAL (toval)) { case lval_internalvar: -- 1.7.7.6