On Thu, 9 Apr 2020 20:05:34 +0200
Martin Pieuchot <[email protected]> wrote:
> +void
> +stacktrace_save_at(struct stacktrace *st, unsigned int skip)
> +{
> + vaddr_t lr, sp, lastsp;
> +
> + sp = (vaddr_t)__builtin_frame_address(0);
> + if (!INKERNEL(sp) && !ININTSTK(sp))
> + return;
> +
> + st->st_count = 0;
> + while (st->st_count < STACKTRACE_MAX) {
> + lr = *(vaddr_t *)(sp + 4) - 4;
> + if (lr & 3)
> + break;
> +
> + if (skip == 0)
> + st->st_pc[st->st_count++] = lr;
> + else
> + skip--;
> +
> + lastsp = sp;
> + sp = *(vaddr_t *)sp;
> +
> + if ((sp == 0) || (sp & 3) || (sp <= lastsp))
> + break;
> + if (!INKERNEL(sp) && !ININTSTK(sp))
> + break;
> + }
> +}
In the trace, #0 and #1 are wrong, but the rest of the trace looks
good enough for WITNESS. I added an artificial lock order reversal to
ums(4) for WITNESS to catch. I got this trace,
#0 0xe4d764
#1 witness_checkorder+0x308
#2 mtx_enter+0x50
#3 ums_attach+0x68
#4 config_attach+0x270
...
"#0 0xe4d764" is stack garbage: a function saves its lr in its
caller's frame, but stacktrace_save_at() first reads the lr slot in
its own frame.
"#1 witness_checkorder+0x308" is a dead value. In the disassembly
(objdump -dlr db_trace.o), clang optimized stacktrace_save_at() to
skip saving its lr on the stack, because it is a leaf function (that
never calls other functions). The lr from the stack isn't the return
address for stacktrace_save_at(), but might be a leftover return
address from some other function (that needed to save lr).
#2 and after seem to be correct; "#3 ums_attach+0x68" points exactly
to where I am grabbing the second lock. This is enough for WITNESS,
so we might want to use your diff now, and fix #0 and #1 later.
Also know that a compiler may optimize stacktrace_save_at() to have
no stack frame. Our clang 8.0.1 never does this (because it always
sets r31 = stack pointer r1, so it always needs a stack frame to save
the caller's r31), but gcc and newer clang might. I don't know how
__builtin_frame_address(0) works if the stack frame is gone.
--George
