Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 12 Jan 2021 12:48:42 GMT
From:      Andrew Turner <andrew@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: d0df1a2d54dc - main - Only allow a store through sp in the arm64 fbt
Message-ID:  <202101121248.10CCmgQi075753@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by andrew:

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

commit d0df1a2d54dcba438018f4be1d8bbaeac7e49d92
Author:     Andrew Turner <andrew@FreeBSD.org>
AuthorDate: 2021-01-12 11:37:06 +0000
Commit:     Andrew Turner <andrew@FreeBSD.org>
CommitDate: 2021-01-12 12:42:23 +0000

    Only allow a store through sp in the arm64 fbt
    
    When searching for an instruction to patch out in the arm64 function
    boundary trace we search for a store pair with a write back. This
    instruction is commonly used to store two registers to the stack
    and update the stack pointer to hold space for more.
    
    This works in many cases, however not all functions use this, e.g.
    when the stack frame is too large. In these cases we may find another
    instruction of the same type that doesn't store through the stack
    pointer. Filter these instructions out and assume if we see one we
    are past the function prologue.
    
    Reported by:    rwatson
    Sponsored by:   Innovate UK
---
 sys/cddl/contrib/opensolaris/uts/common/sys/dtrace.h |  4 +++-
 sys/cddl/dev/fbt/aarch64/fbt_isa.c                   | 16 +++++++++++++---
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace.h b/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace.h
index 31c5983270ca..922c1a9bba9b 100644
--- a/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace.h
+++ b/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace.h
@@ -2479,11 +2479,13 @@ extern void dtrace_helpers_destroy(proc_t *);
 #define	ARG1_MASK	0x1f
 #define	ARG2_SHIFT	10
 #define	ARG2_MASK	0x1f
+#define	ADDR_SHIFT	5
+#define	ADDR_MASK	0x1f
 #define	OFFSET_SHIFT	15
 #define	OFFSET_SIZE	7
 #define	OFFSET_MASK	((1 << OFFSET_SIZE) - 1)
 
-#define	DTRACE_INVOP_PUSHM	1
+#define	DTRACE_INVOP_STP	1
 #define	DTRACE_INVOP_RET	2
 #define	DTRACE_INVOP_B		3
 
diff --git a/sys/cddl/dev/fbt/aarch64/fbt_isa.c b/sys/cddl/dev/fbt/aarch64/fbt_isa.c
index 3c5070785614..9235af1a4c65 100644
--- a/sys/cddl/dev/fbt/aarch64/fbt_isa.c
+++ b/sys/cddl/dev/fbt/aarch64/fbt_isa.c
@@ -87,6 +87,7 @@ fbt_provide_module_function(linker_file_t lf, int symindx,
 	uint32_t *instr, *limit;
 	const char *name;
 	char *modname;
+	bool found;
 	int offs;
 
 	modname = opaque;
@@ -108,12 +109,21 @@ fbt_provide_module_function(linker_file_t lf, int symindx,
 	limit = (uint32_t *)(symval->value + symval->size);
 
 	/* Look for stp (pre-indexed) operation */
+	found = false;
 	for (; instr < limit; instr++) {
-		if ((*instr & LDP_STP_MASK) == STP_64)
+		/* Some functions start with "stp xt1, xt2, [xn, <const>]!" */
+		if ((*instr & LDP_STP_MASK) == STP_64) {
+			/*
+			 * Assume any other store of this type means we
+			 * are past the function prolog.
+			 */
+			if (((*instr >> ADDR_SHIFT) & ADDR_MASK) == 31)
+				found = true;
 			break;
+		}
 	}
 
-	if (instr >= limit)
+	if (!found)
 		return (0);
 
 	fbt = malloc(sizeof (fbt_probe_t), M_FBT, M_WAITOK | M_ZERO);
@@ -125,7 +135,7 @@ fbt_provide_module_function(linker_file_t lf, int symindx,
 	fbt->fbtp_loadcnt = lf->loadcnt;
 	fbt->fbtp_savedval = *instr;
 	fbt->fbtp_patchval = FBT_PATCHVAL;
-	fbt->fbtp_rval = DTRACE_INVOP_PUSHM;
+	fbt->fbtp_rval = DTRACE_INVOP_STP;
 	fbt->fbtp_symindx = symindx;
 
 	fbt->fbtp_hashnext = fbt_probetab[FBT_ADDR2NDX(instr)];



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