[DTrace-devel] [PATCH v2] Optimize dt_cg_store_val() for string values

[Kris Van Hees]

The dt_cg_store_val() implementation was doing more than just copying
a value to the output buffer when dealing with strings.  It was
checking the size of the string to ensure that it was not beyond the
maximum string size, and if it was, it would truncate the string.

That turns out to pose issues because it hides the fact that some of
the string handling code was not ensuring that strings were stored
with the correct string length.  It also hid the fact that string
constants can be longer than the maximum string length, and therefore
stirng functions were being presented with strings of an unacceptable
length.

This patch causes several tests in the testsuite to fail.  This is
expected behaviour and will require bugfix patches to string handling
code to ensure that all strings used in D code have a length that is
the maximum string length or less.

Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
---
 libdtrace/dt_cg.c | 48 +++++++++++++----------------------------------
 1 file changed, 13 insertions(+), 35 deletions(-)

diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index 023d2e33..a42005c6 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -958,9 +958,6 @@ dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
 
 		return 0;
 	} else if (dt_node_is_string(dnp)) {
-		uint_t		size_ok = dt_irlist_label(dlp);
-		int		reg;
-
 		dt_cg_check_notnull(dlp, drp, dnp->dn_reg);
 
 		TRACE_REGSET("store_val(): Begin ");
@@ -968,49 +965,30 @@ dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
 				 size + DT_STRLEN_BYTES + 1, 1, pfp, arg);
 
 		/*
-		 * Retrieve the length of the string, limit it to the maximum
-		 * string size, and store it in the buffer at [%r9 + off].
+		 * Copy the length of the string from the source string as a
+		 * half-word (2 bytes) into the buffer at [%r9 + off].
 		 */
-		reg = dt_regset_alloc(drp);
-		if (reg == -1)
-			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
-		dt_cg_strlen(dlp, drp, reg, dnp->dn_reg);
-		dt_regset_xalloc(drp, BPF_REG_0);
-		emit(dlp,  BPF_BRANCH_IMM(BPF_JLT, reg, size, size_ok));
-		emit(dlp,  BPF_MOV_IMM(reg, size));
-		emitl(dlp, size_ok,
-			   BPF_MOV_REG(BPF_REG_0, reg));
-		emit(dlp,  BPF_ALU64_IMM(BPF_RSH, BPF_REG_0, 8));
-		emit(dlp,  BPF_STORE(BPF_B, BPF_REG_9, off, BPF_REG_0));
-		emit(dlp,  BPF_STORE(BPF_B, BPF_REG_9, off + 1, reg));
-		dt_regset_free(drp, BPF_REG_0);
+		emit(dlp, BPF_LOAD(BPF_H, BPF_REG_0, dnp->dn_reg, 0));
+		emit(dlp, BPF_STORE(BPF_H, BPF_REG_9, off, BPF_REG_0));
 
 		/*
-		 * Copy the string to the output buffer at
-		 * [%r9 + off + DT_STRLEN_BYTES] because we need to skip the
-		 * length prefix.
+		 * Copy the string data (no more than STRSIZE + 1 bytes) to the
+		 * buffer at [%r9 + off + DT_STRLEN_BYTES].  We (ab)use the
+		 * fact that probe_read_str) stops at the terminating NUL byte.
 		 */
 		if (dt_regset_xalloc_args(drp) == -1)
 			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
 
-		emit(dlp,  BPF_MOV_REG(BPF_REG_1, BPF_REG_9));
-		emit(dlp,  BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, off + DT_STRLEN_BYTES));
-		emit(dlp,  BPF_MOV_REG(BPF_REG_2, reg));
-		dt_regset_free(drp, reg);
-		emit(dlp,  BPF_MOV_REG(BPF_REG_3, dnp->dn_reg));
+		emit(dlp, BPF_MOV_REG(BPF_REG_1, BPF_REG_9));
+		emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, off + DT_STRLEN_BYTES));
+		emit(dlp, BPF_MOV_IMM(BPF_REG_2, size + 1));
+		emit(dlp, BPF_MOV_REG(BPF_REG_3, dnp->dn_reg));
 		dt_regset_free(drp, dnp->dn_reg);
 		dt_cg_tstring_free(pcb, dnp);
-		emit(dlp,  BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, DT_STRLEN_BYTES));
+		emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, DT_STRLEN_BYTES));
 		dt_regset_xalloc(drp, BPF_REG_0);
-		emit(dlp,  BPF_CALL_HELPER(BPF_FUNC_probe_read));
+		emit(dlp, BPF_CALL_HELPER(BPF_FUNC_probe_read_str));
 		dt_regset_free_args(drp);
-
-		/*
-		 * Write the NUL terminating byte.
-		 */
-		emit(dlp,  BPF_MOV_REG(BPF_REG_0, BPF_REG_9));
-		emit(dlp,  BPF_ALU64_REG(BPF_ADD, BPF_REG_0, reg));
-		emit(dlp,  BPF_STORE_IMM(BPF_B, BPF_REG_0, off + DT_STRLEN_BYTES, 0));
 		dt_regset_free(drp, BPF_REG_0);
 		TRACE_REGSET("store_val(): End   ");
 
-- 
2.33.0