[Lldb-commits] [PATCH] D121348: Don't try to get memory returns for ABIMacOSX_arm64

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added reviewers: JDevlieghere, jasonmolenda.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The ARM64 ABI does not require restoring the memory return register value (x8) on exit from a function, so lldb can't reliably fetch memory return values.  We were still trying and that caused a bunch of tests in TestReturnValue to fail.  I changed the ABI code to not try to fetch return values for memory returns.  I also changed the tests to assert that for all the cases of memory returns we return no value on Darwin arm64, since we shouldn't be guessing...


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121348

Files:
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
  lldb/test/API/functionalities/return-value/TestReturnValue.py


Index: lldb/test/API/functionalities/return-value/TestReturnValue.py
===================================================================
--- lldb/test/API/functionalities/return-value/TestReturnValue.py
+++ lldb/test/API/functionalities/return-value/TestReturnValue.py
@@ -22,10 +22,19 @@
         return (self.getArchitecture() in ["aarch64", "arm"] and
                 self.getPlatform() in ["freebsd", "linux"])
 
-    # ABIMacOSX_arm(64) can't fetch simple values inside a structure
-    def affected_by_radar_34562999(self):
+    # ABIMacOSX_arm(64) doesn't restore the storage value for memory returns on function
+    # exit, so lldb shouldn't attempt to fetch memory for those return types, as there is
+    # no easy way to guarantee that they will be correct.  This is calculable, but I don't
+    # want to have to encode the arm64 ABI here, so I just listed the ones that we shouldn't
+    # be reporting a value for.
+    darwin_arm_no_return_values = ["return_five_int", "return_one_int_one_double_one_int",
+                                       "return_one_short_one_double_one_short", "return_vector_size_float32_32",
+                                       "return_ext_vector_size_float32_8"]
+    def should_report_return_value(self, func_name):
         arch = self.getArchitecture().lower()
-        return arch in ['arm64', 'arm64e', 'armv7', 'armv7k'] and self.platformIsDarwin()
+        if arch in ['arm64', 'arm64e', 'armv7', 'armv7k'] and self.platformIsDarwin():
+            return not func_name in self.darwin_arm_no_return_values
+        return True
 
     @expectedFailureAll(oslist=["freebsd"], archs=["i386"],
                         bugnumber="llvm.org/pr48376")
@@ -128,14 +137,13 @@
 
         #self.assertEqual(in_float, return_float)
 
-        if not self.affected_by_radar_34562999() and not self.affected_by_pr44132():
+        if not self.affected_by_pr44132():
             self.return_and_test_struct_value("return_one_int")
             self.return_and_test_struct_value("return_two_int")
             self.return_and_test_struct_value("return_three_int")
             self.return_and_test_struct_value("return_four_int")
             if not self.affected_by_pr33042():
                 self.return_and_test_struct_value("return_five_int")
-
             self.return_and_test_struct_value("return_two_double")
             self.return_and_test_struct_value("return_one_double_two_float")
             self.return_and_test_struct_value("return_one_int_one_float_one_int")
@@ -169,7 +177,6 @@
         archs=["i386"])
     @expectedFailureAll(compiler=["gcc"], archs=["x86_64", "i386"])
     @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24778")
-    @expectedFailureDarwin(archs=["arm64"]) # <rdar://problem/33976032> ABIMacOSX_arm64 doesn't get structs this big correctly
     def test_vector_values(self):
         self.build()
         exe = self.getBuildArtifact("a.out")
@@ -185,7 +192,6 @@
             None, None, self.get_process_working_directory())
         self.assertEqual(len(lldbutil.get_threads_stopped_at_breakpoint(
             self.process, main_bktp)), 1)
-
         self.return_and_test_struct_value("return_vector_size_float32_8")
         self.return_and_test_struct_value("return_vector_size_float32_16")
         if not self.affected_by_pr44132():
@@ -269,6 +275,9 @@
 
         frame = thread.GetFrameAtIndex(0)
         ret_value = thread.GetStopReturnValue()
+        if not self.should_report_return_value(func_name):
+            self.assertFalse(ret_value.IsValid(), "Shouldn't have gotten a value")
+            return
 
         self.assertTrue(ret_value.IsValid())
 
Index: lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
===================================================================
--- lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
+++ lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
@@ -590,9 +590,10 @@
   } else {
     const RegisterInfo *reg_info = nullptr;
     if (is_return_value) {
-      // We are assuming we are decoding this immediately after returning from
-      // a function call and that the address of the structure is in x8
-      reg_info = reg_ctx->GetRegisterInfoByName("x8", 0);
+      // The Darwin arm64 ABI doesn't write the return location back to x8
+      // before returning from the function the way the x86_64 ABI does.  So
+      // we can't reconstruct stack based returns on exit from the function:
+      return false;
     } else {
       // We are assuming we are stopped at the first instruction in a function
       // and that the ABI is being respected so all parameters appear where


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D121348.414248.patch
Type: text/x-patch
Size: 4647 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20220310/c4f76d5a/attachment.bin>