dtrace: incorrect aframes value and wrong logic messes up caller and stack

Due to a mistake in how we compensate for the potential ULONG_MAX
sentinel value being added to kernel stacks on x86_64 (by the
save_stack_trace() function), the caller was always reported as 0.

This in turn was hiding a problem with the aframes values that are
used to ensure we skip the right amount of frames when reporting a
stack, caller, and calculating the stackdepth.  Effectively, it tells
the stack walker how many frames were added to the stack due to DTrace
processing.

Orabug: 25727046
Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>

diff --git a/dtrace/dtrace_dif.c b/dtrace/dtrace_dif.c
index 046f314a40d169fc83216ce81e030b7e3b1bc5b9..96598797f996689797bf612e1004e2745846f19b 100644 (file)
--- a/dtrace/dtrace_dif.c
+++ b/dtrace/dtrace_dif.c
@@ -2152,7 +2152,7 @@ static uint64_t dtrace_dif_variable(dtrace_mstate_t *mstate,
                        return 0;
 
                if (!(mstate->dtms_present & DTRACE_MSTATE_CALLER)) {
-                       int     aframes = mstate->dtms_probe->dtpr_aframes + 2;
+                       int     aframes = mstate->dtms_probe->dtpr_aframes + 3;
 
                        if (!DTRACE_ANCHORED(mstate->dtms_probe)) {
                                /*

diff --git a/dtrace/dtrace_isa.c b/dtrace/dtrace_isa.c
index 3d05586643a6fae46b3233fc82221af342353593..124806026547a539b8806b2fe628904148584d27 100644 (file)
--- a/dtrace/dtrace_isa.c
+++ b/dtrace/dtrace_isa.c
@@ -121,6 +121,10 @@ void dtrace_copyinstr(uintptr_t uaddr, uintptr_t kaddr, size_t size,
        dtrace_copyinstr_arch(uaddr, kaddr, size, flags);
 }
 
+/*
+ * FIXME: aframes + 3 should really be aframes + 1, dtrace_stacktrace() in the
+ *       kernel should do its own aframes + 2
+ */
 void dtrace_getpcstack(uint64_t *pcstack, int pcstack_limit, int aframes,
                       uint32_t *intrpc)
 {
@@ -128,7 +132,7 @@ void dtrace_getpcstack(uint64_t *pcstack, int pcstack_limit, int aframes,
                                        pcstack,
                                        NULL,
                                        pcstack_limit,
-                                       aframes,
+                                       aframes + 3,
                                        STACKTRACE_KERNEL
                                     };
 
@@ -187,6 +191,10 @@ void dtrace_getupcstack(uint64_t *pcstack, int pcstack_limit)
        dtrace_getufpstack(pcstack, NULL, pcstack_limit);
 }
 
+/*
+ * FIXME: aframes + 3 should really be aframes + 1, dtrace_stacktrace() in the
+ *       kernel should do its own aframes + 2
+ */
 int dtrace_getstackdepth(dtrace_mstate_t *mstate, int aframes)
 {
        uintptr_t               old = mstate->dtms_scratch_ptr;
@@ -194,7 +202,7 @@ int dtrace_getstackdepth(dtrace_mstate_t *mstate, int aframes)
                                        NULL,
                                        NULL,
                                        0,
-                                       aframes,
+                                       aframes + 3,
                                        STACKTRACE_KERNEL
                                     };
 
@@ -212,7 +220,9 @@ int dtrace_getstackdepth(dtrace_mstate_t *mstate, int aframes)
        st.limit = (mstate->dtms_scratch_base + mstate->dtms_scratch_size -
                    (uintptr_t)st.pcs) >> 3;
 
+       DTRACE_CPUFLAG_SET(CPU_DTRACE_NOFAULT);
        dtrace_stacktrace(&st);
+       DTRACE_CPUFLAG_CLEAR(CPU_DTRACE_NOFAULT);
 
        mstate->dtms_scratch_ptr = old;
 

diff --git a/dtrace/dtrace_probe.c b/dtrace/dtrace_probe.c
index c06089ab0864b41f67608b2c1451fd5f1ea57c41..0dac1e6283d1bf9031a15f32d5960b383cd10051 100644 (file)
--- a/dtrace/dtrace_probe.c
+++ b/dtrace/dtrace_probe.c
@@ -850,7 +850,7 @@ void dtrace_probe(dtrace_id_t id, uintptr_t arg0, uintptr_t arg1,
                                dtrace_getpcstack(
                                        (uint64_t *)(tomax + valoffs),
                                        size / sizeof(pc_t),
-                                       probe->dtpr_aframes,
+                                       probe->dtpr_aframes + 1,
                                        DTRACE_ANCHORED(probe)
                                                ? NULL
                                                : (uint32_t *)arg0);

diff --git a/dtrace/fbt_dev.c b/dtrace/fbt_dev.c
index 0b5d0f2ef490b93edce0851dfa5769b883b8bde0..2d292a0a20e52e327971a3c548d5b6d69fea17ed 100644 (file)
--- a/dtrace/fbt_dev.c
+++ b/dtrace/fbt_dev.c
@@ -53,7 +53,7 @@ static void *fbt_provide_probe(struct module *mp, char *func, int type, int
                fbp = kzalloc(sizeof(fbt_probe_t), GFP_KERNEL);
                fbp->fbp_name = kstrdup(func, GFP_KERNEL);
                fbp->fbp_id = dtrace_probe_create(fbt_id, mp->name, func,
-                                                 "entry", 3, fbp);
+                                                 "entry", FBT_AFRAMES, fbp);
                fbp->fbp_module = mp;
                fbp->fbp_loadcnt = 1; /* FIXME */
                fbp->fbp_primary = 1; /* FIXME */
@@ -76,8 +76,8 @@ static void *fbt_provide_probe(struct module *mp, char *func, int type, int
                        fbp->fbp_id = prev->fbp_id;
                } else {
                        fbp->fbp_id = dtrace_probe_create(fbt_id, mp->name,
-                                                         func, "return", 3,
-                                                         fbp);
+                                                         func, "return",
+                                                         FBT_AFRAMES, fbp);
                }
 
                fbp->fbp_module = mp;

diff --git a/dtrace/fbt_impl.h b/dtrace/fbt_impl.h
index c86355a37e773660cc2819a18ae6eb34c4d5465d..530342d6256e113850efb44fd9be48eaa7d779fa 100644 (file)
--- a/dtrace/fbt_impl.h
+++ b/dtrace/fbt_impl.h
@@ -2,6 +2,7 @@
 #define _FBT_H_
 
 #include <asm/dtrace_arch.h>
+#include <dtrace/fbt_arch.h>
 
 typedef struct fbt_probe {
         char                   *fbp_name;      /* name of probe */

diff --git a/dtrace/include/sparc64/dtrace/fbt_arch.h b/dtrace/include/sparc64/dtrace/fbt_arch.h
new file mode 100644 (file)
index 0000000..473f4a3
--- /dev/null
+++ b/dtrace/include/sparc64/dtrace/fbt_arch.h
@@ -0,0 +1,37 @@
+#ifndef _SPARC64_FBT_ARCH_H
+#define _SPARC64_FBT_ARCH_H
+
+/*
+ * Function Boundary Tracing Implementation defines
+ *
+ * Note: The contents of this file are private to the implementation of the
+ * DTrace subsystem and are subject to change at any time without notice.
+ */
+
+/*
+ * CDDL HEADER START
+ *
+ * The contents of this file are subject to the terms of the
+ * Common Development and Distribution License (the "License").
+ * You may not use this file except in compliance with the License.
+ *
+ * You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+ * or http://www.opensolaris.org/os/licensing.
+ * See the License for the specific language governing permissions
+ * and limitations under the License.
+ *
+ * When distributing Covered Code, include this CDDL HEADER in each
+ * file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+ * If applicable, add the following below this CDDL HEADER, with the
+ * fields enclosed by brackets "[]" replaced with your own identifying
+ * information: Portions Copyright [yyyy] [name of copyright owner]
+ *
+ * CDDL HEADER END
+ *
+ * Copyright 2009-2014 Oracle, Inc.  All rights reserved.
+ * Use is subject to license terms.
+ */
+
+#define FBT_AFRAMES    1
+
+#endif /* _SPARC64_FBT_ARCH_H */

diff --git a/dtrace/include/x86_64/dtrace/fbt_arch.h b/dtrace/include/x86_64/dtrace/fbt_arch.h
new file mode 100644 (file)
index 0000000..0b9a269
--- /dev/null
+++ b/dtrace/include/x86_64/dtrace/fbt_arch.h
@@ -0,0 +1,37 @@
+#ifndef _X86_64_FBT_ARCH_H
+#define _X86_64_FBT_ARCH_H
+
+/*
+ * Function Boundary Tracing Implementation defines
+ *
+ * Note: The contents of this file are private to the implementation of the
+ * DTrace subsystem and are subject to change at any time without notice.
+ */
+
+/*
+ * CDDL HEADER START
+ *
+ * The contents of this file are subject to the terms of the
+ * Common Development and Distribution License (the "License").
+ * You may not use this file except in compliance with the License.
+ *
+ * You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+ * or http://www.opensolaris.org/os/licensing.
+ * See the License for the specific language governing permissions
+ * and limitations under the License.
+ *
+ * When distributing Covered Code, include this CDDL HEADER in each
+ * file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+ * If applicable, add the following below this CDDL HEADER, with the
+ * fields enclosed by brackets "[]" replaced with your own identifying
+ * information: Portions Copyright [yyyy] [name of copyright owner]
+ *
+ * CDDL HEADER END
+ *
+ * Copyright 2017 Oracle, Inc.  All rights reserved.
+ * Use is subject to license terms.
+ */
+
+#define FBT_AFRAMES    9
+
+#endif /* _X86_64_FBT_ARCH_H */