[Lldb-commits] [lldb] c1d55d2 - [lldb] Let Mangled decide whether a name is mangled or not
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Fri Apr 21 10:23:35 PDT 2023
Author: Jonas Devlieghere
Date: 2023-04-21T10:23:24-07:00
New Revision: c1d55d26d373e8804aba92f7d0f07de353ea93c9
URL: https://github.com/llvm/llvm-project/commit/c1d55d26d373e8804aba92f7d0f07de353ea93c9
DIFF: https://github.com/llvm/llvm-project/commit/c1d55d26d373e8804aba92f7d0f07de353ea93c9.diff
LOG: [lldb] Let Mangled decide whether a name is mangled or not
We have a handful of places in LLDB where we try to outsmart the logic
in Mangled to determine whether a string is mangled or not. There's at
least one place (*) where we are getting this wrong and causes a subtle
bug. The `cstring_is_mangled` is cheap enough that we should always rely
on it to determine whether a string is mangled or not.
(*) `ObjectFileMachO` assumes that a symbol that starts with a double
underscore (such as `__pthread_kill`) is mangled. That's mostly
harmless, until you use `function.name-without-args` in the frame
format. The formatter calls `Symbol::GetNameNoArguments()` which is a
wrapper around `Mangled::GetName(ePreferDemangledWithoutArguments)`. The
latter will first try using the appropriate language plugin to get the
demangled name without arguments, and if that fails, falls back to
returning the demangled name. Because we forced Mangled to treat the
symbol as a mangled name (even though it's not) there's no demangled
name. The result is that frames don't show any symbol at all.
Differential revision: https://reviews.llvm.org/D148846
Added:
lldb/test/API/macosx/format/Makefile
lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py
lldb/test/API/macosx/format/main.c
Modified:
lldb/include/lldb/Core/Mangled.h
lldb/source/Core/Mangled.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Core/Mangled.h b/lldb/include/lldb/Core/Mangled.h
index dcaa7a8cda6cc..b7813313d4597 100644
--- a/lldb/include/lldb/Core/Mangled.h
+++ b/lldb/include/lldb/Core/Mangled.h
@@ -185,19 +185,6 @@ class Mangled {
/// The number of bytes that this object occupies in memory.
size_t MemorySize() const;
- /// Set the string value in this object.
- ///
- /// If \a is_mangled is \b true, then the mangled named is set to \a name,
- /// else the demangled name is set to \a name.
- ///
- /// \param[in] name
- /// The already const version of the name for this object.
- ///
- /// \param[in] is_mangled
- /// If \b true then \a name is a mangled name, if \b false then
- /// \a name is demangled.
- void SetValue(ConstString name, bool is_mangled);
-
/// Set the string value in this object.
///
/// This version auto detects if the string is mangled by inspecting the
diff --git a/lldb/source/Core/Mangled.cpp b/lldb/source/Core/Mangled.cpp
index 0c4d9f78c4402..29fb35980036a 100644
--- a/lldb/source/Core/Mangled.cpp
+++ b/lldb/source/Core/Mangled.cpp
@@ -90,23 +90,6 @@ int Mangled::Compare(const Mangled &a, const Mangled &b) {
b.GetName(ePreferMangled));
}
-// Set the string value in this objects. If "mangled" is true, then the mangled
-// named is set with the new value in "s", else the demangled name is set.
-void Mangled::SetValue(ConstString s, bool mangled) {
- if (s) {
- if (mangled) {
- m_demangled.Clear();
- m_mangled = s;
- } else {
- m_demangled = s;
- m_mangled.Clear();
- }
- } else {
- m_demangled.Clear();
- m_mangled.Clear();
- }
-}
-
void Mangled::SetValue(ConstString name) {
if (name) {
if (cstring_is_mangled(name.GetStringRef())) {
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 1f89db0cb5ca0..621df79e52e56 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -3030,7 +3030,7 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
// the first contains a directory and the
// second contains a full path.
sym[sym_idx - 1].GetMangled().SetValue(
- ConstString(symbol_name), false);
+ ConstString(symbol_name));
m_nlist_idx_to_sym_idx[nlist_idx] = sym_idx - 1;
add_nlist = false;
} else {
@@ -3076,7 +3076,7 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
full_so_path += '/';
full_so_path += symbol_name;
sym[sym_idx - 1].GetMangled().SetValue(
- ConstString(full_so_path.c_str()), false);
+ ConstString(full_so_path.c_str()));
add_nlist = false;
m_nlist_idx_to_sym_idx[nlist_idx] = sym_idx - 1;
}
@@ -3468,17 +3468,13 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
sym[sym_idx].GetMangled().SetDemangledName(
ConstString(symbol_name));
} else {
- bool symbol_name_is_mangled = false;
-
if (symbol_name && symbol_name[0] == '_') {
- symbol_name_is_mangled = symbol_name[1] == '_';
symbol_name++; // Skip the leading underscore
}
if (symbol_name) {
ConstString const_symbol_name(symbol_name);
- sym[sym_idx].GetMangled().SetValue(
- const_symbol_name, symbol_name_is_mangled);
+ sym[sym_idx].GetMangled().SetValue(const_symbol_name);
if (is_gsym && is_debug) {
const char *gsym_name =
sym[sym_idx]
@@ -3938,8 +3934,8 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
if ((N_SO_index == sym_idx - 1) && ((sym_idx - 1) < num_syms)) {
// We have two consecutive N_SO entries where the first
// contains a directory and the second contains a full path.
- sym[sym_idx - 1].GetMangled().SetValue(ConstString(symbol_name),
- false);
+ sym[sym_idx - 1].GetMangled().SetValue(
+ ConstString(symbol_name));
m_nlist_idx_to_sym_idx[nlist_idx] = sym_idx - 1;
add_nlist = false;
} else {
@@ -3977,7 +3973,7 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
full_so_path += '/';
full_so_path += symbol_name;
sym[sym_idx - 1].GetMangled().SetValue(
- ConstString(full_so_path.c_str()), false);
+ ConstString(full_so_path.c_str()));
add_nlist = false;
m_nlist_idx_to_sym_idx[nlist_idx] = sym_idx - 1;
}
@@ -4330,17 +4326,14 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
ConstString(symbol_name_non_abi_mangled));
sym[sym_idx].GetMangled().SetDemangledName(ConstString(symbol_name));
} else {
- bool symbol_name_is_mangled = false;
if (symbol_name && symbol_name[0] == '_') {
- symbol_name_is_mangled = symbol_name[1] == '_';
symbol_name++; // Skip the leading underscore
}
if (symbol_name) {
ConstString const_symbol_name(symbol_name);
- sym[sym_idx].GetMangled().SetValue(const_symbol_name,
- symbol_name_is_mangled);
+ sym[sym_idx].GetMangled().SetValue(const_symbol_name);
}
}
diff --git a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
index 6d916df65b9df..cd52233cc8cc7 100644
--- a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -246,7 +246,7 @@ FunctionSP SymbolFileBreakpad::GetOrCreateFunction(CompileUnit &comp_unit) {
if (auto record = FuncRecord::parse(*It)) {
Mangled func_name;
- func_name.SetValue(ConstString(record->Name), false);
+ func_name.SetValue(ConstString(record->Name));
addr_t address = record->Address + base;
SectionSP section_sp = list->FindSectionContainingFileAddress(address);
if (section_sp) {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index e6921ca9cacdb..53068823ceac9 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2417,7 +2417,7 @@ DWARFASTParserClang::ParseFunctionFromDWARF(CompileUnit &comp_unit,
&frame_base)) {
Mangled func_name;
if (mangled)
- func_name.SetValue(ConstString(mangled), true);
+ func_name.SetValue(ConstString(mangled));
else if ((die.GetParent().Tag() == DW_TAG_compile_unit ||
die.GetParent().Tag() == DW_TAG_partial_unit) &&
Language::LanguageIsCPlusPlus(
@@ -2428,9 +2428,9 @@ DWARFASTParserClang::ParseFunctionFromDWARF(CompileUnit &comp_unit,
// If the mangled name is not present in the DWARF, generate the
// demangled name using the decl context. We skip if the function is
// "main" as its name is never mangled.
- func_name.SetValue(ConstructDemangledNameFromDWARF(die), false);
+ func_name.SetValue(ConstructDemangledNameFromDWARF(die));
} else
- func_name.SetValue(ConstString(name), false);
+ func_name.SetValue(ConstString(name));
FunctionSP func_sp;
std::unique_ptr<Declaration> decl_up;
diff --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
index 7f07eec1e4640..241272ae84300 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -1980,7 +1980,7 @@ SymbolFilePDB::GetMangledForPDBFunc(const llvm::pdb::PDBSymbolFunc &pdb_func) {
} else if (!func_undecorated_name.empty()) {
mangled.SetDemangledName(ConstString(func_undecorated_name));
} else if (!func_name.empty())
- mangled.SetValue(ConstString(func_name), false);
+ mangled.SetValue(ConstString(func_name));
return mangled;
}
diff --git a/lldb/test/API/macosx/format/Makefile b/lldb/test/API/macosx/format/Makefile
new file mode 100644
index 0000000000000..10495940055b6
--- /dev/null
+++ b/lldb/test/API/macosx/format/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git a/lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py b/lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py
new file mode 100644
index 0000000000000..88d2e210bccf7
--- /dev/null
+++ b/lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py
@@ -0,0 +1,28 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class TestFunctionNameWithoutArgs(TestBase):
+ @skipUnlessDarwin
+ @no_debug_info_test
+ def test_function_name_without_args(self):
+ self.build()
+ target = self.createTestTarget()
+ target.LaunchSimple(None, None, self.get_process_working_directory())
+
+ self.runCmd("run", RUN_SUCCEEDED)
+ self.expect(
+ "bt",
+ substrs=[
+ "stop reason = hit program assert",
+ "libsystem_kernel.dylib`__pthread_kill",
+ ],
+ )
+ self.runCmd(
+ 'settings set frame-format "frame #${frame.index}: ${function.name-without-args}\n"'
+ )
+ self.expect(
+ "bt",
+ substrs=["stop reason = hit program assert", "frame #0: __pthread_kill"],
+ )
diff --git a/lldb/test/API/macosx/format/main.c b/lldb/test/API/macosx/format/main.c
new file mode 100644
index 0000000000000..e35c18be23558
--- /dev/null
+++ b/lldb/test/API/macosx/format/main.c
@@ -0,0 +1,6 @@
+#include <assert.h>
+
+int main(int argc, char **argv) {
+ assert(argc != 1);
+ return 0;
+}
More information about the lldb-commits
mailing list