Skip to content

Commit

Permalink
Optimize dt_cg_store_val() for string values
Browse files Browse the repository at this point in the history
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
atring 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@oracle.com>
Reviewed-by: Eugene Loh <eugene.loh@oracle.com>
  • Loading branch information
kvanhees committed Nov 20, 2021
1 parent 25d31f8 commit cf1e9df
Showing 1 changed file with 13 additions and 35 deletions.
48 changes: 13 additions & 35 deletions libdtrace/dt_cg.c
Original file line number Diff line number Diff line change
Expand Up @@ -958,59 +958,37 @@ 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 ");
off = dt_rec_add(pcb->pcb_hdl, dt_cg_fill_gap, 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 ");

Expand Down

0 comments on commit cf1e9df

Please sign in to comment.