Adapt the ObjC checker instrumentation to handle objc_msgSend with struct returns

[jingham]

Summary

The objc_object_checker instrumentation inserts a call to the checker function before each call to any of the family of objc_msgSend calls. The checker function gets passed the object, and the selector from the msgSend. These arguments are in different places in the original call instruction depending on whether the method used the struct return convention or not. Traditionally, objc_msgSend was used for scalar returns and objc_msgSent_stret for struct return conventions. But on arm64, both scalar and struct return calls use objc_msgSend, so for struct return methods we were passing the checker the wrong object pointer and the expression was crashing in the checker.

However, the llvm::Instruction generated by the JIT knows whether it was used with struct return convention or not, so add a check for that to the code that inserts the checker.

[clayborg]

source/Expression/IRDynamicChecks.cpp

437

move up above for all architectures?

438–439

remove these two lines after moving eMsgSend_stret up with eMsgSend and eMsgSend_fpret cases?

[jingham]

I chose not to do it that way because I don't think it will ever be the case the msgSend_stret will NOT be a struct return convention so the test isn't relevant (and is a bit confusing) there. And we may at some point need to do some other work that depends on the flavor of msgSend, so I wanted to keep the distinction.

If all we are ever going to care about is the return convention, then we should remove the msgSend_types classification and just encode the return type and have the Inspect pass over the Super sends. But that would obscure the fact that this code is all about instrumenting some of the objc_msgSend flavors, and make it harder to read IMO.

[clayborg]

Sounds good.

packages/Python/lldbsuite/test/lang/objc/objc-checker/TestObjCCheckers.py

 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 
 class ObjCCheckerTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
+    NO_DEBUG_INFO_TESTCASE = True
+
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
 
         # Find the line number to break for main.c.
         self.source_name = 'main.m'
 
     @skipUnlessDarwin
def test_objc_checker(self):
 
         # Make sure the call produced no NSLog stdout.
         stdout = process.GetSTDOUT(100)
         self.assertTrue(stdout is None or (len(stdout) == 0))
 
         # Make sure the error is helpful:
         err_string = expr_error.GetCString()
         self.assertTrue("selector" in err_string)
+
+        #
+        # Check that we correctly insert the checker for an
+        # ObjC method with the struct return convention.
+        # Getting this wrong would cause us to call the checker
+        # with the wrong arguments, and the checker would crash
+        # So I'm just checking "expression runs successfully" here:
+        #
+        expr_value = frame.EvaluateExpression("[my_simple getBigStruct]", False)
+        expr_error = expr_value.GetError()
+        
+        self.assertTrue(expr_error.Success())
+        

packages/Python/lldbsuite/test/lang/objc/objc-checker/main.m

 #import <Foundation/Foundation.h>
 
+// This should be a big enough struct that it will force
+// the struct return convention:
+typedef struct BigStruct {
+  float a, b, c, d, e, f, g, h, i, j, k, l;
+} BigStruct;
+
+
 @interface Simple : NSObject
 {
   int _value;
 }
 - (int) value;
 - (void) setValue: (int) newValue;
+- (BigStruct) getBigStruct;
 @end
 
 @implementation Simple
 - (int) value
 {
   return _value;
 }
 
 - (void) setValue: (int) newValue
 {
   _value = newValue;
 }
+
+- (BigStruct) getBigStruct
+{
+  BigStruct big_struct = {1.0, 2.0, 3.0, 4.0, 5.0, 6.0,
+                          7.0, 8.0, 9.0, 10.0, 11.0, 12.0};
+  return big_struct;
+}
 @end
 
 int main ()
 {
   NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
   Simple *my_simple = [[Simple alloc] init];
   my_simple.value = 20;
   // Set a breakpoint here.
   NSLog (@"Object has value: %d.", my_simple.value); 
   [pool drain];
   return 0;
 }

source/Expression/IRDynamicChecks.cpp

bool InstrumentInstruction(llvm::Instruction *inst) override {
     // id objc_msgSend(id theReceiver, SEL theSelector, ...)
 
     llvm::Value *target_object;
     llvm::Value *selector;
 
     switch (msgSend_types[inst]) {
     case eMsgSend:
     case eMsgSend_fpret:
+      // On arm64, clang uses objc_msgSend for scalar and struct return
+      // calls.  The call instruction will record which was used.
+      if (call_inst->hasStructRetAttr()) {
+        target_object = call_inst->getArgOperand(1);
+        selector = call_inst->getArgOperand(2);
+      } else {
         target_object = call_inst->getArgOperand(0);
         selector = call_inst->getArgOperand(1);
+      }
       break;
     case eMsgSend_stret:
       target_object = call_inst->getArgOperand(1);
       selector = call_inst->getArgOperand(2);
       break;
     case eMsgSendSuper:
     case eMsgSendSuper_stret:
       return true;
     }
 
     // These objects should always be valid according to Sean Calannan
     assert(target_object);