From 2f2c94431f032eb66796bac71f2512f92d451aaf Mon Sep 17 00:00:00 2001 From: Kris Van Hees Date: Wed, 15 Mar 2017 12:52:01 -0400 Subject: [PATCH] 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 --- dtrace/dtrace_dif.c | 2 +- dtrace/dtrace_isa.c | 14 +++++++-- dtrace/dtrace_probe.c | 2 +- dtrace/fbt_dev.c | 6 ++-- dtrace/fbt_impl.h | 1 + dtrace/include/sparc64/dtrace/fbt_arch.h | 37 ++++++++++++++++++++++++ dtrace/include/x86_64/dtrace/fbt_arch.h | 37 ++++++++++++++++++++++++ 7 files changed, 92 insertions(+), 7 deletions(-) create mode 100644 dtrace/include/sparc64/dtrace/fbt_arch.h create mode 100644 dtrace/include/x86_64/dtrace/fbt_arch.h diff --git a/dtrace/dtrace_dif.c b/dtrace/dtrace_dif.c index 046f314a40d1..96598797f996 100644 --- 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 3d05586643a6..124806026547 100644 --- 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 c06089ab0864..0dac1e6283d1 100644 --- 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 0b5d0f2ef490..2d292a0a20e5 100644 --- 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 c86355a37e77..530342d6256e 100644 --- a/dtrace/fbt_impl.h +++ b/dtrace/fbt_impl.h @@ -2,6 +2,7 @@ #define _FBT_H_ #include +#include 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 index 000000000000..473f4a355536 --- /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 index 000000000000..0b9a269d77b4 --- /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 */ -- 2.50.1