[Lldb-commits] [PATCH] D21221: Fix for PrintStackTraces

[Jason Molenda via lldb-commits]

jasonmolenda added a comment.

Hi Ravi, sorry for not having time to look at this patch yet - I was out of the office Friday and was catching up on everything today.  I'll look at this tomorrow.  My initial reactions are to be a little worried.  It sounds like you have functions that are non-ABI conforming -- which is fine for functions that don't have external linkage -- and you're tweaking the unwinder until it works for this case.  When we were discussing this on lldb-dev a couple of weeks ago, the functions in question had no eh_frame code.  Why are you forcing eh_frame for frame 0?  Is this a different case than the one you were debugging ten days ago?  One initial comment is I don't know about adding AlwaysRelyOnEHUnwindInfo to the DynamicLoader - the section of code you added this check to in RegisterContextLLDB is there because we had one or two solibs on darwin where we had to use eh_frame, hence it made sense to ask the DynamicLoader plugin.  If we were to add something like AlwaysRelyOnEHUnwindInfo for an entire platform, I'd probably put it at FuncUnwinders::GetUnwindPlanAtNonCallSite.  But I really don't think that's a good idea unless there's a concrete example where a non-ABI-conforming function actually has eh_frame that describes how to correctly unwind from it.

(my initial opinion is that a debugger trying to unwind out of non-ABI conforming functions without hand-written eh_frame describing how to do it, are in the "it would be nice if it worked" camp; I'd be reluctant to make changes that could compromise lldb otherwise to support this kind of scenario.)

But I really haven't given the patch or the discussion a fair read-through and thought about it more - I'll do that tomorrow.


http://reviews.llvm.org/D21221