On Thu, Apr 09, 2020 at 10:58:29PM -0400, George Koehler wrote: <some cut>
> 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. I wonder if this is the stack to stacktrace_save()? Would sprinkling __inline's at the function declaration help that any? > "#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). I'm gonna boot my G5 in a sec (which I'm considering renaming tractor or jetengine) so that I can see that too. I also haven't had a witness event yet hoping on that in a second. > #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. Oh then __inline perhaps wouldn't work? > --George > Great debug George! Thanks! Best Regards, -peter
