Page MenuHomePhabricator

[lldb] Add GetCurrentException APIs to SBThread, add frame recognizer for objc_exception_throw for Obj-C runtimes
ClosedPublic

Authored by kubamracek on Feb 28 2018, 10:52 AM.

Details

Summary

This adds new APIs and a command to deal with exceptions (mostly Obj-C exceptions):

SBThread and Thread get GetCurrentException API, which returns an SBValue/ValueObjectSP with the current exception for a thread. "Current" means an exception that is currently being thrown, caught or otherwise processed. In this patch, we only know about the exception when in objc_exception_throw, but subsequent patches will expand this (and add GetCurrentExceptionBacktrace, which will return an SBThread/ThreadSP containing a historical thread backtrace retrieved from the exception object. Currently unimplemented, subsequent patches will implement this).

Extracting the exception from objc_exception_throw is implemented by adding a frame recognizer.

This also add a new sub-command "thread exception", which prints the current exception.

Diff Detail

Event Timeline

kubamracek created this revision.Feb 28 2018, 10:52 AM

I have a couple of structural questions.

First, it would be good to stop a bit and think about how to make the StackFrameRecognizer's return data more flexible. At some point, for instance, it would be great to be able to add a StackFrameRecognizerPython and then let people add their own plugins to handle special data from functions they know about in their code. Basically DataFormatters for functions on the stack. But if the way you get data out of a Recognizer is to call specific functions on the recognizer, then you're going to have to enumerate up front all the kinds of data a recognizer can provide, which will make this harder.

It seems however like providing some interesting ValueObjects is a pretty common thing to do, so maybe it would be better to have an API like:

ValueObjectSP GetValueObjectOfKind(const char *kind);

in which case you probably also want:

GetSupportedKinds()

that returns some kind of string list so you could anonymously print all the interesting information.

I actually like having a command to get the current exception (and it's backtrace) for a thread. We do an analogous thing when listing the queue that launched the current thread (and the backtrace when dispatched) with:

thread backtrace -e true

So now we have two very different ways to do analogous things. Maybe it would be better to change --extended to take an enum, and then do:

thread backtrace --extended queues

or

thread backtrace --extended exception

Thanks for the high-level feedback! I'll split the patch and we can discuss further.

Hi, I've moved the part that introduces the frame recognizers to https://reviews.llvm.org/D44603.

Are you planning to rewrite this the command part of this patch using the changes from https://reviews.llvm.org/D44603 once that's approved?

I like the fact that the part of this that prints exception objects gets access to them through the thread, not the frame. Even though the implementation in D44603 only knows how to get the exception object at the point of throw, at some point it would be nice to support the scenario where you stop sayin a breakpoint on a destructor that's getting run as the stack is unwound due to an exception, and we can tell you what exception is currently propagating. For that purpose, asking the thread seems right.

Are you planning to rewrite this the command part of this patch using the changes from https://reviews.llvm.org/D44603 once that's approved?

Yes.

Updating patch for the recently-landed StackFrameRecognizers.

Hi, I've updated this patch after a while. Since then, StackFrameRecognizers have already landed, so this patch now just adds one subclass of a StackFrameRecognizer.

It seems however like providing some interesting ValueObjects is a pretty common thing to do, so maybe it would be better to have an API like:
ValueObjectSP GetValueObjectOfKind(const char *kind);

For now, I've kept the new method, GetExceptionObject(). If you'd still like me to change it into a string-based API, let me know, I'll be happy to change it. I agree that it doesn't make sense to keep adding methods to the base RecognizedStackFrame class.

Maybe it would be better to change --extended to take an enum, and then do:
thread backtrace --extended exception

I can add that. On top of the backtrace, the exception has a class name and message (at least in Obj-C), which is also important to present via some command. Do you want to keep thread exception as well? Or make thread backtrace -e exception be the only way to access the class+message of an exception?

kubamracek retitled this revision from [lldb] Add GetCurrentException and GetCurrentExceptionBacktrace APIs to SBThread to [lldb] Add GetCurrentException APIs to SBThread, add frame recognizer for objc_exception_throw for Obj-C runtimes.
kubamracek edited the summary of this revision. (Show Details)

Simplifying this a bit further, adding a test, and commenting out the backtrace API, which I'll add in a subsequent commit.

The exception breakpoints know that libobjc.B.dylib`objc_exception_throw is where you stop for exception breakpoints, and now the ObjCExceptionRecognizer knows the same thing. It always makes me nervous when two different places have the same hard-coded string. Can we add an API to ObjCLanguageRuntime to get the FileSpec for the module and a function name for the exception function, and then have both CreateExceptionResolver and RegisterObjCExceptionRecognizer use that? Actually it would be better to just add this to the LanguageRuntime, because then we could use it for the C++ CreateExceptionResolver and later (when one of us gets to it) for RegisterCPlusPlusExceptionRecognizer as well. But since C++ can throw in a couple of places, it will need to return a vector of name+library pairs.

Other than this niggle, this looks great.

Updated patch. I've added a static AppleObjCRuntime::GetExceptionThrowLocation(), which returns the location of the exception throw breakpoint for the Obj-C runtime. I tried adding it as virtual method on LanguageRuntime instead, but then it wasn't clear to me how to make it easily return either an optional (there's runtimes that don't have exceptions) or an array (as you said, C++ will need multiple locations) and how to make existing code CreateExceptionResolver work with that. Let me know if that's okay.

jingham accepted this revision.Nov 28 2018, 1:24 PM

This isn't an external API, so we can generalize it when we get around to extending the Recognizers to C++ exceptions. This is fine for now.

Thanks for doing this!

This revision is now accepted and ready to land.Nov 28 2018, 1:24 PM
This revision was automatically updated to reflect the committed changes.

Landed, thanks for the review!