[DTrace-devel] [PATCH 39/47] Fix the stack layout for register spilling
Kris Van Hees
kris.van.hees at oracle.com
Sun May 3 20:18:11 PDT 2020
The layout of the stack and the constants controlling that were poorly
defined at the onset of the DTrace v2 development. As a result, when
register spilling was implemented, various bugs came to light. E.g. it
was possible to overwrite other fields in the stack (cpu, local
variables).
This patch includes tests that verify that the stack layout is correct,
although it should be noted that the stack corruption can happen in
more subtle ways and not all cases can be tested for in a formal manner.
The tests should cover the more comon cases though.
This patch also adds a test utility to print out the offset of the
various fields in the stack used by DTrace for BPF programs (and one of
the tests uses this).
Since this patch makes some modifications to the build system machinery
for installing the print-stack-layout utlity, a little additional
reworking of the install-test target for utils is included to ensure
that executables are installed correctly. It also removes the ETCDIR
and INSTETCDIR makefile variables that are no longer used.
Orabug: 31187562
Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
---
GNUmakefile | 2 -
cmd/Build | 2 +-
include/bpf_asm.h | 12 +++
libdtrace/dt_cc.c | 2 +-
libdtrace/dt_dis.c | 4 +-
libdtrace/dt_impl.h | 91 +++++++++++++++----
test/Build | 2 +-
.../codegen/tst.reg_spilling.bug31187562-2.d | 30 ++++++
.../codegen/tst.reg_spilling.bug31187562-2.r | 5 +
test/unittest/codegen/tst.stack_layout.r | 19 ++++
test/unittest/codegen/tst.stack_layout.sh | 14 +++
test/utils/.gitignore | 1 +
test/utils/Build | 15 ++-
test/utils/print-stack-layout.c | 41 +++++++++
14 files changed, 213 insertions(+), 27 deletions(-)
create mode 100644 test/unittest/codegen/tst.reg_spilling.bug31187562-2.d
create mode 100644 test/unittest/codegen/tst.reg_spilling.bug31187562-2.r
create mode 100644 test/unittest/codegen/tst.stack_layout.r
create mode 100755 test/unittest/codegen/tst.stack_layout.sh
create mode 100644 test/utils/print-stack-layout.c
diff --git a/GNUmakefile b/GNUmakefile
index 22755446..5a1eb64a 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -87,8 +87,6 @@ DOCDIR := $(prefix)/share/doc/dtrace-$(VERSION)
INSTDOCDIR := $(DESTDIR)$(DOCDIR)
MANDIR := $(prefix)/share/man/man1
INSTMANDIR := $(DESTDIR)$(MANDIR)
-ETCDIR := /etc
-INSTETCDIR := $(DESTDIR)$(ETCDIR)
TESTDIR := $(prefix)/lib$(BITNESS)/dtrace/testsuite
INSTTESTDIR := $(DESTDIR)$(TESTDIR)
TARGETS =
diff --git a/cmd/Build b/cmd/Build
index fdad59e6..bde7debe 100644
--- a/cmd/Build
+++ b/cmd/Build
@@ -54,7 +54,7 @@ fi\n"
install::
mkdir -p $(INSTBINDIR) $(INSTSBINDIR) $(INSTMANDIR) \
- $(INSTLIBDIR)/dtrace $(INSTETCDIR)
+ $(INSTLIBDIR)/dtrace
$(call describe-install-target,$(INSTSBINDIR),dtrace)
install -m 755 $(objdir)/dtrace $(INSTSBINDIR)
$(call describe-install-target,$(INSTBINDIR),ctf_module_dump)
diff --git a/include/bpf_asm.h b/include/bpf_asm.h
index 7891a597..59484697 100644
--- a/include/bpf_asm.h
+++ b/include/bpf_asm.h
@@ -15,6 +15,18 @@
#endif
#define BPF_REG_FP BPF_REG_10
+/*
+ * The maximum stack size for BPF programs is defined in the kernel headers and
+ * is not generally available outside of the kernel source tree. We define it
+ * here because we depend on it in the layout of the DTrace BPF program stack.
+ * If the kernel supports a larger stack, we end up not taking advantage of it.
+ * If the kernel supports a smaller stack, we may end up generating programs
+ * that the BPF verifier will reject.
+ */
+#ifndef MAX_BPF_STACK
+# define MAX_BPF_STACK 512
+#endif
+
#define BPF_ALU64_REG(op, dst, src) \
((struct bpf_insn) { \
.code = BPF_ALU64 | (op) | BPF_X, \
diff --git a/libdtrace/dt_cc.c b/libdtrace/dt_cc.c
index 770ee4f3..5319c173 100644
--- a/libdtrace/dt_cc.c
+++ b/libdtrace/dt_cc.c
@@ -2065,7 +2065,7 @@ dt_compile(dtrace_hdl_t *dtp, int context, dtrace_probespec_t pspec, void *arg,
yypcb->pcb_idents = dt_idhash_create("ambiguous", NULL, 0, 0);
yypcb->pcb_locals = dt_idhash_create("clause local", NULL,
- 0, DT_VAR_LOCAL_MAX);
+ 0, DT_LVAR_MAX);
if (yypcb->pcb_idents == NULL || yypcb->pcb_locals == NULL)
longjmp(yypcb->pcb_jmpbuf, EDT_NOMEM);
diff --git a/libdtrace/dt_dis.c b/libdtrace/dt_dis.c
index 9b272958..1da56198 100644
--- a/libdtrace/dt_dis.c
+++ b/libdtrace/dt_dis.c
@@ -61,8 +61,8 @@ static char *
dt_dis_lvarname(const dtrace_difo_t *dp, int reg, int var, char *buf, int len)
{
if (reg == BPF_REG_FP) {
- var = DT_STK_LVAR_ID(var);
- if (var >= 0 && var < DT_VAR_LOCAL_MAX) {
+ var = DT_LVAR_OFF2ID(var);
+ if (var != -1) {
const char *vname;
vname = dt_dis_varname(dp, var, DIFV_SCOPE_LOCAL);
diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
index dd4128d4..8f360047 100644
--- a/libdtrace/dt_impl.h
+++ b/libdtrace/dt_impl.h
@@ -24,6 +24,7 @@
#include <math.h>
#include <string.h>
#include <stddef.h>
+#include <bpf_asm.h>
#ifndef PATH_MAX
#define PATH_MAX 1024
@@ -457,25 +458,81 @@ struct dtrace_hdl {
#define DT_USYMADDR_CTFP(dtp) ((dtp)->dt_ddefs->dm_ctfp)
#define DT_USYMADDR_TYPE(dtp) ((dtp)->dt_type_usymaddr)
+/*
+ * DTrace BPF programs can use all BPF registers except for the the %fp (frame
+ * pointer) register and the highest numbered register (currently %r9) that is
+ * used to store the base pointer for the trace output record.
+ */
+#define DT_STK_NREGS (MAX_BPF_REG - 2)
+
+/*
+ * An area of 256 bytes is set aside on the stack as scratch area for things
+ * like string operations. It extends from the end of the stack towards the
+ * local variable storage area. DT_STK_LVAR_END is therefore the last byte
+ * location on the stack that can be used for local variable storage.
+ */
+#define DT_STK_SCRATCH_BASE (-MAX_BPF_STACK)
+#define DT_STK_SCRATCH_SZ (256)
+#define DT_STK_LVAR_END (DT_STK_SCRATCH_BASE + DT_STK_SCRATCH_SZ)
+
/*
* The stack layout for functions that implement a D clause is encoded with the
* following constants.
+ *
+ * Note: The BPF frame pointer points to the address of the first byte past the
+ * end of the stack. If the stack size is 512 bytes, valid offsets are
+ * -1 through -512 (inclusive), So, the first 64-bit value on the stack
+ * occupies bytes at offsets -8 through -1. The second -16 through -9,
+ * and so on... 64-bit values are properly aligned at offsets -n where
+ * n is a multiple of 8 (sizeof(uint64_t)).
+ *
+ * The following diagram shows the stack layout for a size of 512 bytes.
+ *
+ * +----------------+
+ * SCRATCH_BASE = -512 | Scratch Memory |
+ * +----------------+
+ * LVAR_END = LVAR(n) = -256 | LVAR n | (n = DT_VAR_LOCAL_MAX = 19)
+ * +----------------+
+ * | ... |
+ * +----------------+
+ * LVAR(1) = -112 | LVAR 1 |
+ * +----------------+
+ * LVAR_BASE = LVAR(0) = -104 | LVAR 0 |
+ * +----------------+
+ * SPILL(n) = -96 | %r8 | (n = DT_STK_NREGS - 1 = 8)
+ * +----------------+
+ * | ... |
+ * +----------------+
+ * SPILL(1) = -40 | %r1 |
+ * +----------------+
+ * SPILL_BASE = SPILL(0) = -32 | %r0 |
+ * +----------------+
+ * CPU = -24 | CPU ID |
+ * +----------------+
+ * DCTX = -16 | DTrace Context |
+ * +----------------+
+ * CTX = -8 | BPF Context | -1
+ * +----------------+
+ */
+#define DT_STK_BASE (0)
+#define DT_STK_SLOT_SZ ((int)sizeof(uint64_t))
+
+#define DT_STK_CTX (DT_STK_BASE - DT_STK_SLOT_SZ)
+#define DT_STK_DCTX (DT_STK_CTX - DT_STK_SLOT_SZ)
+#define DT_STK_CPU (DT_STK_DCTX - DT_STK_SLOT_SZ)
+#define DT_STK_SPILL_BASE (DT_STK_CPU - DT_STK_SLOT_SZ)
+#define DT_STK_SPILL(n) (DT_STK_SPILL_BASE - (n) * DT_STK_SLOT_SZ)
+#define DT_STK_LVAR_BASE (DT_STK_SPILL(DT_STK_NREGS - 1) - \
+ DT_STK_SLOT_SZ)
+#define DT_STK_LVAR(n) (DT_STK_LVAR_BASE - (n) * DT_STK_SLOT_SZ)
+
+/*
+ * Calculate a local variable ID based on a given stack offset. If the stack
+ * offset is outside the valid range, this should evaluate as -1.
*/
-#define DT_STK_CTX -8
-#define DT_STK_DCTX -16
-#define DT_STK_CPU -24
-#define DT_STK_SPILL(n) ((DT_STK_CPU) - 8 * (n))
-#define DT_STK_R1 DT_STK_SPILL(1)
-#define DT_STK_R2 DT_STK_SPILL(2)
-#define DT_STK_R3 DT_STK_SPILL(3)
-#define DT_STK_R4 DT_STK_SPILL(4)
-#define DT_STK_R5 DT_STK_SPILL(5)
-#define DT_STK_R6 DT_STK_SPILL(6)
-#define DT_STK_R7 DT_STK_SPILL(7)
-#define DT_STK_LVAR(n) ((DT_STK_R7) - 8 * (n))
-#define DT_STK_LVAR_ID(n) (-((n) - (DT_STK_R7)) / 8)
-#define DT_STK_SCRATCH -512
-#define DT_STK_SCRATCH_SIZE 256
+#define DT_LVAR_OFF2ID(n) (((n) > DT_STK_LVAR_BASE || \
+ (n) < DT_STK_LVAR_END) ? -1 : \
+ (- ((n) - DT_STK_LVAR_BASE) / DT_STK_SLOT_SZ))
/*
* Maximum number of local variables stored by value (scalars). This is bound
@@ -483,8 +540,8 @@ struct dtrace_hdl {
* and 256 bytes set aside as string scratch space. We also use the fact that
* the (current) maximum stack space for BPF programs is 512 bytes.
*/
-#define DT_VAR_LOCAL_MAX ((-(DT_STK_SCRATCH) - (DT_STK_SCRATCH_SIZE) + \
- (DT_STK_R7)) / 8)
+#define DT_LVAR_MAX (- (DT_STK_LVAR_END - DT_STK_LVAR_BASE) / \
+ DT_STK_SLOT_SZ)
/*
* Actions and subroutines are both DT_NODE_FUNC nodes; to avoid confusing
diff --git a/test/Build b/test/Build
index 7c5bf31d..853fcc79 100644
--- a/test/Build
+++ b/test/Build
@@ -6,7 +6,7 @@ test_DIR := $(current-dir)
install-test::
mkdir -p $(INSTTESTDIR)/test
set -e; \
- for name in $(filter-out test/triggers test/Build,$(wildcard $(test_DIR)*)); do \
+ for name in $(filter-out test/triggers test/utils test/Build,$(wildcard $(test_DIR)*)); do \
printf "INSTALL: %s\n" "$(INSTTESTDIR)/$$name"; \
rm -rf "$(INSTTESTDIR)/test/$$(basename $$name)"; \
cp -a $$name "$(INSTTESTDIR)/test/$$(basename $$name)"; \
diff --git a/test/unittest/codegen/tst.reg_spilling.bug31187562-2.d b/test/unittest/codegen/tst.reg_spilling.bug31187562-2.d
new file mode 100644
index 00000000..76e0663f
--- /dev/null
+++ b/test/unittest/codegen/tst.reg_spilling.bug31187562-2.d
@@ -0,0 +1,30 @@
+/*
+ * 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.
+ * Test some other stack variables for corruption.
+ */
+
+BEGIN
+{
+ this->x = this->y = this-> z = 123456789;
+ a = b = c = d = e = f = g = h = i = j = k = 87654321;
+
+ /* cause register spills */
+ trace(a++ & (b++ & (c++ & (d++ & (e++ & (f++ & (g++ & (h++ & (i++ & (j++ & k++))))))))));
+ trace(a++ & (b++ & (c++ & (d++ & (e++ & (f++ & (g++ & (h++ & (i++ & (j++ & k++))))))))));
+ trace(a++ & (b++ & (c++ & (d++ & (e++ & (f++ & (g++ & (h++ & (i++ & (j++ & k++))))))))));
+
+ /* check local variables (on the stack near spilled registers) */
+ trace(this->x);
+ trace(this->y);
+ trace(this->z);
+
+ exit(0);
+}
diff --git a/test/unittest/codegen/tst.reg_spilling.bug31187562-2.r b/test/unittest/codegen/tst.reg_spilling.bug31187562-2.r
new file mode 100644
index 00000000..02577a15
--- /dev/null
+++ b/test/unittest/codegen/tst.reg_spilling.bug31187562-2.r
@@ -0,0 +1,5 @@
+ FUNCTION:NAME
+ :BEGIN 87654321 87654322 87654323 123456789 123456789 123456789
+
+-- @@stderr --
+dtrace: script 'test/unittest/codegen/tst.reg_spilling.bug31187562-2.d' matched 1 probe
diff --git a/test/unittest/codegen/tst.stack_layout.r b/test/unittest/codegen/tst.stack_layout.r
new file mode 100644
index 00000000..f9afa815
--- /dev/null
+++ b/test/unittest/codegen/tst.stack_layout.r
@@ -0,0 +1,19 @@
+Base: 0
+ctx: -8
+dctx: -16
+cpu: -24
+%r0: -32
+%r1: -40
+%r2: -48
+%r3: -56
+%r4: -64
+%r5: -72
+%r6: -80
+%r7: -88
+%r8: -96
+lvar[ -1]: -103 (ID -1)
+lvar[ 0]: -104 (ID 0)
+lvar[ 1]: -112 (ID 1)
+lvar[ 19]: -256 (ID 19)
+lvar[ -1]: -257 (ID -1)
+scratch: -257 .. -512
diff --git a/test/unittest/codegen/tst.stack_layout.sh b/test/unittest/codegen/tst.stack_layout.sh
new file mode 100755
index 00000000..1fccf2fc
--- /dev/null
+++ b/test/unittest/codegen/tst.stack_layout.sh
@@ -0,0 +1,14 @@
+#!/bin/bash
+#
+# 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.
+#
+
+#
+# This script verifies that the DTrace BPF program stack layout constants have
+# the proper values.
+#
+
+exec test/utils/print-stack-layout
diff --git a/test/utils/.gitignore b/test/utils/.gitignore
index 68a6921f..d399bd68 100644
--- a/test/utils/.gitignore
+++ b/test/utils/.gitignore
@@ -2,3 +2,4 @@
baddof
badioctl
showUSDT
+print-stack-layout
diff --git a/test/utils/Build b/test/utils/Build
index 0e762a3a..2c366a54 100644
--- a/test/utils/Build
+++ b/test/utils/Build
@@ -1,9 +1,9 @@
# Oracle Linux DTrace.
-# Copyright (c) 2011, 2013, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2011, 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.
-TEST_UTILS = baddof badioctl showUSDT
+TEST_UTILS = baddof badioctl showUSDT print-stack-layout
define test-util-template
CMDS += $(1)
@@ -11,7 +11,7 @@ $(1)_DIR := $(current-dir)
$(1)_TARGET = $(1)
$(1)_SOURCES = $(1).c
$(1)_POST := link-test-util
-$(1)_CFLAGS := -Ilibdtrace
+$(1)_CFLAGS := -Ilibdtrace -Ilibproc
$(1)_NOCFLAGS := --coverage
$(1)_NOLDFLAGS := --coverage
$(1)_DEPS = libdtrace.so
@@ -33,6 +33,9 @@ $(foreach util,$(TEST_UTILS),$(eval $(call test-util-template,$(util))))
showUSDT_DEPS =
showUSDT_LIBS = -lelf
+# The print-stack-layout utility needs dt_impl.h which needs dt_git_version.h.
+print-stack-layout_SRCDEPS := $(objdir)/dt_git_version.h
+
# Install the showUSDT utility in $(DOCDIR); do not count on this utility to
# always be present --- it is only included there to assist in early debugging of
# the (semi-new) USDT feature.
@@ -40,3 +43,9 @@ install::
mkdir -p $(INSTDOCDIR)
$(call describe-install-target,$(INSTDOCDIR),showUSDT)
install -m 755 test/utils/showUSDT $(INSTDOCDIR)
+
+install-test::
+ $(call describe-install-target,$(INSTTESTDIR)/test/utils,$(TEST_UTILS) include-test.d)
+ mkdir -p $(INSTTESTDIR)/test/utils
+ install -m 755 $(addprefix test/utils/,$(TEST_UTILS)) $(INSTTESTDIR)/test/utils
+ install -m 644 test/utils/include-test.d $(INSTTESTDIR)/test/utils
diff --git a/test/utils/print-stack-layout.c b/test/utils/print-stack-layout.c
new file mode 100644
index 00000000..fc50f92b
--- /dev/null
+++ b/test/utils/print-stack-layout.c
@@ -0,0 +1,41 @@
+/*
+ * 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.
+ */
+
+#include <stdio.h>
+#include <dt_impl.h>
+
+int main(void)
+{
+ printf("Base: % 4d\n", DT_STK_BASE);
+ printf("ctx: % 4d\n", DT_STK_CTX);
+ printf("dctx: % 4d\n", DT_STK_DCTX);
+ printf("cpu: % 4d\n", DT_STK_CPU);
+ printf("%%r0: % 4d\n", DT_STK_SPILL(0));
+ printf("%%r1: % 4d\n", DT_STK_SPILL(1));
+ printf("%%r2: % 4d\n", DT_STK_SPILL(2));
+ printf("%%r3: % 4d\n", DT_STK_SPILL(3));
+ printf("%%r4: % 4d\n", DT_STK_SPILL(4));
+ printf("%%r5: % 4d\n", DT_STK_SPILL(5));
+ printf("%%r6: % 4d\n", DT_STK_SPILL(6));
+ printf("%%r7: % 4d\n", DT_STK_SPILL(7));
+ printf("%%r8: % 4d\n", DT_STK_SPILL(8));
+ printf("lvar[% 3d]: % 4d (ID % 3d)\n", -1, DT_STK_LVAR(0) + 1,
+ DT_LVAR_OFF2ID(DT_STK_LVAR(0) + 1));
+ printf("lvar[% 3d]: % 4d (ID % 3d)\n", 0, DT_STK_LVAR(0),
+ DT_LVAR_OFF2ID(DT_STK_LVAR(0)));
+ printf("lvar[% 3d]: % 4d (ID % 3d)\n", 1, DT_STK_LVAR(1),
+ DT_LVAR_OFF2ID(DT_STK_LVAR(1)));
+ printf("lvar[% 3d]: % 4d (ID % 3d)\n", DT_LVAR_MAX,
+ DT_STK_LVAR(DT_LVAR_MAX),
+ DT_LVAR_OFF2ID(DT_STK_LVAR(DT_LVAR_MAX)));
+ printf("lvar[% 3d]: % 4d (ID % 3d)\n", -1,
+ DT_STK_LVAR(DT_LVAR_MAX) - 1,
+ DT_LVAR_OFF2ID(DT_STK_LVAR(DT_LVAR_MAX) - 1));
+ printf("scratch: % 4d .. % 4d\n", DT_STK_LVAR_END - 1,
+ DT_STK_SCRATCH_BASE);
+ exit(0);
+}
--
2.26.0
More information about the DTrace-devel
mailing list