Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 4 Oct 2024 15:57:01 GMT
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 8b1d908cdeb7 - stable/14 - dtrace/amd64: Fix probe argument fetching
Message-ID:  <202410041557.494Fv10p048372@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/14 has been updated by markj:

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

commit 8b1d908cdeb7fb236011ed9c42517ce0ee9c9613
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-09-19 09:21:38 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2024-10-04 15:56:42 +0000

    dtrace/amd64: Fix probe argument fetching
    
    dtrace_getarg() previously walked the call stack looking for a frame
    matching the dtrace_invop_callsite symbol, in order to look for a
    trapframe corresponding to an invop (i.e., FBT or kinst) probe.  Commit
    3ba8e9dc4a0e broke this in some cases by breaking the expected alignment
    of the dtrace_invop_callsite symbol.
    
    Rather than groveling around the stack to find invop probe arguments,
    simply use the trapframe reference saved by dtrace_invop().  This is
    simpler and less fragile.
    
    Reported by:    avg
    Reviewed by:    avg
    MFC after:      2 weeks
    Fixes:  3ba8e9dc4a0e ("dtrace/amd64: Implement emulation of call instructions")
    Differential Revision:  https://reviews.freebsd.org/D46672
    
    (cherry picked from commit 9aabab09c4e163fa8187b265d6836d822516b1c9)
---
 sys/cddl/dev/dtrace/amd64/dtrace_isa.c | 77 ++++++++++++++--------------------
 1 file changed, 31 insertions(+), 46 deletions(-)

diff --git a/sys/cddl/dev/dtrace/amd64/dtrace_isa.c b/sys/cddl/dev/dtrace/amd64/dtrace_isa.c
index 9db5a16190db..83d34abbd270 100644
--- a/sys/cddl/dev/dtrace/amd64/dtrace_isa.c
+++ b/sys/cddl/dev/dtrace/amd64/dtrace_isa.c
@@ -32,6 +32,8 @@
 #include <sys/stack.h>
 #include <sys/pcpu.h>
 
+#include <cddl/dev/dtrace/dtrace_cddl.h>
+
 #include <machine/frame.h>
 #include <machine/md_var.h>
 #include <machine/stack.h>
@@ -355,6 +357,7 @@ zero:
 uint64_t
 dtrace_getarg(int arg, int aframes)
 {
+	struct thread *td;
 	uintptr_t val;
 	struct amd64_frame *fp = (struct amd64_frame *)dtrace_getfp();
 	uintptr_t *stack;
@@ -366,57 +369,39 @@ dtrace_getarg(int arg, int aframes)
 	 */
 	int inreg = 5;
 
-	for (i = 1; i <= aframes; i++) {
-		fp = fp->f_frame;
-
-		if (roundup2(fp->f_retaddr, 16) ==
-		    (long)dtrace_invop_callsite) {
-			/*
-			 * In the case of amd64, we will use the pointer to the
-			 * regs structure that was pushed when we took the
-			 * trap.  To get this structure, we must increment
-			 * beyond the frame structure, and then again beyond
-			 * the calling RIP stored in dtrace_invop().  If the
-			 * argument that we're seeking is passed on the stack,
-			 * we'll pull the true stack pointer out of the saved
-			 * registers and decrement our argument by the number
-			 * of arguments passed in registers; if the argument
-			 * we're seeking is passed in registers, we can just
-			 * load it directly.
-			 */
-			struct trapframe *tf = (struct trapframe *)&fp[1];
-
-			if (arg <= inreg) {
-				switch (arg) {
-				case 0:
-					stack = (uintptr_t *)&tf->tf_rdi;
-					break;
-				case 1:
-					stack = (uintptr_t *)&tf->tf_rsi;
-					break;
-				case 2:
-					stack = (uintptr_t *)&tf->tf_rdx;
-					break;
-				case 3:
-					stack = (uintptr_t *)&tf->tf_rcx;
-					break;
-				case 4:
-					stack = (uintptr_t *)&tf->tf_r8;
-					break;
-				case 5:
-					stack = (uintptr_t *)&tf->tf_r9;
-					break;
-				}
-				arg = 0;
-			} else {
-				stack = (uintptr_t *)(tf->tf_rsp);
-				arg -= inreg;
+	/*
+	 * Did we arrive here via dtrace_invop()?  We can simply fetch arguments
+	 * from the trap frame if so.
+	 */
+	td = curthread;
+	if (td->t_dtrace_trapframe != NULL) {
+		struct trapframe *tf = td->t_dtrace_trapframe;
+
+		if (arg <= inreg) {
+			switch (arg) {
+			case 0:
+				return (tf->tf_rdi);
+			case 1:
+				return (tf->tf_rsi);
+			case 2:
+				return (tf->tf_rdx);
+			case 3:
+				return (tf->tf_rcx);
+			case 4:
+				return (tf->tf_r8);
+			case 5:
+				return (tf->tf_r9);
 			}
-			goto load;
 		}
 
+		arg -= inreg;
+		stack = (uintptr_t *)tf->tf_rsp;
+		goto load;
 	}
 
+	for (i = 1; i <= aframes; i++)
+		fp = fp->f_frame;
+
 	/*
 	 * We know that we did not come through a trap to get into
 	 * dtrace_probe() -- the provider simply called dtrace_probe()



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202410041557.494Fv10p048372>