[Lldb-commits] [PATCH] D58699: Adapt the ObjC checker instrumentation to handle objc_msgSend with struct returns

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added a reviewer: clayborg.
Herald added subscribers: lldb-commits, teemperor, abidh, kristof.beyls, javed.absar.
Herald added a reviewer: serge-sans-paille.
Herald added a project: LLDB.

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.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D58699

Files:
  packages/Python/lldbsuite/test/lang/objc/objc-checker/TestObjCCheckers.py
  packages/Python/lldbsuite/test/lang/objc/objc-checker/main.m
  source/Expression/IRDynamicChecks.cpp


Index: source/Expression/IRDynamicChecks.cpp
===================================================================
--- source/Expression/IRDynamicChecks.cpp
+++ source/Expression/IRDynamicChecks.cpp
@@ -424,8 +424,15 @@
     switch (msgSend_types[inst]) {
     case eMsgSend:
     case eMsgSend_fpret:
-      target_object = call_inst->getArgOperand(0);
-      selector = call_inst->getArgOperand(1);
+      // 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);
Index: packages/Python/lldbsuite/test/lang/objc/objc-checker/main.m
===================================================================
--- packages/Python/lldbsuite/test/lang/objc/objc-checker/main.m
+++ packages/Python/lldbsuite/test/lang/objc/objc-checker/main.m
@@ -1,11 +1,19 @@
 #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
@@ -18,6 +26,13 @@
 {
   _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 ()
Index: packages/Python/lldbsuite/test/lang/objc/objc-checker/TestObjCCheckers.py
===================================================================
--- packages/Python/lldbsuite/test/lang/objc/objc-checker/TestObjCCheckers.py
+++ packages/Python/lldbsuite/test/lang/objc/objc-checker/TestObjCCheckers.py
@@ -18,13 +18,15 @@
 
     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
     @add_test_categories(['pyapi'])
     def test_objc_checker(self):
@@ -77,3 +79,16 @@
         # 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())
+        


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D58699.188464.patch
Type: text/x-patch
Size: 3203 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20190226/e700946c/attachment.bin>