Fix the check in StopInfoBreakpoint for "are we running an expression"

[jingham]

Summary

If you hit a breakpoint while running an expression we don't want to run other commands that might end up running expressions again because that doesn't currently work recursively. But the code that was checking there was checking "WasTheLastResumeForUserExpression" which isn't quite right, since that expression might have already completed - because it was part of the handling of the breakpoint before we got to the StopInfo. We are tracking whether we are currently running an expression, however, which is the correct thing to check. So I made a function to check that and changed the StopInfoBreakpoint to use it.

It's actually pretty hard to get into a situation where this causes problems, because the expression has to happen internally in lldb between the stop and the fetching of the stop event. However, that's just what the objc_exception_throw frame recognizer does. Some code that was added to the StopInfoBreakpoint turned this "oops I'm running an expression" into a resume - which was necessary for some Windows fix, but caused us to fail to stop for the objc_exception_throw breakpoint.

So I added a test for the built-in objc_exception_throw built-in recognizer. Without the fix the test would fail because we didn't stop at the exception throw point. This also tests that we got all the values right.

[JDevlieghere]

LGTM modulo naming preference

lldb/include/lldb/Target/Process.h

287–292

I would prefer IsRunningExpression for consistency with other IsFoo methods. It still conveys that the process is in the process of executing an expression. /end bikeshedding

[bulbazord]

LGTM. It'd be nice if we could have a test that wasn't specific to Darwin or ObjC though.

lldb/include/lldb/Target/Process.h

bool IsLastResumeForUserExpression() const {
     // If we haven't yet resumed the target, then it can't be for a user
     // expression...
     if (m_resume_id == 0)
       return false;
 
     return m_resume_id == m_last_user_expression_resume;
   }
 
+  bool IsRunningExpression() const {
+    // Don't return true if we are no longer running an expression:
+    if (m_running_user_expression || m_running_utility_function)
+      return true;
+    return false;
+  }
+
   void SetRunningUserExpression(bool on) {
     if (on)
       m_running_user_expression++;
     else
       m_running_user_expression--;
   }
 
   void SetRunningUtilityFunction(bool on) {

lldb/source/Target/StopInfo.cpp

if (thread_sp) {
 
           // We don't select threads as we go through them testing breakpoint
           // conditions and running commands. So we need to set the thread for
           // expression evaluation here:
           ThreadList::ExpressionExecutionThreadPusher thread_pusher(thread_sp);
 
           ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0));
           Process *process = exe_ctx.GetProcessPtr();
-          if (process->GetModIDRef().IsLastResumeForUserExpression()) {
+          if (process->GetModIDRef().IsRunningExpression()) {
             // If we are in the middle of evaluating an expression, don't run
             // asynchronous breakpoint commands or expressions.  That could
             // lead to infinite recursion if the command or condition re-calls
             // the function with this breakpoint.
             // TODO: We can keep a list of the breakpoints we've seen while
             // running expressions in the nested
             // PerformAction calls that can arise when the action runs a
             // function that hits another breakpoint, and only stop running

lldb/test/API/macosx/objc_exception_recognizer/Makefile

+OBJC_SOURCES := main.m
+LD_EXTRAS := -framework Foundation
+
+include Makefile.rules

lldb/test/API/macosx/objc_exception_recognizer/TestObjCRecognizer.py

+"""
+Test that the built in ObjC exception throw recognizer works
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+class TestObjCRecognizer(TestBase):
+
+    NO_DEBUG_INFO_TESTCASE = True
+
+    @skipUnlessDarwin
+    def test_exception_recognizer_sub_class(self):
+        """There can be many tests in a test case - describe this test here."""
+        self.build()
+        self.main_source_file = lldb.SBFileSpec("main.m")
+        self.objc_recognizer_test(True)
+        
+    @skipUnlessDarwin
+    def test_exception_recognizer_plain(self):
+        """There can be many tests in a test case - describe this test here."""
+        self.build()
+        self.main_source_file = lldb.SBFileSpec("main.m")
+        self.objc_recognizer_test(False)
+        
+    def objc_recognizer_test(self, sub_class):
+        """Make sure we stop at the exception and get all the fields out of the recognizer.
+           If sub_class is True, we make a subclass of NSException and throw that."""
+        if sub_class:
+            bkpt_string = "Set a breakpoint here for MyException"
+        else:
+            bkpt_string = "Set a breakpoint here for plain exception"
+            
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+                                   bkpt_string, self.main_source_file)
+
+        # Now turn on the ObjC Exception breakpoint and continue to hit it:
+        exception_bkpt = target.BreakpointCreateForException(lldb.eLanguageTypeObjC, False, True)
+        self.assertTrue(exception_bkpt.GetNumLocations() > 0, "Got some exception locations")
+
+        threads = lldbutil.continue_to_breakpoint(process, exception_bkpt)
+        self.assertEqual(len(threads), 1, "One thread hit exception breakpoint")
+        frame = threads[0].frame[0]
+
+        var_opts = lldb.SBVariablesOptions()
+        var_opts.SetIncludeRecognizedArguments(True)
+        var_opts.SetUseDynamic(True)
+        vars = frame.GetVariables(var_opts)
+        self.assertEqual(len(vars), 1, "Got the synthetic argument")
+        self.assertTrue(vars[0].IsValid(), "Got a valid Exception variable")
+
+        # This will be a pointer
+
+        ns_exception_children = [ValueCheck(type="NSObject"),
+                                 ValueCheck(name="name", summary='"NSException"'),
+                                 ValueCheck(name="reason", summary='"Simple Reason"'),
+                                 ValueCheck(name="userInfo"),
+                                 ValueCheck(name="reserved")]
+        ns_exception = ValueCheck(type="NSException", children=ns_exception_children) 
+        if not sub_class:
+            simple_check = ValueCheck(name="exception", dereference=ns_exception)
+            simple_check.check_value(self, vars[0], "Simple exception is right")
+        else:
+            my_exception_children = [ns_exception, ValueCheck(name="extra_info", type="int", value="100")]
+            my_exception = ValueCheck(type="MyException", children=my_exception_children)
+            sub_check = ValueCheck(name="exception", type="MyException *", dereference=my_exception)
+            sub_check.check_value(self, vars[0], "Subclass exception is right")

lldb/test/API/macosx/objc_exception_recognizer/main.m

+#import <Foundation/Foundation.h>
+
+@interface MyException : NSException
+{
+  int extra_info;
+}
+- (NSException *) initWithExtraInfo: (int) info;
+@end
+
+@implementation MyException
+- (NSException *) initWithExtraInfo: (int) info
+{
+  [super initWithName: @"NSException" reason: @"Simple Reason" userInfo: nil];
+  self->extra_info = info;
+  return self;
+}
+@end
+
+int
+main(int argc, char **argv)
+{
+  // Set a breakpoint here for plain exception:
+  @try {
+    NSException *plain_exc = [[NSException alloc] initWithName: @"NSException" reason: @"Simple Reason" userInfo: nil];
+    [plain_exc raise];
+  }
+  @catch (id anException) {}
+
+  // Set a breakpoint here for MyException:
+  @try {
+    MyException *my_exc = [[MyException alloc] initWithExtraInfo: 100];
+    [my_exc raise];
+  }
+  @catch (id anException) {}
+
+  return 0;
+}