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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to