[DTrace-devel] [PATCH 34/47] Check register allocation requests and handle function calls
Kris Van Hees
kris.van.hees at oracle.com
Sun May 3 20:17:58 PDT 2020
Some register allocation calls did not check the return value to ensure
we were able to allocate the register. That has been fixed.
BPF function calls (to BPF functions or helper functions) potentially
clobber %r1 through %r5. The verifier ensures that these registers
cannot be read from after a call until an explicit store is done into
the register.
This patch adds dt_regset_xalloc_args() and dt_regset_free_args() as
convenient ways to bulk allocate and free %r1 through %r5. The alloc
fails if any of the registers is already in use.
Due to the reservation of %r9 as output buffer base pointer, the need
for %r1 through %r5 as function arguments, and the use of %r0 as call
return value (even if none is returned), only %r6 through %r8 are
available as general scratch registers in code that makes function
calls. This imposes a severe limitation. In order to provide for an
adequate number of general purpose registers, register spilling is
implemented.
Orabug: 31187562
Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
---
libdtrace/dt_cg.c | 72 +++++++++---
libdtrace/dt_regset.c | 104 +++++++++++++++---
libdtrace/dt_regset.h | 17 ++-
.../codegen/tst.reg_spilling.bug31187562.d | 21 ++++
.../codegen/tst.reg_spilling.bug31187562.r | 5 +
5 files changed, 179 insertions(+), 40 deletions(-)
create mode 100644 test/unittest/codegen/tst.reg_spilling.bug31187562.d
create mode 100644 test/unittest/codegen/tst.reg_spilling.bug31187562.r
diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index 8f1120bd..1fe79e97 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -222,6 +222,26 @@ dt_cg_fill_gap(dt_pcb_t *pcb, int gap)
}
}
+static void
+dt_cg_spill_store(int reg)
+{
+ dt_irlist_t *dlp = &yypcb->pcb_ir;
+ struct bpf_insn instr;
+
+ instr = BPF_STORE(BPF_DW, BPF_REG_FP, DT_STK_SPILL(reg), reg);
+ dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
+}
+
+static void
+dt_cg_spill_load(int reg)
+{
+ dt_irlist_t *dlp = &yypcb->pcb_ir;
+ struct bpf_insn instr;
+
+ instr = BPF_LOAD(BPF_DW, reg, BPF_REG_FP, DT_STK_SPILL(reg));
+ dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
+}
+
static void
dt_cg_act_breakpoint(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
{
@@ -799,6 +819,9 @@ dt_cg_load_var(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp)
idp->di_flags |= DT_IDFLG_DIFR;
if (idp->di_flags & DT_IDFLG_LOCAL) { /* local var */
+ if ((dst->dn_reg = dt_regset_alloc(drp)) == -1)
+ longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
+
/*
* If this is the first read for this local variable, we know
* the value is 0. This avoids storing an initial 0 value in
@@ -812,7 +835,8 @@ dt_cg_load_var(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp)
dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
} else if (idp->di_flags & DT_IDFLG_TLS) { /* TLS var */
- dt_regset_xalloc(drp, BPF_REG_1);
+ if (dt_regset_xalloc_args(drp) == -1)
+ longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
instr = BPF_MOV_IMM(BPF_REG_1,
idp->di_id - DIF_VAR_OTHER_UBASE);
dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
@@ -822,12 +846,17 @@ dt_cg_load_var(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp)
instr = BPF_CALL_FUNC(idp->di_id);
dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
dlp->dl_last->di_extern = idp;
+ dt_regset_free_args(drp);
+
+ if ((dst->dn_reg = dt_regset_alloc(drp)) == -1)
+ longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
+
instr = BPF_MOV_REG(dst->dn_reg, BPF_REG_0);
dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
dt_regset_free(drp, BPF_REG_0);
- dt_regset_free(drp, BPF_REG_1);
} else { /* global var */
- dt_regset_xalloc(drp, BPF_REG_1);
+ if (dt_regset_xalloc_args(drp) == -1)
+ longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
if (idp->di_id < DIF_VAR_OTHER_UBASE) { /* built-in var */
instr = BPF_MOV_IMM(BPF_REG_1, idp->di_id);
dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE,
@@ -845,10 +874,14 @@ dt_cg_load_var(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp)
instr = BPF_CALL_FUNC(idp->di_id);
dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
dlp->dl_last->di_extern = idp;
+ dt_regset_free_args(drp);
+
+ if ((dst->dn_reg = dt_regset_alloc(drp)) == -1)
+ longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
+
instr = BPF_MOV_REG(dst->dn_reg, BPF_REG_0);
dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
dt_regset_free(drp, BPF_REG_0);
- dt_regset_free(drp, BPF_REG_1);
}
}
@@ -1128,10 +1161,10 @@ dt_cg_store_var(dt_node_t *src, dt_irlist_t *dlp, dt_regset_t *drp,
src->dn_reg);
dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
} else if (idp->di_flags & DT_IDFLG_TLS) { /* TLS var */
- dt_regset_xalloc(drp, BPF_REG_2);
+ if (dt_regset_xalloc_args(drp) == -1)
+ longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
instr = BPF_MOV_REG(BPF_REG_2, src->dn_reg);
dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
- dt_regset_xalloc(drp, BPF_REG_1);
instr = BPF_MOV_IMM(BPF_REG_1,
idp->di_id - DIF_VAR_OTHER_UBASE);
dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
@@ -1142,13 +1175,12 @@ dt_cg_store_var(dt_node_t *src, dt_irlist_t *dlp, dt_regset_t *drp,
dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
dlp->dl_last->di_extern = idp;
dt_regset_free(drp, BPF_REG_0);
- dt_regset_free(drp, BPF_REG_1);
- dt_regset_free(drp, BPF_REG_2);
+ dt_regset_free_args(drp);
} else { /* global var */
- dt_regset_xalloc(drp, BPF_REG_2);
+ if (dt_regset_xalloc_args(drp) == -1)
+ longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
instr = BPF_MOV_REG(BPF_REG_2, src->dn_reg);
dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
- dt_regset_xalloc(drp, BPF_REG_1);
instr = BPF_MOV_IMM(BPF_REG_1,
idp->di_id - DIF_VAR_OTHER_UBASE);
dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
@@ -1159,8 +1191,7 @@ dt_cg_store_var(dt_node_t *src, dt_irlist_t *dlp, dt_regset_t *drp,
dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
dlp->dl_last->di_extern = idp;
dt_regset_free(drp, BPF_REG_0);
- dt_regset_free(drp, BPF_REG_1);
- dt_regset_free(drp, BPF_REG_2);
+ dt_regset_free_args(drp);
}
}
@@ -2449,6 +2480,8 @@ dt_cg_node(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
* the actual string:
* get_string(stroff);
*/
+ if (dt_regset_xalloc_args(drp) == -1)
+ longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
instr = BPF_MOV_IMM(BPF_REG_1, stroff);
dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
idp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_string");
@@ -2458,6 +2491,7 @@ dt_cg_node(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
dlp->dl_last->di_extern = idp;
instr = BPF_MOV_REG(dnp->dn_reg, BPF_REG_0);
dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
+ dt_regset_free_args(drp);
#endif
break;
@@ -2537,9 +2571,6 @@ if ((idp = dnp->dn_ident)->di_kind != DT_IDENT_FUNC)
break;
}
- if ((dnp->dn_reg = dt_regset_alloc(drp)) == -1)
- longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
-
dt_cg_load_var(dnp, dlp, drp);
break;
@@ -2606,9 +2637,13 @@ dt_cg(dt_pcb_t *pcb, dt_node_t *dnp)
dt_xlator_t *dxp = NULL;
dt_node_t *act;
- if (pcb->pcb_regs == NULL && (pcb->pcb_regs =
- dt_regset_create(pcb->pcb_hdl->dt_conf.dtc_difintregs)) == NULL)
- longjmp(pcb->pcb_jmpbuf, EDT_NOMEM);
+ if (pcb->pcb_regs == NULL) {
+ pcb->pcb_regs = dt_regset_create(
+ pcb->pcb_hdl->dt_conf.dtc_difintregs,
+ dt_cg_spill_store, dt_cg_spill_load);
+ if (pcb->pcb_regs == NULL)
+ longjmp(pcb->pcb_jmpbuf, EDT_NOMEM);
+ }
dt_regset_reset(pcb->pcb_regs);
@@ -2756,6 +2791,7 @@ dt_cg(dt_pcb_t *pcb, dt_node_t *dnp)
} else {
dt_cg_node(act->dn_expr, &pcb->pcb_ir,
pcb->pcb_regs);
+ assert (pcb->pcb_dret->dn_reg != -1);
dt_regset_free(pcb->pcb_regs,
pcb->pcb_dret->dn_reg);
}
diff --git a/libdtrace/dt_regset.c b/libdtrace/dt_regset.c
index 5cfc6296..ffccd28f 100644
--- a/libdtrace/dt_regset.c
+++ b/libdtrace/dt_regset.c
@@ -11,41 +11,50 @@
#include <string.h>
#include <stdlib.h>
+#include <stdio.h>
+
#include <dt_debug.h>
#include <dt_regset.h>
dt_regset_t *
-dt_regset_create(ulong_t size)
+dt_regset_create(ulong_t size, dt_cg_spill_f stf, dt_cg_spill_f ldf)
{
- dt_regset_t *drp = malloc(sizeof (dt_regset_t));
+ dt_regset_t *drp = malloc(sizeof(dt_regset_t));
if (drp == NULL)
- return (NULL);
+ return NULL;
size++; /* for %r0 */
drp->dr_size = size;
- drp->dr_bitmap = malloc(BT_SIZEOFMAP(size));
- if (drp->dr_bitmap == NULL) {
+ drp->dr_spill_store = stf;
+ drp->dr_spill_load = ldf;
+ drp->dr_active = malloc(BT_SIZEOFMAP(size));
+ drp->dr_spilled = malloc(BT_SIZEOFMAP(size));
+ if (drp->dr_active == NULL || drp->dr_spilled == NULL) {
+ free(drp->dr_active);
+ free(drp->dr_spilled);
free(drp);
- return (NULL);
+ return NULL;
}
dt_regset_reset(drp);
- return (drp);
+ return drp;
}
void
dt_regset_destroy(dt_regset_t *drp)
{
- free(drp->dr_bitmap);
+ free(drp->dr_active);
+ free(drp->dr_spilled);
free(drp);
}
void
dt_regset_reset(dt_regset_t *drp)
{
- memset(drp->dr_bitmap, 0, BT_SIZEOFMAP(drp->dr_size));
+ memset(drp->dr_active, 0, BT_SIZEOFMAP(drp->dr_size));
+ memset(drp->dr_spilled, 0, BT_SIZEOFMAP(drp->dr_size));
}
int
@@ -54,27 +63,86 @@ dt_regset_alloc(dt_regset_t *drp)
int reg;
for (reg = drp->dr_size - 1; reg >= 0; reg--) {
- if (BT_TEST(drp->dr_bitmap, reg) == 0) {
- BT_SET(drp->dr_bitmap, reg);
- return (reg);
+ if (BT_TEST(drp->dr_active, reg) == 0) {
+ BT_SET(drp->dr_active, reg);
+ return reg;
+ }
+ }
+
+ for (reg = drp->dr_size - 1; reg >= 0; reg--) {
+ if (BT_TEST(drp->dr_spilled, reg) == 0) {
+ drp->dr_spill_store(reg);
+ BT_SET(drp->dr_spilled, reg);
+ return reg;
}
}
- return (-1); /* no available registers */
+
+ return -1; /* no available registers */
}
-void
+/*
+ * Allocate a specific register.
+ */
+int
dt_regset_xalloc(dt_regset_t *drp, int reg)
{
assert(reg >= 0 && reg < drp->dr_size);
- assert(BT_TEST(drp->dr_bitmap, reg) == 0);
+ if (BT_TEST(drp->dr_active, reg) != 0) {
+ if (BT_TEST(drp->dr_spilled, reg) != 0)
+ return -1; /* register in use (and spilled)*/
+
+ drp->dr_spill_store(reg);
+ BT_SET(drp->dr_spilled, reg);
+ }
+
+ BT_SET(drp->dr_active, reg);
- BT_SET(drp->dr_bitmap, reg);
+ return 0;
}
void
dt_regset_free(dt_regset_t *drp, int reg)
{
assert(reg >= 0 && reg < drp->dr_size);
- assert(BT_TEST(drp->dr_bitmap, reg) != 0);
- BT_CLEAR(drp->dr_bitmap, reg);
+ assert(BT_TEST(drp->dr_active, reg) != 0);
+
+ if (BT_TEST(drp->dr_spilled, reg) != 0) {
+ drp->dr_spill_load(reg);
+ BT_CLEAR(drp->dr_spilled, reg);
+ return;
+ }
+
+ BT_CLEAR(drp->dr_active, reg);
+}
+
+/*
+ * Allocate %r1 through %r5 for use as function call arguments in BPF.
+ */
+int
+dt_regset_xalloc_args(dt_regset_t *drp)
+{
+ int reg;
+
+ for (reg = 1; reg <= 5; reg++) {
+ if (dt_regset_xalloc(drp, reg) == -1) {
+ while (--reg > 0)
+ dt_regset_free(drp, reg);
+
+ return -1; /* register in use */
+ }
+ }
+
+ return 0;
+}
+
+/*
+ * Free %r1 through %r5 after they were used as function call arguments in BPF.
+ */
+void
+dt_regset_free_args(dt_regset_t *drp)
+{
+ int reg;
+
+ for (reg = 1; reg <= 5; reg++)
+ dt_regset_free(drp, reg);
}
diff --git a/libdtrace/dt_regset.h b/libdtrace/dt_regset.h
index 08427828..d91febbf 100644
--- a/libdtrace/dt_regset.h
+++ b/libdtrace/dt_regset.h
@@ -11,21 +11,30 @@
#include <sys/types.h>
#include <sys/dtrace_types.h>
+#include <dt_as.h>
+
#ifdef __cplusplus
extern "C" {
#endif
+typedef void (*dt_cg_spill_f)(int reg);
+
typedef struct dt_regset {
- ulong_t dr_size; /* number of registers in set */
- ulong_t *dr_bitmap; /* bitmap of active registers */
+ ulong_t dr_size; /* number of registers in set */
+ dt_cg_spill_f dr_spill_store; /* register spill store function */
+ dt_cg_spill_f dr_spill_load; /* register spill load function */
+ ulong_t *dr_active; /* bitmap of active registers */
+ ulong_t *dr_spilled; /* bitmap of spilled registers */
} dt_regset_t;
-extern dt_regset_t *dt_regset_create(ulong_t);
+extern dt_regset_t *dt_regset_create(ulong_t, dt_cg_spill_f, dt_cg_spill_f);
extern void dt_regset_destroy(dt_regset_t *);
extern void dt_regset_reset(dt_regset_t *);
extern int dt_regset_alloc(dt_regset_t *);
-extern void dt_regset_xalloc(dt_regset_t *, int);
+extern int dt_regset_xalloc(dt_regset_t *, int);
extern void dt_regset_free(dt_regset_t *, int);
+extern int dt_regset_xalloc_args(dt_regset_t *);
+extern void dt_regset_free_args(dt_regset_t *);
#ifdef __cplusplus
}
diff --git a/test/unittest/codegen/tst.reg_spilling.bug31187562.d b/test/unittest/codegen/tst.reg_spilling.bug31187562.d
new file mode 100644
index 00000000..5b7395e1
--- /dev/null
+++ b/test/unittest/codegen/tst.reg_spilling.bug31187562.d
@@ -0,0 +1,21 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * Licensed under the Universal Permissive License v 1.0 as shown at
+ * http://oss.oracle.com/licenses/upl.
+ */
+
+/*
+ * ASSERTION: Test that the code generator will spill registers to the stack
+ * and restore them in order to make them available for allocation.
+ */
+
+BEGIN
+{
+ a = 1000;
+ b = 200;
+ c = 30;
+ d = 4;
+ trace(a++ + (b++ + (c++ + d++)));
+ exit(0);
+}
diff --git a/test/unittest/codegen/tst.reg_spilling.bug31187562.r b/test/unittest/codegen/tst.reg_spilling.bug31187562.r
new file mode 100644
index 00000000..78254e1e
--- /dev/null
+++ b/test/unittest/codegen/tst.reg_spilling.bug31187562.r
@@ -0,0 +1,5 @@
+ FUNCTION:NAME
+ :BEGIN 1234
+
+-- @@stderr --
+dtrace: script 'test/unittest/codegen/tst.reg_spilling.bug31187562.d' matched 1 probe
--
2.26.0
More information about the DTrace-devel
mailing list