All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Peter Zijlstra <peterz@infradead.org>, x86@kernel.org, tdevries@suse.de
Cc: linux-kernel@vger.kernel.org, andrew.cooper3@citrix.com,
	Frederic Weisbecker <fweisbec@gmail.com>
Subject: [PATCH] selftests: breakpoints: Add "WINE" test for x86
Date: Fri, 29 Jan 2021 00:28:41 +0100	[thread overview]
Message-ID: <87eei4d4k6.fsf@nanos.tec.linutronix.de> (raw)
In-Reply-To: <20210128211627.GB4348@worktop.programming.kicks-ass.net>

The DR6 handling failure which made GDB upset was caused by a commit
which addressed a WINE test case regression. The test case runs a
trivial

        NOP; NOP; RET;

sequence in the tracee. The tracer runs the following steps:

 1 - Set data breakpoint on the first instruction in DR0
   - Continue tracee, wait for trap
   - Expect DR6::BS == 0 and DR6::BR0 == 1 and IP == first instruction

 2 - Clear DR6, set TF in EFLAGS
   - Set data breakpoint on the second instruction in DR0
   - Continue tracee, wait for trap
   - Expect DR6::BS == 1 and DR6::BR0 == X and IP == second instruction

 3 - Clear DR6, set TF in EFLAGS
   - Continue tracee, wait for trap
   - Expect DR6::BS == 0 and DR6::BR0 == 1 and IP == second instruction

 4 - Clear DR6, set TF in EFLAGS
   - Remove data breakpoint from DR0
   - Continue tracee, wait for trap
   - Expect DR6::BS == 1 and DR6::BR0 == 0 and IP == third instruction

The important step is #2. WINE does not care about the state of DR6::BR0
as Windows versions seem to be inconsistent. The offending commit just
excluded the BR bits when delivering the single step (BS == 1) which
made WINE happy, but broke GDB which expects the BR bits to be merged
when the single step resulted in triggering an armed data breakpoint at
the same time.

Add a test case which covers this scenario. This is modeled after the
WINE testcase, but changes the expect in step #2 to:

   - Expect DR6::BS == 1 and DR6::BR0 == 1 and IP == second instruction

to ensure that the GDB expectations are met as well.

Make it work for both 32 and 64 bit and fix the broken calculation of
number of tests for 32 bit as well.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 tools/testing/selftests/breakpoints/breakpoint_test.c |  115 ++++++++++++++++--
 1 file changed, 107 insertions(+), 8 deletions(-)

--- a/tools/testing/selftests/breakpoints/breakpoint_test.c
+++ b/tools/testing/selftests/breakpoints/breakpoint_test.c
@@ -23,6 +23,13 @@
 #define COUNT_ISN_BPS	4
 #define COUNT_WPS	4
 
+#ifdef __i386__
+# define rip		eip
+# define COUNT_LEN	3
+#else
+# define COUNT_LEN	4
+#endif
+
 /* Breakpoint access modes */
 enum {
 	BP_X = 1,
@@ -50,6 +57,16 @@ static void set_breakpoint_addr(void *ad
 			strerror(errno));
 }
 
+static void set_dr7(unsigned long dr7)
+{
+	int ret;
+
+	ret = ptrace(PTRACE_POKEUSER, child_pid,
+		     offsetof(struct user, u_debugreg[7]), dr7);
+	if (ret)
+		ksft_exit_fail_msg("Can't set dr7: %s\n", strerror(errno));
+}
+
 static void toggle_breakpoint(int n, int type, int len,
 			      int local, int global, int set)
 {
@@ -105,12 +122,7 @@ static void toggle_breakpoint(int n, int
 	else
 		dr7 &= ~vdr7;
 
-	ret = ptrace(PTRACE_POKEUSER, child_pid,
-		     offsetof(struct user, u_debugreg[7]), dr7);
-	if (ret) {
-		ksft_print_msg("Can't set dr7: %s\n", strerror(errno));
-		exit(-1);
-	}
+	set_dr7(dr7);
 }
 
 /* Dummy variables to test read/write accesses */
@@ -196,6 +208,13 @@ static void read_var(int len)
 	}
 }
 
+void __wine_test(void);
+
+static void wine_test(void)
+{
+	asm volatile (".global __wine_test; __wine_test: nop; nop; ret\n");
+}
+
 /*
  * Do the r/w/x accesses to trigger the breakpoints. And run
  * the usual traps.
@@ -258,6 +277,11 @@ static void trigger_tests(void)
 	asm("int $3\n");
 	check_trapped();
 
+	/*
+	 * The wine test: NOP, NOP, RET with singlestep and data breakpoint.
+	 */
+	wine_test();
+
 	kill(getpid(), SIGUSR1);
 }
 
@@ -327,6 +351,76 @@ static void launch_watchpoints(char *buf
 	}
 }
 
+static void wine_test_step(int step, void *ip, int bs, int b0, char *buf)
+{
+	unsigned long eflags, dr6, child_ip, ipoffs;
+	int dr6_bs, dr6_b0, status;
+
+	ptrace(PTRACE_CONT, child_pid, NULL, 0);
+	wait(&status);
+
+	if (WSTOPSIG(status) != SIGTRAP)
+		ksft_exit_fail_msg("Expected SIGTRAP got %d\n", status);
+
+	/* Get DR6 from the child and clear it */
+	dr6 = ptrace(PTRACE_PEEKUSER, child_pid,
+		     offsetof(struct user, u_debugreg[6]), 0),
+	ptrace(PTRACE_POKEUSER, child_pid,
+	       offsetof(struct user, u_debugreg[6]), 0);
+
+	/* Get the IP from the child */
+	child_ip = ptrace(PTRACE_PEEKUSER, child_pid,
+			  offsetof(struct user, regs.rip), 0);
+
+	/* Except for the last step, set TF in eflags */
+	if (step < 3) {
+		eflags = ptrace(PTRACE_PEEKUSER, child_pid,
+				offsetof(struct user, regs.eflags), 0);
+		ptrace(PTRACE_POKEUSER, child_pid,
+		       offsetof(struct user, regs.eflags), eflags | 0x100);
+	}
+
+	/* Calculate the IP offset and isolate the DR6 bits to check */
+	ipoffs = (unsigned long) ip - child_ip;
+	dr6_bs = !!(dr6 & 0x4000);
+	dr6_b0 = !!(dr6 & 0x0001);
+
+	sprintf(buf, "Test wine_test step: %d dr6_bs: %d (%d) dr6_b0: %d (%d) ipoffs: %lx\n",
+		step, dr6_bs, bs, dr6_b0, b0, ipoffs);
+
+	nr_tests++;
+
+	/* Fail if IPOFFS != 0 or BS, B0 are not matching */
+	if (ipoffs || bs != dr6_bs || b0 != dr6_b0)
+		ksft_test_result_fail(buf);
+	else
+		ksft_test_result_pass(buf);
+}
+
+static void launch_wine_test(char *buf)
+{
+	void *addr = __wine_test;
+
+	/* Data break point (RW) on first instruction */
+	set_breakpoint_addr(addr, 0);
+	set_dr7(0x03);
+	/* Expect: DR6::BS == 0 DR6::BR0 == 1 IP == instr[0] */
+	wine_test_step(0, addr, 0, 1, buf);
+	/* Data break point (RW) on second instruction */
+	set_breakpoint_addr(++addr, 0);
+	/*
+	 * Expect: DR6::BS == 1 DR6::BR0 == 1  IP == instr[1]
+	 * Wine does not care about BR0 here but GDB does ...
+	 */
+	wine_test_step(1, addr, 1, 1, buf);
+	/* Expect: DR6::BS == 0 DR6::BR0 == 1  IP == instr[1] */
+	wine_test_step(2, addr, 0, 1, buf);
+	/* Remove the data breakpoint
+	set_breakpoint_addr(NULL, 0);
+	/* Expect: DR6::BS == 1 DR6::BR0 == 0 IP == instr[2] */
+	wine_test_step(3, ++addr, 1, 0, buf);
+}
+
 /* Set the breakpoints and check the child successfully trigger them */
 static void launch_tests(void)
 {
@@ -335,9 +429,12 @@ static void launch_tests(void)
 	int len, local, global, i;
 
 	tests += 3 * COUNT_ISN_BPS;
-	tests += sizeof(long) / 2 * 3 * COUNT_WPS;
-	tests += sizeof(long) / 2 * 3 * COUNT_WPS;
+	tests += COUNT_LEN * 3 * COUNT_WPS;
+	tests += COUNT_LEN * 3 * COUNT_WPS;
+	/* ICEBP, INT3 */
 	tests += 2;
+	/* WINE test */
+	tests += 4;
 	ksft_set_plan(tests);
 
 	/* Instruction breakpoints */
@@ -381,6 +478,8 @@ static void launch_tests(void)
 	ptrace(PTRACE_CONT, child_pid, NULL, 0);
 	check_success("Test int 3 trap\n");
 
+	launch_wine_test(buf);
+
 	ptrace(PTRACE_CONT, child_pid, NULL, 0);
 }
 


  reply	other threads:[~2021-01-28 23:30 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-28 18:20 [PATCH] x86/debug: 'Fix' ptrace dr6 output Peter Zijlstra
2021-01-28 18:45 ` Andrew Cooper
2021-01-28 20:21 ` Tom de Vries
2021-01-28 21:16 ` [PATCH v2] x86/debug: Fix DR6 handling Peter Zijlstra
2021-01-28 23:28   ` Thomas Gleixner [this message]
     [not found]     ` <YBPQq6ccKL68aIZg@hirez.programming.kicks-ass.net>
2021-01-29 19:09       ` [PATCH] selftests: breakpoints: Add "WINE" test for x86 Thomas Gleixner
     [not found]   ` <20210129154109.GA1391@redhat.com>
2021-01-29 16:27     ` [PATCH v2] x86/debug: Fix DR6 handling Borislav Petkov
     [not found]   ` <20210129144816.GB27841@zn.tnic>
2021-01-29 16:59     ` Tom de Vries

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=87eei4d4k6.fsf@nanos.tec.linutronix.de \
    --to=tglx@linutronix.de \
    --cc=andrew.cooper3@citrix.com \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=tdevries@suse.de \
    --cc=x86@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.