From: will schmidt <will_schmidt@vnet.ibm.com>
To: gdb-patches@sourceware.org
Cc: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>,
rogerio <rogealve@br.ibm.com>
Subject: [PATCH,rs6000,v3] gdb-power10-single-step
Date: Mon, 29 Mar 2021 17:43:53 -0500 [thread overview]
Message-ID: <896b57a450849aa1b45408b74f927d2ebd6833a3.camel@vnet.ibm.com> (raw)
In-Reply-To: <2abf296edd936056d4e8932049fac751f0ae7152.camel@vnet.ibm.com>
Hi,
This is V3 of the patch that enables power10 single-stepping.
[v1]
> This is a patch written by Alan Modra. With his permission
> I'm submitting this for review and helping get this upstream.
>
> Powerpc / Power10 ISA 3.1 adds prefixed instructions, which
> are 8 bytes in length. This is in contrast to powerpc previously
> always having 4 byte instruction length. This patch implements
> changes to allow GDB to better detect prefixed instructions, and
> handle single stepping across the 8 byte instructions.
[v2]
Multiple updates, changes and additions based on feedback and
subsequent work to get breakpoints to behave on prefixed and
pc-relative instructions. Including..
Added #defines to help test for PNOP and prefix instructions.
Update ppc_displaced_step_copy_insn() to handle pnop and prefixed
instructions whem R=0 (non-pc-relative).
Update ppc_displaced_step_fixup() to properly handle the offset
value matching the current instruction size
Update the for-loop within ppc_deal_with_atomic_sequence() to
count instructions properly in case we have a mix of 4-byte and
8-byte instructions within the atomic_sequence_length.
Added testcase and harness to exercise pc-relative load/store
instructions with R=0.
[v3]
Assorted updates per review feedback. Fixed ChangeLog. Removed
some extraneous debug statements. Used -wrap in testcase harnesses.
YYYY-MM-DD Will Schmidt <will_schmidt@vnet.ibm.com>
gdb/ChangeLog:
* rs6000-tdep.c: Add support for single-stepping of
prefixed instructions.
gdb/testsuite/ChangeLog:
* gdb.arch/powerpc-plxv-nonrel.s: Testcase using
non-relative plxv instructions.
* /gdb.arch/powerpc-plxv-nonrel.exp: Testcase harness.
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index cbb3e013ad9..8861fd5d0f2 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -839,11 +839,11 @@ constexpr gdb_byte little_breakpoint[] = { 0x08, 0x10, 0x82, 0x7d };
typedef BP_MANIPULATION_ENDIAN (little_breakpoint, big_breakpoint)
rs6000_breakpoint;
/* Instruction masks for displaced stepping. */
-#define BRANCH_MASK 0xfc000000
+#define OP_MASK 0xfc000000
#define BP_MASK 0xFC0007FE
#define B_INSN 0x48000000
#define BC_INSN 0x40000000
#define BXL_INSN 0x4c000000
#define BP_INSN 0x7C000008
@@ -867,10 +867,15 @@ typedef BP_MANIPULATION_ENDIAN (little_breakpoint, big_breakpoint)
#define ADDPCIS_INSN 0x4c000004
#define ADDPCIS_INSN_MASK 0xfc00003e
#define ADDPCIS_TARGET_REGISTER 0x03F00000
#define ADDPCIS_INSN_REGSHIFT 21
+#define PNOP_MASK 0xfff3ffff
+#define PNOP_INSN 0x07000000
+#define R_MASK 0x00100000
+#define R_ZERO 0x00000000
+
/* Check if insn is one of the Load And Reserve instructions used for atomic
sequences. */
#define IS_LOAD_AND_RESERVE_INSN(insn) ((insn & LOAD_AND_RESERVE_MASK) == LWARX_INSTRUCTION \
|| (insn & LOAD_AND_RESERVE_MASK) == LDARX_INSTRUCTION \
|| (insn & LOAD_AND_RESERVE_MASK) == LBARX_INSTRUCTION \
@@ -899,14 +904,40 @@ ppc_displaced_step_copy_insn (struct gdbarch *gdbarch,
(new ppc_displaced_step_copy_insn_closure (len));
gdb_byte *buf = closure->buf.data ();
enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
int insn;
- read_memory (from, buf, len);
+ len = target_read (current_inferior()->top_target(), TARGET_OBJECT_MEMORY, NULL,
+ buf, from, len);
+ if ((ssize_t) len < PPC_INSN_SIZE)
+ memory_error (TARGET_XFER_E_IO, from);
insn = extract_signed_integer (buf, PPC_INSN_SIZE, byte_order);
+ /* Check for PNOP and for prefixed instructions with R=0. Those
+ instructions are safe to displace. Prefixed instructions with R=1
+ will read/write data to/from locations relative to the current PC.
+ We would not be able to fixup after an instruction has written data
+ into a displaced location, so decline to displace those instructions. */
+ if ((insn & OP_MASK) == 1 << 26)
+ {
+ if (((insn & PNOP_MASK) != PNOP_INSN) &&
+ ((insn & R_MASK) != R_ZERO))
+ {
+ displaced_debug_printf ("Not displacing prefixed instruction %08x at %s",
+ insn, paddress (gdbarch, from));
+ return NULL;
+ }
+ }
+ else
+ /* Non-prefixed instructions.. */
+ {
+ /* Set the instruction length to 4 to match the actual instruction
+ length. */
+ len = 4;
+ }
+
/* Assume all atomic sequences start with a Load and Reserve instruction. */
if (IS_LOAD_AND_RESERVE_INSN (insn))
{
displaced_debug_printf ("can't displaced step atomic sequence at %s",
paddress (gdbarch, from));
@@ -916,11 +947,11 @@ ppc_displaced_step_copy_insn (struct gdbarch *gdbarch,
write_memory (to, buf, len);
displaced_debug_printf ("copy %s->%s: %s",
paddress (gdbarch, from), paddress (gdbarch, to),
- displaced_step_dump_bytes (buf, len).c_str ());;
+ displaced_step_dump_bytes (buf, len).c_str ());
/* This is a work around for a problem with g++ 4.8. */
return displaced_step_copy_insn_closure_up (closure.release ());
}
@@ -936,15 +967,21 @@ ppc_displaced_step_fixup (struct gdbarch *gdbarch,
/* Our closure is a copy of the instruction. */
ppc_displaced_step_copy_insn_closure *closure
= (ppc_displaced_step_copy_insn_closure *) closure_;
ULONGEST insn = extract_unsigned_integer (closure->buf.data (),
PPC_INSN_SIZE, byte_order);
- ULONGEST opcode = 0;
+ ULONGEST opcode;
/* Offset for non PC-relative instructions. */
- LONGEST offset = PPC_INSN_SIZE;
+ LONGEST offset;
- opcode = insn & BRANCH_MASK;
+ opcode = insn & OP_MASK;
+
+ /* set offset to 8 if this is an 8-byte (prefixed) instruction. */
+ if ((opcode) == 1 << 26)
+ offset = 2 * PPC_INSN_SIZE;
+ else
+ offset = PPC_INSN_SIZE;
displaced_debug_printf ("(ppc) fixup (%s, %s)",
paddress (gdbarch, from), paddress (gdbarch, to));
/* Handle the addpcis/lnia instruction. */
@@ -1112,17 +1149,21 @@ ppc_deal_with_atomic_sequence (struct regcache *regcache)
/* Assume that no atomic sequence is longer than "atomic_sequence_length"
instructions. */
for (insn_count = 0; insn_count < atomic_sequence_length; ++insn_count)
{
- loc += PPC_INSN_SIZE;
+ int cur_insn = read_memory_integer (loc, PPC_INSN_SIZE, byte_order);
+ if ((cur_insn & OP_MASK) == 1 << 26)
+ loc += 2*PPC_INSN_SIZE;
+ else
+ loc += PPC_INSN_SIZE;
insn = read_memory_integer (loc, PPC_INSN_SIZE, byte_order);
/* Assume that there is at most one conditional branch in the atomic
sequence. If a conditional branch is found, put a breakpoint in
its destination address. */
- if ((insn & BRANCH_MASK) == BC_INSN)
+ if ((insn & OP_MASK) == BC_INSN)
{
int immediate = ((insn & 0xfffc) ^ 0x8000) - 0x8000;
int absolute = insn & 2;
if (bc_insn_count >= 1)
@@ -7094,11 +7135,11 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
set_gdbarch_displaced_step_prepare (gdbarch, ppc_displaced_step_prepare);
set_gdbarch_displaced_step_finish (gdbarch, ppc_displaced_step_finish);
set_gdbarch_displaced_step_restore_all_in_ptid
(gdbarch, ppc_displaced_step_restore_all_in_ptid);
- set_gdbarch_max_insn_length (gdbarch, PPC_INSN_SIZE);
+ set_gdbarch_max_insn_length (gdbarch, 2 * PPC_INSN_SIZE);
/* Hook in ABI-specific overrides, if they have been registered. */
info.target_desc = tdesc;
info.tdesc_data = tdesc_data.get ();
gdbarch_init_osabi (info, gdbarch);
diff --git a/gdb/testsuite/gdb.arch/powerpc-plxv-nonrel.exp b/gdb/testsuite/gdb.arch/powerpc-plxv-nonrel.exp
new file mode 100644
index 00000000000..08f1a379efb
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/powerpc-plxv-nonrel.exp
@@ -0,0 +1,131 @@
+# Copyright 2021 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/>.
+
+# Test to see if gdb is properly single stepping over the
+# displaced plxv instruction.
+
+if { ![istarget powerpc*-*] } {
+ verbose "Skipping powerpc plxv test."
+ return
+}
+
+set retval 0
+
+standard_testfile .s
+
+if { [prepare_for_testing "failed to prepare" $testfile "$srcfile" \
+ {debug quiet}] } {
+ return -1
+}
+
+gdb_test "set radix 0b10000"
+gdb_test "set debug displaced"
+
+if ![runto_main] then {
+ return
+}
+
+gdb_test "set debug displaced on"
+
+# Proc to extract the uint128 hex value from the output of
+# a print vector statement.
+proc get_vector_hexadecimal_valueof { exp default {test ""} } {
+ set val "0x0000"
+ global gdb_prompt
+ if {$test == ""} {
+ set test "get vector_hexadecimal valueof \"${exp}\""
+ }
+ gdb_test_multiple "print $${exp}.uint128" $test {
+ -re -wrap "\\$\[0-9\]* = (0x\[0-9a-zA-Z\]+).*" {
+ set val $expect_out(1,string)
+ pass "$test"
+ }
+ -re -wrap ".*Illegal instruction.* $" {
+ fail "Illegal instruction on print."
+ set val 0xffff
+ }
+ }
+ return ${val}
+}
+
+# Proc to do a single-step, and ensure we gently handle
+# an illegal instruction situation.
+proc stepi_over_instruction { xyz } {
+ global gdb_prompt
+ gdb_test_multiple "stepi" "${xyz} " {
+ -re -wrap ".*Illegal instruction.*" {
+ fail "Illegal instruction on single step."
+ return
+ }
+ -re -wrap ".*" {
+ pass "stepi ${xyz}"
+ }
+ }
+}
+
+set check_pc [get_hexadecimal_valueof "\$pc" "default0"]
+
+# set some breakpoints on the instructions below main().
+gdb_test "disas /r main"
+set bp1 *$check_pc+4
+set bp2 *$check_pc+0d12
+set bp3 *$check_pc+0d20
+set bp4 *$check_pc+0d28
+gdb_breakpoint $bp1
+gdb_breakpoint $bp2
+gdb_breakpoint $bp3
+gdb_breakpoint $bp4
+
+# single-step through the plxv instructions, and retrieve the
+# register values as we proceed.
+
+stepi_over_instruction "stepi over NOP"
+stepi_over_instruction "stepi over lnia"
+stepi_over_instruction "stepi over addi"
+
+stepi_over_instruction "stepi over vs4 assignment"
+set check_vs4 [get_vector_hexadecimal_valueof "vs4" "default0"]
+
+stepi_over_instruction "stepi over vs5 assignment"
+set check_vs5 [get_vector_hexadecimal_valueof "vs5" "default0"]
+
+stepi_over_instruction "stepi over vs6 assignment"
+set check_vs6 [get_vector_hexadecimal_valueof "vs6" "default0"]
+
+stepi_over_instruction "stepi over vs7 assignment"
+set check_vs7 [get_vector_hexadecimal_valueof "vs7" "default0"]
+
+set vs4_expected 0xa5b5c5d5a4b4c4d4a3b3c3d3a2b2c2d2
+set vs5_expected 0xa7b7c7d7a6b6c6d6a5b5c5d5a4b4c4d4
+set vs6_expected 0xa9b9c9d9a8b8c8d8a7b7c7d7a6b6c6d6
+set vs7_expected 0xabbbcbdbaabacadaa9b9c9d9a8b8c8d8
+
+if [expr $check_vs4 != $vs4_expected] {
+ fail "unexpected value vs4; actual:$check_vs4 expected:$vs4_expected"
+}
+if [expr $check_vs5 != $vs5_expected ] {
+ fail "unexpected value vs5; actual:$check_vs5 expected:$vs5_expected"
+}
+if [expr $check_vs6 != $vs6_expected ] {
+ fail "unexpected value vs6; actual:$check_vs6 expected:$vs6_expected"
+}
+if [expr $check_vs7 != $vs7_expected ] {
+ fail "unexpected value vs7; actual:$check_vs7 expected:$vs7_expected"
+}
+
+gdb_test "info break"
+gdb_test "info register vs4 vs5 vs6 vs7 "
+gdb_test "disas main #2"
+
diff --git a/gdb/testsuite/gdb.arch/powerpc-plxv-nonrel.s b/gdb/testsuite/gdb.arch/powerpc-plxv-nonrel.s
new file mode 100644
index 00000000000..4708b214bb0
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/powerpc-plxv-nonrel.s
@@ -0,0 +1,45 @@
+# Copyright 2021 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/>.
+
+
+# test to verify that the prefixed instructions that
+# load/store non-relative values work OK.
+
+.global main
+.type main,function
+main:
+ nop
+ lnia 4
+ addi 4,4,40
+ plxv 4,4(4),0
+ plxv 5,12(4),0
+ plxv 6,20(4),0
+ plxv 7,28(4),0
+check_here:
+ blr
+mydata:
+ .long 0xa1b1c1d1 # <<-
+ .long 0xa2b2c2d2 # <<- loaded into vs4
+ .long 0xa3b3c3d3 # <<- loaded into vs4
+ .long 0xa4b4c4d4 # <<- loaded into vs4, vs5
+ .long 0xa5b5c5d5 # <<- loaded into vs4, vs5
+ .long 0xa6b6c6d6 # <<- loaded into vs5, vs6
+ .long 0xa7b7c7d7 # <<- loaded into vs5, vs6
+ .long 0xa8b8c8d8 # <<- loaded into vs6, vs7
+ .long 0xa9b9c9d9 # <<- loaded into vs6, vs7
+ .long 0xaabacada # <<- loaded into vs7
+ .long 0xabbbcbdb # <<- loaded into vs7
+ .long 0xacbcccdc # <<-
+
next prev parent reply other threads:[~2021-03-29 22:43 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-27 0:25 [PATCH,powerpc,v2] gdb-power10-single-step will schmidt
2021-03-29 15:33 ` Ulrich Weigand
2021-03-29 22:43 ` will schmidt [this message]
2021-03-30 19:47 ` [PATCH,rs6000,v3] gdb-power10-single-step Ulrich Weigand
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=896b57a450849aa1b45408b74f927d2ebd6833a3.camel@vnet.ibm.com \
--to=will_schmidt@vnet.ibm.com \
--cc=Ulrich.Weigand@de.ibm.com \
--cc=gdb-patches@sourceware.org \
--cc=rogealve@br.ibm.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).