llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: David Spickett (DavidSpickett) <details> <summary>Changes</summary> I discovered this building the lldb test suite with `-mthumb` set, so that all test programs are purely Arm Thumb code. When C++ expressions did a function lookup, they took a different path from C programs. That path happened to land on the line that I've changed. Where we try to look something up as a function, but don't then resolve the address as if it's a callable. With Thumb, if you do the non-callable lookup, the bottom bit won't be set. This means when lldb's expression wrapper function branches into the found function, it'll be in Arm mode trying to execute Thumb code. Thumb is the only instance where you'd notice this. Aside from maybe MicroMIPS or MIPS16 perhaps but I expect that there are 0 users of that and lldb. I have added a new test case that will simulate this situation in "normal" Arm builds so that it will get run on Linaro's buildbot. This change also fixes the following existing tests when `-mthumb` is used: ``` lldb-api :: commands/expression/anonymous-struct/TestCallUserAnonTypedef.py lldb-api :: commands/expression/argument_passing_restrictions/TestArgumentPassingRestrictions.py lldb-api :: commands/expression/call-function/TestCallStopAndContinue.py lldb-api :: commands/expression/call-function/TestCallUserDefinedFunction.py lldb-api :: commands/expression/char/TestExprsChar.py lldb-api :: commands/expression/class_template_specialization_empty_pack/TestClassTemplateSpecializationParametersHandling.py lldb-api :: commands/expression/context-object/TestContextObject.py lldb-api :: commands/expression/formatters/TestFormatters.py lldb-api :: commands/expression/import_base_class_when_class_has_derived_member/TestImportBaseClassWhenClassHasDerivedMember.py lldb-api :: commands/expression/inline-namespace/TestInlineNamespace.py lldb-api :: commands/expression/namespace-alias/TestInlineNamespaceAlias.py lldb-api :: commands/expression/no-deadlock/TestExprDoesntBlock.py lldb-api :: commands/expression/pr35310/TestExprsBug35310.py lldb-api :: commands/expression/static-initializers/TestStaticInitializers.py lldb-api :: commands/expression/test/TestExprs.py lldb-api :: commands/expression/timeout/TestCallWithTimeout.py lldb-api :: commands/expression/top-level/TestTopLevelExprs.py lldb-api :: commands/expression/unwind_expression/TestUnwindExpression.py lldb-api :: commands/expression/xvalue/TestXValuePrinting.py lldb-api :: functionalities/thread/main_thread_exit/TestMainThreadExit.py lldb-api :: lang/cpp/call-function/TestCallCPPFunction.py lldb-api :: lang/cpp/chained-calls/TestCppChainedCalls.py lldb-api :: lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py lldb-api :: lang/cpp/constructors/TestCppConstructors.py lldb-api :: lang/cpp/function-qualifiers/TestCppFunctionQualifiers.py lldb-api :: lang/cpp/function-ref-qualifiers/TestCppFunctionRefQualifiers.py lldb-api :: lang/cpp/global_operators/TestCppGlobalOperators.py lldb-api :: lang/cpp/llvm-style/TestLLVMStyle.py lldb-api :: lang/cpp/multiple-inheritance/TestCppMultipleInheritance.py lldb-api :: lang/cpp/namespace/TestNamespace.py lldb-api :: lang/cpp/namespace/TestNamespaceLookup.py lldb-api :: lang/cpp/namespace_conflicts/TestNamespaceConflicts.py lldb-api :: lang/cpp/operators/TestCppOperators.py lldb-api :: lang/cpp/overloaded-functions/TestOverloadedFunctions.py lldb-api :: lang/cpp/rvalue-references/TestRvalueReferences.py lldb-api :: lang/cpp/static_methods/TestCPPStaticMethods.py lldb-api :: lang/cpp/template/TestTemplateArgs.py lldb-api :: python_api/thread/TestThreadAPI.py ``` There are other failures that are due to different problems, and this change does not make those worse. (I have no plans to run the test suite with `-mthumb` regularly, I just did it to test some other refactoring) --- Full diff: https://github.com/llvm/llvm-project/pull/151973.diff 4 Files Affected: - (modified) lldb/source/Expression/IRExecutionUnit.cpp (+3-2) - (added) lldb/test/API/arm/thumb-function-addr/Makefile (+3) - (added) lldb/test/API/arm/thumb-function-addr/TestThumbFunctionAddr.py (+67) - (added) lldb/test/API/arm/thumb-function-addr/main.c (+9) ``````````diff diff --git a/lldb/source/Expression/IRExecutionUnit.cpp b/lldb/source/Expression/IRExecutionUnit.cpp index 5e40df282e7b0..e7a26d3c2dcf4 100644 --- a/lldb/source/Expression/IRExecutionUnit.cpp +++ b/lldb/source/Expression/IRExecutionUnit.cpp @@ -737,8 +737,9 @@ class LoadAddressResolver { // If that didn't work, try the function. if (load_address == LLDB_INVALID_ADDRESS && candidate_sc.function) { Address addr = candidate_sc.function->GetAddress(); - load_address = m_target.GetProcessSP() ? addr.GetLoadAddress(&m_target) - : addr.GetFileAddress(); + load_address = m_target.GetProcessSP() + ? addr.GetCallableLoadAddress(&m_target) + : addr.GetFileAddress(); } // We found a load address. diff --git a/lldb/test/API/arm/thumb-function-addr/Makefile b/lldb/test/API/arm/thumb-function-addr/Makefile new file mode 100644 index 0000000000000..10495940055b6 --- /dev/null +++ b/lldb/test/API/arm/thumb-function-addr/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c + +include Makefile.rules diff --git a/lldb/test/API/arm/thumb-function-addr/TestThumbFunctionAddr.py b/lldb/test/API/arm/thumb-function-addr/TestThumbFunctionAddr.py new file mode 100644 index 0000000000000..d08099f6331e5 --- /dev/null +++ b/lldb/test/API/arm/thumb-function-addr/TestThumbFunctionAddr.py @@ -0,0 +1,67 @@ +""" +Test that addresses of functions compiled for Arm Thumb include the Thumb mode +bit (bit 0 of the address) when resolved and used in expressions. +""" + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestThumbFunctionAddr(TestBase): + def do_thumb_function_test(self, language): + self.build(dictionary={"CFLAGS_EXTRAS": f"-x {language} -mthumb"}) + + exe = self.getBuildArtifact("a.out") + line = line_number("main.c", "// Set break point at this line.") + self.runCmd("target create %s" % exe) + bpid = lldbutil.run_break_set_by_file_and_line(self, "main.c", line) + + self.runCmd("run") + self.assertIsNotNone( + lldbutil.get_one_thread_stopped_at_breakpoint_id(self.process(), bpid), + "Process is not stopped at breakpoint", + ) + + # The compiler set this, so the mode bit will be included here. + a_function_addr_var = ( + self.thread().GetFrameAtIndex(0).FindVariable("a_function_addr") + ) + self.assertTrue(a_function_addr_var.IsValid()) + a_function_addr = a_function_addr_var.GetValueAsUnsigned() + self.assertTrue(a_function_addr & 1) + + self.expect("p/x a_function_addr", substrs=[f"0x{a_function_addr:08x}"]) + # If lldb did not pay attention to the mode bit this would SIGILL trying + # to execute Thumb encodings in Arm mode. + self.expect("expression -- a_function()", substrs=["= 123"]) + + # We cannot call GetCallableLoadAdress via. the API, so we expect this + # to not have the bit set as it's treating it as a non-function symbol. + found_function = self.target().FindFunctions("a_function")[0] + self.assertTrue(found_function.IsValid()) + found_function = found_function.GetFunction() + self.assertTrue(found_function.IsValid()) + found_function_addr = found_function.GetStartAddress() + a_function_load_addr = found_function_addr.GetLoadAddress(self.target()) + self.assertEqual(a_function_load_addr, a_function_addr & ~1) + + # image lookup should not include the mode bit. + a_function_file_addr = found_function_addr.GetFileAddress() + self.expect( + "image lookup -n a_function", substrs=[f"0x{a_function_file_addr:08x}"] + ) + + # This test is run for C and C++ because the two will take different paths + # trying to resolve the function's address. + + @skipIf(archs=no_match(["arm$"])) + @skipIf(archs=["arm64"]) + def test_function_addr_c(self): + self.do_thumb_function_test("c") + + @skipIf(archs=no_match(["arm$"])) + @skipIf(archs=["arm64"]) + def test_function_addr_cpp(self): + self.do_thumb_function_test("c++") diff --git a/lldb/test/API/arm/thumb-function-addr/main.c b/lldb/test/API/arm/thumb-function-addr/main.c new file mode 100644 index 0000000000000..f3e01b78f575b --- /dev/null +++ b/lldb/test/API/arm/thumb-function-addr/main.c @@ -0,0 +1,9 @@ +#include <stdint.h> + +int a_function() { return 123; } + +int main() { + const uintptr_t a_function_addr = (uintptr_t)a_function; + // Set break point at this line. + return a_function(); +} `````````` </details> https://github.com/llvm/llvm-project/pull/151973 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
