jingham updated this revision to Diff 414765.
jingham added a comment.

At Pavel's request I extended this to the SysV_arm64 ABI.  Since I don't want 
to have to guess which arch's are which ABI's, I exposed the ABI plugin's name 
and then used that in the test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121348/new/

https://reviews.llvm.org/D121348

Files:
  lldb/bindings/interface/SBTarget.i
  lldb/include/lldb/API/SBTarget.h
  lldb/include/lldb/Target/Target.h
  lldb/source/API/SBTarget.cpp
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/functionalities/return-value/TestReturnValue.py
  lldb/test/API/python_api/target/TestTargetAPI.py

Index: lldb/test/API/python_api/target/TestTargetAPI.py
===================================================================
--- lldb/test/API/python_api/target/TestTargetAPI.py
+++ lldb/test/API/python_api/target/TestTargetAPI.py
@@ -112,6 +112,29 @@
         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)
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,18 @@
         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 @@
 
         #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 @@
         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 @@
             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 @@
 
         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/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -287,6 +287,17 @@
   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;
Index: lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
===================================================================
--- lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
+++ lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
@@ -561,9 +561,12 @@
   } 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
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
Index: lldb/source/API/SBTarget.cpp
===================================================================
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -1588,6 +1588,18 @@
   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);
 
Index: lldb/include/lldb/Target/Target.h
===================================================================
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -969,6 +969,9 @@
   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.
   ///
Index: lldb/include/lldb/API/SBTarget.h
===================================================================
--- lldb/include/lldb/API/SBTarget.h
+++ lldb/include/lldb/API/SBTarget.h
@@ -321,6 +321,8 @@
   uint32_t GetAddressByteSize();
 
   const char *GetTriple();
+  
+  const char *GetABIName();
 
   /// Architecture data byte width accessor
   ///
Index: lldb/bindings/interface/SBTarget.i
===================================================================
--- lldb/bindings/interface/SBTarget.i
+++ lldb/bindings/interface/SBTarget.i
@@ -394,6 +394,9 @@
     const char *
     GetTriple ();
 
+    const char *
+    GetABIName();
+
     %feature("docstring", "
     Architecture data byte width accessor
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to