On 09/04/20(Thu) 22:58, George Koehler wrote: > On Thu, 9 Apr 2020 20:05:34 +0200 > Martin Pieuchot <m...@openbsd.org> wrote: > [...] > 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.
Thanks for the debugging and explanation George! I committed the function it was. I hope you or someone else can fix it ;)
