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; +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
