Don't report memory return values on MacOS_arm64 of SysV_arm64 ABI's.

They don't require that the memory return address be restored prior to
function exit, so there's no guarantee the value is correct.  It's better
to return nothing that something that's not accurate.

Differential Revision: https://reviews.llvm.org/D121348

lldb/bindings/interface/SBTarget.i

@@ -394,6 +394,9 @@ public:
     const char *
     GetTriple ();
 
+    const char *
+    GetABIName();
+
     %feature("docstring", "
     Architecture data byte width accessor
 

lldb/include/lldb/API/SBTarget.h

@@ -321,6 +321,8 @@ class LLDB_API SBTarget {
   uint32_t GetAddressByteSize();
 
   const char *GetTriple();
+
+  const char *GetABIName();
 
   /// Architecture data byte width accessor
   ///

lldb/include/lldb/Target/Target.h

@@ -969,6 +969,9 @@ class Target : public std::enable_shared_from_this<Target>,
   ModuleIsExcludedForUnconstrainedSearches(const lldb::ModuleSP &module_sp);
 
   const ArchSpec &GetArchitecture() const { return m_arch.GetSpec(); }
+
+  /// Returns the name of the target's ABI plugin.
+  llvm::StringRef GetABIName() const;
 
   /// Set the architecture for this target.
   ///

lldb/source/API/SBTarget.cpp

@@ -1588,6 +1588,18 @@ const char *SBTarget::GetTriple() {
   return nullptr;
 }
 
+const char *SBTarget::GetABIName() {
+  LLDB_INSTRUMENT_VA(this);
+
+  TargetSP target_sp(GetSP());
+  if (target_sp) {
+    std::string abi_name(target_sp->GetABIName().str());
+    ConstString const_name(abi_name.c_str());
+    return const_name.GetCString();
+  }
+  return nullptr;
+}
+
 uint32_t SBTarget::GetDataByteSize() {
   LLDB_INSTRUMENT_VA(this);
 

lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp

@@ -590,9 +590,10 @@ static bool LoadValueFromConsecutiveGPRRegisters(
   } 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

lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp

@@ -561,9 +561,12 @@ static bool LoadValueFromConsecutiveGPRRegisters(
   } 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 SysV arm64 ABI doesn't require you to write the return location 
+      // back to x8 before returning from the function the way the x86_64 ABI 
+      // does.  It looks like all the users of this ABI currently choose not to
+      // do that, and 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

lldb/source/Target/Target.cpp

@@ -287,6 +287,17 @@ void Target::Destroy() {
   m_suppress_stop_hooks = false;
 }
 
+llvm::StringRef Target::GetABIName() const {
+  lldb::ABISP abi_sp;
+  if (m_process_sp)
+    abi_sp = m_process_sp->GetABI();
+  if (!abi_sp)
+    abi_sp = ABI::FindPlugin(ProcessSP(), GetArchitecture());
+  if (abi_sp)
+      return abi_sp->GetPluginName();
+  return {};
+}
+
 BreakpointList &Target::GetBreakpointList(bool internal) {
   if (internal)
     return m_internal_breakpoint_list;

lldb/test/API/functionalities/return-value/TestReturnValue.py

@@ -22,10 +22,18 @@ def affected_by_pr44132(self):
         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):
-        arch = self.getArchitecture().lower()
-        return arch in ['arm64', 'arm64e', 'armv7', 'armv7k'] and self.platformIsDarwin()
+    # ABIMacOSX_arm64 and the SysV_arm64 don'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 a list of the memory
+    # return functions defined in the test file:
+    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):
+        abi = self.target.GetABIName()
+        if not abi in ["SysV-arm64", "ABIMacOSX_arm64", "macosx-arm"]:
+            return True
+        return not func_name in self.arm_no_return_values
 
     @expectedFailureAll(oslist=["freebsd"], archs=["i386"],
                         bugnumber="llvm.org/pr48376")
@@ -128,14 +136,13 @@ def test_with_python(self):
 
         #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 +176,6 @@ def test_with_python(self):
         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 +191,6 @@ def test_vector_values(self):
             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 +274,9 @@ def return_and_test_struct_value(self, func_name):
 
         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())
 

lldb/test/API/python_api/target/TestTargetAPI.py

@@ -112,6 +112,29 @@ def test_resolve_file_address(self):
         self.assertIsNotNone(data_section2)
         self.assertEqual(data_section.name, data_section2.name)
 
+    def test_get_ABIName(self):
+        d = {'EXE': 'b.out'}
+        self.build(dictionary=d)
+        self.setTearDownCleanup(dictionary=d)
+        target = self.create_simple_target('b.out')
+
+        abi_pre_launch = target.GetABIName()
+        self.assertTrue(len(abi_pre_launch) != 0, "Got an ABI string")
+
+        breakpoint = target.BreakpointCreateByLocation(
+            "main.c", self.line_main)
+        self.assertTrue(breakpoint, VALID_BREAKPOINT)
+
+        # Put debugger into synchronous mode so when we target.LaunchSimple returns
+        # it will guaranteed to be at the breakpoint
+        self.dbg.SetAsync(False)
+
+        # Launch the process, and do not stop at the entry point.
+        process = target.LaunchSimple(
+            None, None, self.get_process_working_directory())
+        abi_after_launch = target.GetABIName()
+        self.assertEqual(abi_pre_launch, abi_after_launch, "ABI's match before and during run")
+
     def test_read_memory(self):
         d = {'EXE': 'b.out'}
         self.build(dictionary=d)