Skip to content

Commit 4077e66

Browse files
[lldb] Treat address found via function name as a callable address (#151973)
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)
1 parent 907b7d0 commit 4077e66

File tree

4 files changed

+82
-2
lines changed

4 files changed

+82
-2
lines changed

lldb/source/Expression/IRExecutionUnit.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -737,8 +737,9 @@ class LoadAddressResolver {
737737
// If that didn't work, try the function.
738738
if (load_address == LLDB_INVALID_ADDRESS && candidate_sc.function) {
739739
Address addr = candidate_sc.function->GetAddress();
740-
load_address = m_target.GetProcessSP() ? addr.GetLoadAddress(&m_target)
741-
: addr.GetFileAddress();
740+
load_address = m_target.GetProcessSP()
741+
? addr.GetCallableLoadAddress(&m_target)
742+
: addr.GetFileAddress();
742743
}
743744

744745
// We found a load address.
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
C_SOURCES := main.c
2+
3+
include Makefile.rules
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
"""
2+
Test that addresses of functions compiled for Arm Thumb include the Thumb mode
3+
bit (bit 0 of the address) when resolved and used in expressions.
4+
"""
5+
6+
import lldb
7+
from lldbsuite.test.decorators import *
8+
from lldbsuite.test.lldbtest import *
9+
from lldbsuite.test import lldbutil
10+
11+
12+
class TestThumbFunctionAddr(TestBase):
13+
def do_thumb_function_test(self, language):
14+
self.build(dictionary={"CFLAGS_EXTRAS": f"-x {language} -mthumb"})
15+
16+
exe = self.getBuildArtifact("a.out")
17+
line = line_number("main.c", "// Set break point at this line.")
18+
self.runCmd("target create %s" % exe)
19+
bpid = lldbutil.run_break_set_by_file_and_line(self, "main.c", line)
20+
21+
self.runCmd("run")
22+
self.assertIsNotNone(
23+
lldbutil.get_one_thread_stopped_at_breakpoint_id(self.process(), bpid),
24+
"Process is not stopped at breakpoint",
25+
)
26+
27+
# The compiler set this, so the mode bit will be included here.
28+
a_function_addr_var = (
29+
self.thread().GetFrameAtIndex(0).FindVariable("a_function_addr")
30+
)
31+
self.assertTrue(a_function_addr_var.IsValid())
32+
a_function_addr = a_function_addr_var.GetValueAsUnsigned()
33+
self.assertTrue(a_function_addr & 1)
34+
35+
self.expect("p/x a_function_addr", substrs=[f"0x{a_function_addr:08x}"])
36+
# If lldb did not pay attention to the mode bit this would SIGILL trying
37+
# to execute Thumb encodings in Arm mode.
38+
self.expect("expression -- a_function()", substrs=["= 123"])
39+
40+
# We cannot call GetCallableLoadAdress via. the API, so we expect this
41+
# to not have the bit set as it's treating it as a non-function symbol.
42+
found_function = self.target().FindFunctions("a_function")[0]
43+
self.assertTrue(found_function.IsValid())
44+
found_function = found_function.GetFunction()
45+
self.assertTrue(found_function.IsValid())
46+
found_function_addr = found_function.GetStartAddress()
47+
a_function_load_addr = found_function_addr.GetLoadAddress(self.target())
48+
self.assertEqual(a_function_load_addr, a_function_addr & ~1)
49+
50+
# image lookup should not include the mode bit.
51+
a_function_file_addr = found_function_addr.GetFileAddress()
52+
self.expect(
53+
"image lookup -n a_function", substrs=[f"0x{a_function_file_addr:08x}"]
54+
)
55+
56+
# This test is run for C and C++ because the two will take different paths
57+
# trying to resolve the function's address.
58+
59+
@skipIf(archs=no_match(["arm$"]))
60+
@skipIf(archs=["arm64"])
61+
def test_function_addr_c(self):
62+
self.do_thumb_function_test("c")
63+
64+
@skipIf(archs=no_match(["arm$"]))
65+
@skipIf(archs=["arm64"])
66+
def test_function_addr_cpp(self):
67+
self.do_thumb_function_test("c++")
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#include <stdint.h>
2+
3+
int a_function() { return 123; }
4+
5+
int main() {
6+
const uintptr_t a_function_addr = (uintptr_t)a_function;
7+
// Set break point at this line.
8+
return a_function();
9+
}

0 commit comments

Comments
 (0)