git: cdfa49f2584a - main - dtrace: dtrace_getpcstack() tweaks for riscv

From: Mitchell Horne <mhorne_at_FreeBSD.org>
Date: Thu, 12 Jan 2023 15:06:54 UTC
The branch main has been updated by mhorne:

URL: https://cgit.FreeBSD.org/src/commit/?id=cdfa49f2584ac174648ced04d76a47ad6773f2f0

commit cdfa49f2584ac174648ced04d76a47ad6773f2f0
Author:     Mitchell Horne <mhorne@FreeBSD.org>
AuthorDate: 2023-01-11 18:06:02 +0000
Commit:     Mitchell Horne <mhorne@FreeBSD.org>
CommitDate: 2023-01-12 15:04:58 +0000

    dtrace: dtrace_getpcstack() tweaks for riscv
    
    Backtraces for fbt probes are missing the caller's frame. Despite what
    the inherited comment claims, we do need to insert this manually on
    riscv. In fbt_invop(), set cpu_dtrace_caller to be the return address,
    not addr.
    
    We should not increment aframes within this function, since we begin the
    main loop by unwinding past the current frame.
    
    Plus some very small comment/style tweaks.
    
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D37661
---
 sys/cddl/dev/dtrace/riscv/dtrace_isa.c | 32 ++++++++++++++++++++------------
 sys/cddl/dev/fbt/riscv/fbt_isa.c       |  2 +-
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/sys/cddl/dev/dtrace/riscv/dtrace_isa.c b/sys/cddl/dev/dtrace/riscv/dtrace_isa.c
index 1f585aa6fa3c..bc8426a752df 100644
--- a/sys/cddl/dev/dtrace/riscv/dtrace_isa.c
+++ b/sys/cddl/dev/dtrace/riscv/dtrace_isa.c
@@ -64,42 +64,50 @@ dtrace_getpcstack(pc_t *pcstack, int pcstack_limit, int aframes,
     uint32_t *intrpc)
 {
 	struct unwind_state state;
-	int scp_offset;
+	uintptr_t caller;
 	register_t sp;
+	int scp_offset;
 	int depth;
 
 	depth = 0;
+	caller = solaris_cpu[curcpu].cpu_dtrace_caller;
 
 	if (intrpc != 0) {
-		pcstack[depth++] = (pc_t) intrpc;
+		pcstack[depth++] = (pc_t)intrpc;
 	}
 
-	aframes++;
-
+	/*
+	 * Construct the unwind state, starting from this function. This frame,
+	 * and 'aframes' others will be skipped.
+	 */
 	__asm __volatile("mv %0, sp" : "=&r" (sp));
 
 	state.fp = (uintptr_t)__builtin_frame_address(0);
-	state.sp = sp;
+	state.sp = (uintptr_t)sp;
 	state.pc = (uintptr_t)dtrace_getpcstack;
 
 	while (depth < pcstack_limit) {
 		if (!unwind_frame(curthread, &state))
 			break;
 
-		if (!INKERNEL(state.pc) || !INKERNEL(state.fp))
+		if (!INKERNEL(state.pc) || !kstack_contains(curthread,
+		    (vm_offset_t)state.fp, sizeof(uintptr_t)))
 			break;
 
-		/*
-		 * NB: Unlike some other architectures, we don't need to
-		 * explicitly insert cpu_dtrace_caller as it appears in the
-		 * normal kernel stack trace rather than a special trap frame.
-		 */
 		if (aframes > 0) {
 			aframes--;
+
+			/*
+			 * fbt_invop() records the return address at the time
+			 * the FBT probe fires. We need to insert this into the
+			 * backtrace manually, since the stack frame state at
+			 * the time of the probe does not capture it.
+			 */
+			if (aframes == 0 && caller != 0)
+				pcstack[depth++] = caller;
 		} else {
 			pcstack[depth++] = state.pc;
 		}
-
 	}
 
 	for (; depth < pcstack_limit; depth++) {
diff --git a/sys/cddl/dev/fbt/riscv/fbt_isa.c b/sys/cddl/dev/fbt/riscv/fbt_isa.c
index 659a9d44c81c..a67e83811d9e 100644
--- a/sys/cddl/dev/fbt/riscv/fbt_isa.c
+++ b/sys/cddl/dev/fbt/riscv/fbt_isa.c
@@ -57,7 +57,7 @@ fbt_invop(uintptr_t addr, struct trapframe *frame, uintptr_t rval)
 
 	for (; fbt != NULL; fbt = fbt->fbtp_hashnext) {
 		if ((uintptr_t)fbt->fbtp_patchpoint == addr) {
-			cpu->cpu_dtrace_caller = addr;
+			cpu->cpu_dtrace_caller = frame->tf_ra - INSN_SIZE;
 
 			if (fbt->fbtp_roffset == 0) {
 				dtrace_probe(fbt->fbtp_id, frame->tf_a[0],