Hi, Petr pointed out on irc this patch had slipped through the cracks and hadn't been reviewed yet. Sorry about that. I like the idea, but have some questions on the heuristics used to detect the corrupt stack frame.
On Fri, Jan 03, 2014 at 10:18:09PM +0100, Jan Kratochvil wrote: > I have found I forgot to implement what GDB has as > gdb.dwarf2/dw2-dup-frame.exp > Backtrace stopped: previous frame identical to this frame (corrupt > stack?) > > I find it easier in the case of elfutils as all supported unwinding archs have > grows-down kind of stack. > > It breaks support for -fsplit-stack (GDB gdb.base/morestack.exp) but that is > IMO an add-on extension which just coincidentally worked with current > checked-in elfutils unwinder (I did not test it). I don't think it is coincidence that split-stack would work. I believe we don't assume anything about the stack values and so it should work as expected since DWARF is expressive enough to describe it. http://gcc.gnu.org/wiki/SplitStacks#Debugging Using different stacks seems like a valid execution model and something that is also used by things like sigaltstack. So if we assume CFA values correspond to stack values then detecting corruption through ordering of the CFA values seems not ideal. We could use the PC combined with the CFA or SP as frame identifier. If both are equal to the values in the previous frame then the unwind stack frame is corrupted. > + // Set unwound->CFA. > + Dwarf_Op *cfa_ops; > + size_t cfa_nops; > + if (dwarf_frame_cfa (frame, &cfa_ops, &cfa_nops) != 0 > + || ! expr_eval (state, cfa_ops, cfa_nops, &unwound->cfa, bias, > elfclass)) > + { > + __libdwfl_seterrno (DWFL_E_LIBDW); > + return; > + } > + if (unwound->cfa == 0 > + || (! state->initial_frame && unwound->cfa <= state->cfa)) > + { > + __libdwfl_seterrno (DWFL_E_UNWIND_BAD_CFA); > + return; > + } So I don't think the "<=" is correct here. That should be "==" and I would combine it with checking the PC because the CFA/stack could be reused. Another question is whether it is guaranteed that the CFA is always defined. I think the standard says an activation doesn't need to have a CFA defined. If not, then we probably should use the value of the SP register instead. The problem with using the SP value is that it is architecture specific. But we could define the register value through ebl. Thanks, Mark
