From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6296 invoked by alias); 10 Sep 2012 02:02:10 -0000 Received: (qmail 5999 invoked by uid 22791); 10 Sep 2012 02:02:07 -0000 X-SWARE-Spam-Status: No, hits=-4.4 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,TW_BM 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; Mon, 10 Sep 2012 02:01:51 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1TAtJs-0002a6-Oy from Yao_Qi@mentor.com ; Sun, 09 Sep 2012 19:01:48 -0700 Received: from SVR-ORW-FEM-04.mgc.mentorg.com ([147.34.97.41]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Sun, 9 Sep 2012 19:01:48 -0700 Received: from qiyao.dyndns.org (147.34.91.1) by svr-orw-fem-04.mgc.mentorg.com (147.34.97.41) with Microsoft SMTP Server id 14.1.289.1; Sun, 9 Sep 2012 19:01:47 -0700 Message-ID: <504D49DA.6070006@codesourcery.com> Date: Mon, 10 Sep 2012 02:02:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:14.0) Gecko/20120717 Thunderbird/14.0 MIME-Version: 1.0 To: Jan Kratochvil CC: Tom Tromey , Doug Evans , Subject: Re: [RFC] Don't allow setting register in non-innermost frame References: <1345170040-25959-1-git-send-email-yao@codesourcery.com> <87hartpodt.fsf@fleche.redhat.com> <20120907164544.GA18234@host2.jankratochvil.net> In-Reply-To: <20120907164544.GA18234@host2.jankratochvil.net> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit 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-09/txt/msg00099.txt.bz2 On 09/08/2012 12:45 AM, Jan Kratochvil wrote: > (gdb) set variable i=20 > warning: Assigning to register in non-innermost frame. > > Why? > > I would find correct to print such warning for: > (gdb) set $rbx=20 > There it should happen only if user has explicitly specified the register > itself. I checked the difference of value of register and value of variable (saved in register), TYPE_OBJFILE_OWNED (value_type (toval)) is the only difference between them. IIUC, type of variable is owned by an objfile, while type of register is not (alloc_type vs. alloc_type_arch), so looks we can use it to differentiate values in register and values in variables. This new patch includes TYPE_OBJFILE_OWNED into the condition, and adjust the NEWS entry discussed with Eli before. -- Yao gdb/testsuite: 2012-09-10 Yao Qi * gdb.base/set-reg.exp: New. * gdb.base/set-reg.c: New. gdb: 2012-09-10 Yao Qi Doug Evans * valops.c (value_assign): Emit warning when assigning to registers in non-innermost frames. * NEWS: Mention it. --- gdb/NEWS | 3 + gdb/testsuite/gdb.base/set-reg.c | 40 ++++++++++ gdb/testsuite/gdb.base/set-reg.exp | 139 ++++++++++++++++++++++++++++++++++++ gdb/valops.c | 16 ++++ 4 files changed, 198 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/NEWS b/gdb/NEWS index dba6937..f7476e4 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -17,6 +17,9 @@ * The 'cd' command now defaults to using '~' (the home directory) if not given an argument. +* GDB will display a warning when assigning to registers by CLI command + "set" or MI command "-var-assign" in non-innermost frames. + * New configure options --enable-libmcheck/--disable-libmcheck diff --git a/gdb/testsuite/gdb.base/set-reg.c b/gdb/testsuite/gdb.base/set-reg.c new file mode 100644 index 0000000..03938eb --- /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 . */ + +static int +inner (int x) +{ + return x + 2; +} + +static int +middle (int x) +{ + return 2 * inner (x); +} + +static int +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..5ea65da --- /dev/null +++ b/gdb/testsuite/gdb.base/set-reg.exp @@ -0,0 +1,139 @@ +# 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 + +# A value in register in previous frame. E.g,. call-clobbered register. +set reg_prev_frame_in_reg "unknown" +# A (register) value in memory in previous frame. E.g., call-saved register. +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. +} + +# Name of register is not set, skip the rest of test. +if { [string equal $reg_prev_frame_in_reg "unknown"] } { + return +} + +if { [gdb_compile $srcdir/$subdir/$srcfile $binfile executable {debug}] != "" } { + untested "Failed to compile" + return -1 +} + +proc set_register { register check_value } {with_test_prefix "${register} ${check_value}" { + global decimal + global gdb_prompt + + set reg_val "" + gdb_test_multiple "p/d \$${register}" "print register ${register}" { + -re ".*${decimal} = ($decimal).*$gdb_prompt $" { + set reg_val $expect_out(1,string) + } + } + if { $reg_val == "" } { + fail "get the value of register ${register}" + return + } + + # Assign the same value to register. + gdb_test_no_output "set \$${register}=${reg_val}" \ + "set register ${register} on inner frame (unchanged)" + + # Assign the different value to register. + set reg_val [expr $reg_val + 4] + gdb_test_no_output "set \$${register}=${reg_val}" \ + "set register ${register} on inner frame (changed)" + gdb_test "p/d \$${register}" ".*${decimal} = $reg_val.*" \ + "register ${register} is changed" + + # Restore register. + set reg_val [expr $reg_val - 4] + gdb_test_no_output "set \$${register}=${reg_val}" \ + "set register ${register} on inner frame (restored)" + gdb_test "p/d \$${register}" ".*${decimal} = $reg_val.*" \ + "register ${register} is restored" + + gdb_test "up" + + gdb_test_multiple "p/d \$${register}" "print register ${register}" { + -re ".*${decimal} = ($decimal).*$gdb_prompt $" { + set reg_val $expect_out(1,string) + } + } + if { $reg_val == "" } { + fail "get the value of register ${register}" + return + } + # Assign the same value to register. + gdb_test "set \$${register}=${reg_val}" \ + "Assigning to register in non-innermost frame.*" \ + "set register ${register} on non-inner frame" + + if { $check_value } { + # Assign the different value to register. + set reg_val [expr $reg_val + 4] + gdb_test "set \$${register}=${reg_val}" \ + "Assigning to register in non-innermost frame.*" \ + "set register ${register} on non-inner frame (changed)" + gdb_test "p/d \$${register}" ".*${decimal} = $reg_val.*" \ + "register ${register} is changed" + # Restore register. + set reg_val [expr $reg_val - 4] + gdb_test "set \$${register}=${reg_val}" \ + "Assigning to register in non-innermost frame.*" \ + "set register ${register} on non-inner frame (restored)" + gdb_test "p/d \$${register}" ".*${decimal} = $reg_val.*" \ + "register ${register} is restored" + } + gdb_test "down" +}} + +# 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 + } + + gdb_test "break inner" "Breakpoint.*at.* file .*, line.*" + gdb_test "continue" "Continuing.\r\n\r\nBreakpoint.*inner.*at.*" \ + "continue to inner" + + set_register ${reg_prev_frame_in_reg} 1 + # Change the register saved on frame may clobber the frame, and get + # the contents of registers in mess. Don't check register value. + set_register ${reg_prev_frame_in_memory} 0 +}} + +test_set_reg_on_frame diff --git a/gdb/valops.c b/gdb/valops.c index 17696ee..7ac23e3 100644 --- a/gdb/valops.c +++ b/gdb/valops.c @@ -1241,6 +1241,22 @@ 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. A call-saved register saved in memory will have + 'VALUE_REGNUM >= 0' but 'VALUE_LVAL == lval_memory'. We also have to + avoid emitting warning when assign value to some local variables which + are stored in registers, TYPE_OBJFILE_OWNED helps to differentiate + we are assigning to a register explicitly or to a variable saved in + register. */ + if (VALUE_REGNUM (toval) >= 0 && !TYPE_OBJFILE_OWNED (type)) + { + /* 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) + warning (_("Assigning to register in non-innermost frame.")); + } + switch (VALUE_LVAL (toval)) { case lval_internalvar: -- 1.7.7.6