[lldb] Change synthetic symbol names to have file address #138416
+73
−12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lldb Author: Ely Ronnen (eronnen) Changes
This is a new PR after the first PR (#137512) was reverted because it didn't update the way unnamed symbols were searched in the symbol table, which relied on the index being in the name. This time also added extra test to make sure the symbol is found as expected Full diff: https://github.com/llvm/llvm-project/pull/138416.diff 9 Files Affected:
diff --git a/lldb/include/lldb/Symbol/Symbol.h b/lldb/include/lldb/Symbol/Symbol.h
index e05c845a69f3e..688c8a5931feb 100644
--- a/lldb/include/lldb/Symbol/Symbol.h
+++ b/lldb/include/lldb/Symbol/Symbol.h
@@ -258,7 +258,7 @@ class Symbol : public SymbolContextScope {
bool ContainsFileAddress(lldb::addr_t file_addr) const;
static llvm::StringRef GetSyntheticSymbolPrefix() {
- return "___lldb_unnamed_symbol";
+ return "___lldb_unnamed_symbol_";
}
/// Decode a serialized version of this object from data.
diff --git a/lldb/source/Symbol/Symbol.cpp b/lldb/source/Symbol/Symbol.cpp
index 4828de4fdfa37..d6689a647062a 100644
--- a/lldb/source/Symbol/Symbol.cpp
+++ b/lldb/source/Symbol/Symbol.cpp
@@ -639,7 +639,9 @@ void Symbol::SynthesizeNameIfNeeded() const {
// breakpoints on them.
llvm::SmallString<256> name;
llvm::raw_svector_ostream os(name);
- os << GetSyntheticSymbolPrefix() << GetID();
+ os << GetSyntheticSymbolPrefix()
+ << llvm::format_hex_no_prefix(
+ m_addr_range.GetBaseAddress().GetFileAddress(), 0);
m_mangled.SetDemangledName(ConstString(os.str()));
}
}
diff --git a/lldb/source/Symbol/Symtab.cpp b/lldb/source/Symbol/Symtab.cpp
index 9aee5d3e813d8..52545f22322d2 100644
--- a/lldb/source/Symbol/Symtab.cpp
+++ b/lldb/source/Symbol/Symtab.cpp
@@ -654,7 +654,7 @@ uint32_t Symtab::GetNameIndexes(ConstString symbol_name,
if (count)
return count;
// Synthetic symbol names are not added to the name indexes, but they start
- // with a prefix and end with a the symbol UserID. This allows users to find
+ // with a prefix and end with the symbol file address. This allows users to find
// these symbols without having to add them to the name indexes. These
// queries will not happen very often since the names don't mean anything, so
// performance is not paramount in this case.
@@ -663,11 +663,12 @@ uint32_t Symtab::GetNameIndexes(ConstString symbol_name,
if (!name.consume_front(Symbol::GetSyntheticSymbolPrefix()))
return 0; // Not a synthetic symbol name
- // Extract the user ID from the symbol name
- unsigned long long uid = 0;
- if (getAsUnsignedInteger(name, /*Radix=*/10, uid))
+ // Extract the file address from the symbol name
+ unsigned long long file_address = 0;
+ if (getAsUnsignedInteger(name, /*Radix=*/16, file_address))
return 0; // Failed to extract the user ID as an integer
- Symbol *symbol = FindSymbolByID(uid);
+
+ Symbol *symbol = FindSymbolAtFileAddress(static_cast<addr_t>(file_address));
if (symbol == nullptr)
return 0;
const uint32_t symbol_idx = GetIndexForSymbol(symbol);
diff --git a/lldb/test/API/python_api/unnamed_symbol_lookup/Makefile b/lldb/test/API/python_api/unnamed_symbol_lookup/Makefile
new file mode 100644
index 0000000000000..9ba76898b61ba
--- /dev/null
+++ b/lldb/test/API/python_api/unnamed_symbol_lookup/Makefile
@@ -0,0 +1,12 @@
+C_SOURCES := main.c
+
+include Makefile.rules
+
+all: a.out.stripped
+
+a.out.stripped:
+ $(STRIP) --keep-symbol=main -o a.out.stripped a.out
+
+ifneq "$(CODESIGN)" ""
+ $(CODESIGN) -fs - a.out.stripped
+endif
diff --git a/lldb/test/API/python_api/unnamed_symbol_lookup/TestUnnamedSymbolLookup.py b/lldb/test/API/python_api/unnamed_symbol_lookup/TestUnnamedSymbolLookup.py
new file mode 100644
index 0000000000000..09d43a34c7e30
--- /dev/null
+++ b/lldb/test/API/python_api/unnamed_symbol_lookup/TestUnnamedSymbolLookup.py
@@ -0,0 +1,40 @@
+"""
+Test lookup unnamed symbols.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestUnnamedSymbolLookup(TestBase):
+ def test_unnamed_symbol_lookup(self):
+ """Test looking up unnamed symbol synthetic name"""
+ self.build()
+ (target, process, thread, bkpt) = lldbutil.run_to_name_breakpoint(
+ self, "main", exe_name="a.out.stripped"
+ )
+
+ main_frame = thread.GetFrameAtIndex(0)
+
+ # Step until reaching the unnamed symbol called from main
+ for _ in range(100):
+ thread.StepInto()
+ if thread.GetFrameAtIndex(0) != main_frame:
+ break
+
+ thread.StepInto()
+
+ self.assertEqual(
+ main_frame, thread.GetFrameAtIndex(1), "Expected to be called from main"
+ )
+ symbol = thread.GetFrameAtIndex(0).GetSymbol()
+ self.assertIsNotNone(symbol, "unnamed symbol called from main not reached")
+ self.assertTrue(symbol.name.startswith("___lldb_unnamed_symbol"))
+
+ exe_module = symbol.GetStartAddress().GetModule()
+ found_symbols = exe_module.FindSymbols(symbol.name)
+ self.assertIsNotNone(found_symbols)
+ self.assertEqual(found_symbols.GetSize(), 1)
diff --git a/lldb/test/API/python_api/unnamed_symbol_lookup/main.c b/lldb/test/API/python_api/unnamed_symbol_lookup/main.c
new file mode 100644
index 0000000000000..1c9ce8e0e6697
--- /dev/null
+++ b/lldb/test/API/python_api/unnamed_symbol_lookup/main.c
@@ -0,0 +1,10 @@
+__attribute__((nodebug)) int stripped_function(int val)
+{
+ return val * val;
+}
+
+int main (void)
+{
+ stripped_function(10);
+ return 0;
+}
diff --git a/lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml b/lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
index 0dcc9fb76bd4f..709c37e79d878 100644
--- a/lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
+++ b/lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
@@ -3,8 +3,8 @@
# CHECK: Index UserID DSX Type File Address/Value Load Address Size Flags Name
# CHECK: [ 0] 1 SourceFile 0x0000000000000000 0x0000000000000000 0x00000004 -
-# CHECK: [ 1] 2 SX Code 0x0000000000201180 0x0000000000000010 0x00000000 ___lldb_unnamed_symbol{{[0-9]*}}
-# CHECK: [ 2] 3 SX Code 0x0000000000201190 0x0000000000000006 0x00000000 ___lldb_unnamed_symbol{{[0-9]*}}
+# CHECK: [ 1] 2 SX Code 0x0000000000201180 0x0000000000000010 0x00000000 ___lldb_unnamed_symbol_{{[0-9a-f]*}}
+# CHECK: [ 2] 3 SX Code 0x0000000000201190 0x0000000000000006 0x00000000 ___lldb_unnamed_symbol_{{[0-9a-f]*}}
--- !ELF
FileHeader:
diff --git a/lldb/test/Shell/SymbolFile/Breakpad/symtab-sorted-by-size.test b/lldb/test/Shell/SymbolFile/Breakpad/symtab-sorted-by-size.test
index 98052ea20bedd..00e04eb39a98e 100644
--- a/lldb/test/Shell/SymbolFile/Breakpad/symtab-sorted-by-size.test
+++ b/lldb/test/Shell/SymbolFile/Breakpad/symtab-sorted-by-size.test
@@ -3,7 +3,7 @@
# RUN: -s %s | FileCheck %s
# CHECK: num_symbols = 4 (sorted by size):
-# CHECK: [ 0] 0 SX Code 0x0000000000400000 0x00000000000000b0 0x00000000 ___lldb_unnamed_symbol0
+# CHECK: [ 0] 0 SX Code 0x0000000000400000 0x00000000000000b0 0x00000000 ___lldb_unnamed_symbol_400000
# CHECK: [ 1] 0 X Code 0x00000000004000d0 0x0000000000000022 0x00000000 _start
# CHECK: [ 2] 0 X Code 0x00000000004000b0 0x0000000000000010 0x00000000 f1
# CHECK: [ 3] 0 X Code 0x00000000004000c0 0x0000000000000010 0x00000000 f2
diff --git a/lldb/test/Shell/SymbolFile/Breakpad/symtab.test b/lldb/test/Shell/SymbolFile/Breakpad/symtab.test
index ef41bb3bea955..a32eb5808426f 100644
--- a/lldb/test/Shell/SymbolFile/Breakpad/symtab.test
+++ b/lldb/test/Shell/SymbolFile/Breakpad/symtab.test
@@ -5,7 +5,7 @@
# CHECK-LABEL: (lldb) image dump symtab symtab.out
# CHECK: Symtab, file = {{.*}}symtab.out, num_symbols = 4:
# CHECK: Index UserID DSX Type File Address/Value Load Address Size Flags Name
-# CHECK: [ 0] 0 SX Code 0x0000000000400000 0x00000000000000b0 0x00000000 ___lldb_unnamed_symbol{{[0-9]*}}
+# CHECK: [ 0] 0 SX Code 0x0000000000400000 0x00000000000000b0 0x00000000 ___lldb_unnamed_symbol_{{[0-9a-f]*}}
# CHECK: [ 1] 0 X Code 0x00000000004000b0 0x0000000000000010 0x00000000 f1
# CHECK: [ 2] 0 X Code 0x00000000004000c0 0x0000000000000010 0x00000000 f2
# CHECK: [ 3] 0 X Code 0x00000000004000d0 0x0000000000000022 0x00000000 _start
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
@felipepiovezan Hopefully this should fix the problem in the macOS tests (#137512 (comment)) though I don't know how to trigger these tests from the PR :| |
Unfortunately there's no pre-commit testing for Darwin. I applied your patch locally and ran the test suite and the test is still failing because |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a new PR after the first PR (#137512) was reverted because it didn't update the way unnamed symbols were searched in the symbol table, which relied on the index being in the name.
This time also added extra test to make sure the symbol is found as expected