[Lldb-commits] [lldb] 33f9fc7 - Don't report memory return values on MacOS_arm64 of SysV_arm64 ABI's.
Jim Ingham via lldb-commits
lldb-commits at lists.llvm.org
Mon Mar 14 15:25:46 PDT 2022
Author: Jim Ingham
Date: 2022-03-14T15:25:40-07:00
New Revision: 33f9fc77d1ffe7d6c86b6c7a692ff9f2b99cf82e
URL: https://github.com/llvm/llvm-project/commit/33f9fc77d1ffe7d6c86b6c7a692ff9f2b99cf82e
DIFF: https://github.com/llvm/llvm-project/commit/33f9fc77d1ffe7d6c86b6c7a692ff9f2b99cf82e.diff
LOG: 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
Added:
Modified:
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
Removed:
################################################################################
diff --git a/lldb/bindings/interface/SBTarget.i b/lldb/bindings/interface/SBTarget.i
index b98aa70849be6..a6a764944d02e 100644
--- a/lldb/bindings/interface/SBTarget.i
+++ b/lldb/bindings/interface/SBTarget.i
@@ -394,6 +394,9 @@ public:
const char *
GetTriple ();
+ const char *
+ GetABIName();
+
%feature("docstring", "
Architecture data byte width accessor
diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h
index abd9ebf074076..018acfd595ade 100644
--- a/lldb/include/lldb/API/SBTarget.h
+++ b/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
///
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index faa77a2ac5957..59617ed63ff10 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/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.
///
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 70befcffe9f98..bf64bb342aafe 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/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);
diff --git a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
index cbb4e90cbca62..9dfc50564e64f 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
+++ b/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
diff --git a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
index 25eb72f862cd6..99623ce74eff4 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
+++ b/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
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index a164be7fb4929..7a7ef345682f3 100644
--- a/lldb/source/Target/Target.cpp
+++ b/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;
diff --git a/lldb/test/API/functionalities/return-value/TestReturnValue.py b/lldb/test/API/functionalities/return-value/TestReturnValue.py
index 002f7fb50d10c..f2f50385b8bd7 100644
--- a/lldb/test/API/functionalities/return-value/TestReturnValue.py
+++ b/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())
diff --git a/lldb/test/API/python_api/target/TestTargetAPI.py b/lldb/test/API/python_api/target/TestTargetAPI.py
index 5b80081315561..c02a0a29e0aae 100644
--- a/lldb/test/API/python_api/target/TestTargetAPI.py
+++ b/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)
More information about the lldb-commits
mailing list