Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Yao Qi <yao@codesourcery.com>
To: Tom Tromey <tromey@redhat.com>
Cc: Jan Kratochvil <jan.kratochvil@redhat.com>,
	Doug Evans <dje@google.com>,	<gdb-patches@sourceware.org>
Subject: Re: [RFC] Don't allow setting register in non-innermost frame
Date: Wed, 12 Sep 2012 00:51:00 -0000	[thread overview]
Message-ID: <504FDC49.5090806@codesourcery.com> (raw)
In-Reply-To: <87ehm8a3gg.fsf@fleche.redhat.com>

On 09/12/2012 01:12 AM, Tom Tromey wrote:
> After re-reading the whole thread I wonder whether pressing forward with
> this patch is best.  IIUC this didn't arise from any user complaint but
> rather just thinking about the problem.  And, while I agree it could be
> confusing in some situations, in practice it seems that perhaps it has
> never actually bitten anybody.

I also realize that this patch becomes "unnatural" :) so I drop it.
I'll revisit it when we do have the need to fix this problem.  The test
case is still useful, as we don't have test for setting registers in
testsuite.  Is the test case OK?

-- 
Yao

gdb/testsuite

2012-09-12  Yao Qi  <yao@codesourcery.com>

	* gdb.base/set-reg.exp: New.
	* gdb.base/set-reg.c: New.
---
 gdb/testsuite/gdb.base/set-reg.c   |   40 +++++++++++
 gdb/testsuite/gdb.base/set-reg.exp |  136 ++++++++++++++++++++++++++++++++++++
 2 files changed, 176 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..86d6496
--- /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 <http://www.gnu.org/licenses/>.  */
+
+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..3353a10
--- /dev/null
+++ b/gdb/testsuite/gdb.base/set-reg.exp
@@ -0,0 +1,136 @@
+# 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 <http://www.gnu.org/licenses/>.
+
+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_no_output "set \$${register}=${reg_val}" \
+	"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_no_output "set \$${register}=${reg_val}" \
+	    "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_no_output "set \$${register}=${reg_val}" \
+	    "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
-- 
1.7.7.6



  parent reply	other threads:[~2012-09-12  0:51 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-17  2:21 Yao Qi
2012-08-20 20:19 ` Doug Evans
2012-08-21  3:27   ` Yao Qi
2012-08-23 16:25   ` Tom Tromey
2012-08-29  9:51     ` Yao Qi
2012-09-04 22:37       ` dje
2012-09-07 10:01         ` Yao Qi
2012-09-07 10:11           ` Eli Zaretskii
2012-09-07 10:21             ` Yao Qi
2012-09-07 11:27               ` Eli Zaretskii
2012-09-07 13:14                 ` Yao Qi
2012-09-07 14:32                   ` Eli Zaretskii
2012-09-07 16:46     ` Jan Kratochvil
2012-09-09  2:31       ` Yao Qi
2012-09-10  2:02       ` Yao Qi
2012-09-10  7:47         ` Jan Kratochvil
2012-09-10 19:43           ` Jan Kratochvil
2012-09-11 17:12         ` Tom Tromey
2012-09-11 17:19           ` Jan Kratochvil
2012-09-11 17:23             ` Tom Tromey
2012-09-12  0:51           ` Yao Qi [this message]
2012-09-12 13:19             ` Jan Kratochvil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=504FDC49.5090806@codesourcery.com \
    --to=yao@codesourcery.com \
    --cc=dje@google.com \
    --cc=gdb-patches@sourceware.org \
    --cc=jan.kratochvil@redhat.com \
    --cc=tromey@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox