NetBSD Problem Report #56866

From www@netbsd.org  Tue Jun  7 01:40:27 2022
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id C02161A921F
	for <gnats-bugs@gnats.NetBSD.org>; Tue,  7 Jun 2022 01:40:27 +0000 (UTC)
Message-Id: <20220607014026.444FC1A923C@mollari.NetBSD.org>
Date: Tue,  7 Jun 2022 01:40:26 +0000 (UTC)
From: tgl@sss.pgh.pa.us
Reply-To: tgl@sss.pgh.pa.us
To: gnats-bugs@NetBSD.org
Subject: hppa: kernel gets confused between actual breakpoints and single-step breakpoints
X-Send-Pr-Version: www-1.0

>Number:         56866
>Category:       port-hppa
>Synopsis:       hppa: kernel gets confused between actual breakpoints and single-step breakpoints
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    port-hppa-maintainer
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Jun 07 01:45:00 +0000 2022
>Closed-Date:    Fri Jun 17 06:01:07 +0000 2022
>Last-Modified:  Fri Jun 17 06:01:07 +0000 2022
>Originator:     Tom Lane
>Release:        HEAD/202206030100Z
>Organization:
PostgreSQL Global Development Group
>Environment:
NetBSD sss2.sss.pgh.pa.us 9.99.97 NetBSD 9.99.97 (SD2) #0: Fri Jun  3 12:30:06 EDT 2022  tgl@nuc1.sss.pgh.pa.us:/home/tgl/netbsd-H-202206030100Z/obj.hppa/sys/arch/hppa/compile/SD2 hppa
>Description:
The stepN and setstepN (for N>1) test cases in t_ptrace_wait and sibling test programs frequently fail with

failed: /home/tgl/netbsd-H-202206030100Z/usr/src/tests/lib/libc/sys/t_ptrace_step_wait.h:143: info.psi_siginfo.si_code != TRAP_TRACE

Investigation shows that the kernel is reporting si_code = TRAP_BRKPT rather than the expected TRAP_TRACE.  I believe the reason for this is that the identical "break 0,4" instruction is inserted for both true breakpoints (those inserted from userland using PTRACE_BREAKPOINT or PTRACE_BREAKPOINT_ASM as prototype code) and single-stepping (which temporarily inserts SSBREAKPOINT which is the same thing).  In hppa's trap.c, we report SIGTRAP with si_code = TRAP_BRKPT if the trapping instruction was SSBREAKPOINT, or si_code = TRAP_TRACE if it wasn't.  Thus, we're going to produce TRAP_BRKPT during single-stepping, which if not wrong is at least not what the test case expects.
>How-To-Repeat:
$ cd /usr/tests/
$ atf-run lib/libc/sys/t_ptrace_wait

Note that this test has other issues too.  What I'm on about right now are the reports of "si_code != TRAP_TRACE".

>Fix:
I assume we don't want to change PTRACE_BREAKPOINT since that's known to userland.  Changing trap.c to use a different break code for single-stepping is a much more localized change, and it seems to fix this issue (though I've not tested very extensively for other side effects).  An annoying cosmetic problem is that PTRACE_BREAKPOINT is defined in terms of HPPA_BREAK_SS, which is completely misleading if we separate these functions.  I did this as a quick hack for testing:

Index: sys/arch/hppa/include/trap.h
===================================================================
RCS file: /cvsroot/src/sys/arch/hppa/include/trap.h,v
retrieving revision 1.5
diff -u -r1.5 trap.h
--- sys/arch/hppa/include/trap.h        6 Sep 2021 21:56:03 -0000       1.5
+++ sys/arch/hppa/include/trap.h        7 Jun 2022 01:13:05 -0000
@@ -99,6 +99,7 @@
 /* im13 */
 #define        HPPA_BREAK_SS           4
 #define        HPPA_BREAK_KGDB         5
+#define        HPPA_BREAK_SS1          6
 #define        HPPA_BREAK_GET_PSW      9
 #define        HPPA_BREAK_SET_PSW      10

Index: sys/arch/hppa/hppa/trap.c
===================================================================
RCS file: /cvsroot/src/sys/arch/hppa/hppa/trap.c,v
retrieving revision 1.117
diff -u -r1.117 trap.c
--- sys/arch/hppa/hppa/trap.c   28 May 2022 21:14:56 -0000      1.117
+++ sys/arch/hppa/hppa/trap.c   7 Jun 2022 01:13:05 -0000
@@ -108,8 +108,10 @@
 int ss_put_value(struct lwp *, vaddr_t, u_int);
 int ss_get_value(struct lwp *, vaddr_t, u_int *);

+/* externally set breakpoint (see PTRACE_BREAKPOINT) */
+#define EXTBREAKPOINT  (HPPA_BREAK_KERNEL | (HPPA_BREAK_SS << 13))
 /* single-step breakpoint */
-#define SSBREAKPOINT   (HPPA_BREAK_KERNEL | (HPPA_BREAK_SS << 13))
+#define SSBREAKPOINT   (HPPA_BREAK_KERNEL | (HPPA_BREAK_SS1 << 13))

 #endif

@@ -690,7 +692,7 @@
                ksi.ksi_addr = (void *)(frame->tf_iioq_head & ~HPPA_PC_PRIV_MASK);
 #ifdef PTRACE
                ss_clear_breakpoints(l);
-               if (opcode == SSBREAKPOINT)
+               if (opcode == EXTBREAKPOINT)
                        ksi.ksi_code = TRAP_BRKPT;
 #endif
                /* pass to user debugger */

It would be better to rename HPPA_BREAK_SS to HPPA_BREAK_BR or something like that, but I don't know if that's OK from an API stability standpoint.

>Release-Note:

>Audit-Trail:
From: Rin Okuyama <rokuyama.rk@gmail.com>
To: gnats-bugs@netbsd.org, port-hppa-maintainer@netbsd.org,
 gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Cc: 
Subject: Re: port-hppa/56866: hppa: kernel gets confused between actual
 breakpoints and single-step breakpoints
Date: Tue, 7 Jun 2022 21:04:33 +0900

 I'm not familiar to hppa, but this seems like something more than
 si_code.

 Probably, we should check whether the breakpoint was inserted by
 ptrace(2) or not. This is what we do for powerpc/{ibm4xx,booke},
 for which the software single-stepping is implemented. See:

 https://nxr.netbsd.org/xref/src/sys/arch/powerpc/ibm4xx/trap.c#300
 https://nxr.netbsd.org/xref/src/sys/arch/powerpc/powerpc/process_machdep.c#286

 Note that we use two breakpoints for powerpc in general, at PC+4
 and address where it can branch into.

 Thanks,
 rin

From: "Nick Hudson" <skrll@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/56866 CVS commit: src/sys/arch/hppa/hppa
Date: Thu, 9 Jun 2022 16:45:38 +0000

 Module Name:	src
 Committed By:	skrll
 Date:		Thu Jun  9 16:45:38 UTC 2022

 Modified Files:
 	src/sys/arch/hppa/hppa: trap.c

 Log Message:
 Only report the SSBREAKPOINT break instruction as SIGTRAP/TRAP_TRACE. All
 other break instructions will be reported as SIGTRAP/TRAP_BRKPT

 This fixes a mistake I made back in 2008.

 PR/56866: hppa: kernel gets confused between actual breakpoints and single-step breakpoints


 To generate a diff of this commit:
 cvs rdiff -u -r1.119 -r1.120 src/sys/arch/hppa/hppa/trap.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Tom Lane <tgl@sss.pgh.pa.us>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: port-hppa/56866: hppa: kernel gets confused between actual breakpoints and single-step breakpoints
Date: Thu, 09 Jun 2022 14:23:10 -0400

 Further testing shows that Nick's committed fix is correct and
 mine isn't --- mine produced the wrong si_code in some other
 test programs that I'd not been paying attention to.

 			regards, tom lane

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/56866 CVS commit: [netbsd-9] src/sys/arch/hppa
Date: Fri, 10 Jun 2022 17:34:22 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Fri Jun 10 17:34:22 UTC 2022

 Modified Files:
 	src/sys/arch/hppa/hppa [netbsd-9]: trap.c
 	src/sys/arch/hppa/include [netbsd-9]: ptrace.h

 Log Message:
 Pull up following revision(s) (requested by skrll in ticket #1468):

 	sys/arch/hppa/hppa/trap.c: revision 1.120
 	sys/arch/hppa/include/ptrace.h: revision 1.11-1.12 (via patch)

 Define a PTRACE_ILLEGAL_ASM

 Match up PTRACE_BREAKPOINT_ASM with PTRACE_BREAKPOINT which is the
 gdb breakpoint instruction.

 Only report the SSBREAKPOINT break instruction as SIGTRAP/TRAP_TRACE. All
 other break instructions will be reported as SIGTRAP/TRAP_BRKPT
 This fixes a mistake I made back in 2008.

 PR/56866: hppa: kernel gets confused between actual breakpoints and
 single-step breakpoints


 To generate a diff of this commit:
 cvs rdiff -u -r1.111.4.2 -r1.111.4.3 src/sys/arch/hppa/hppa/trap.c
 cvs rdiff -u -r1.9 -r1.9.2.1 src/sys/arch/hppa/include/ptrace.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->feedback
State-Changed-By: skrll@NetBSD.org
State-Changed-When: Thu, 16 Jun 2022 06:16:25 +0000
State-Changed-Why:
I think this is all fixed now. OK to close?


From: Tom Lane <tgl@sss.pgh.pa.us>
To: gnats-bugs@netbsd.org
Cc: port-hppa-maintainer@netbsd.org
Subject: Re: port-hppa/56866 (hppa: kernel gets confused between actual breakpoints and single-step breakpoints)
Date: Thu, 16 Jun 2022 09:41:25 -0400

 skrll@NetBSD.org writes:
 > I think this is all fixed now. OK to close?

 Yup, looks good here.

 			regards, tom lane

State-Changed-From-To: feedback->closed
State-Changed-By: skrll@NetBSD.org
State-Changed-When: Fri, 17 Jun 2022 06:01:07 +0000
State-Changed-Why:
Confirmed fixed


>Unformatted:

Home	PR Database Search