From 8abbc17ff3b1390581600d1b9b37888f4b8ba50e Mon Sep 17 00:00:00 2001 From: Nick Desaulniers Date: Mon, 5 Jun 2023 14:59:46 -0700 Subject: [PATCH] reland: [Demangle] make llvm::demangle take std::string_view rather than const std::string& As suggested by @erichkeane in https://reviews.llvm.org/D141451#inline-1429549 There's potential for a lot more cleanups around these APIs. This is just a start. Callers need to be more careful about sub-expressions producing strings that don't outlast the expression using `llvm::demangle`. Add a release note. Differential Revision: https://reviews.llvm.org/D149104 --- clang/lib/CodeGen/CodeGenAction.cpp | 9 ++++----- lld/COFF/Symbols.cpp | 4 ++-- lld/ELF/SymbolTable.cpp | 15 +++++++++------ lld/ELF/Symbols.cpp | 4 +--- lld/MachO/Symbols.cpp | 2 +- lld/wasm/Symbols.cpp | 2 +- llvm/docs/ReleaseNotes.rst | 5 +++++ llvm/include/llvm/Demangle/Demangle.h | 2 +- .../LogicalView/Readers/LVCodeViewVisitor.cpp | 2 +- llvm/lib/Demangle/Demangle.cpp | 16 ++++++++-------- llvm/lib/IR/DiagnosticInfo.cpp | 3 +-- llvm/tools/llvm-objdump/ELFDump.cpp | 5 +---- llvm/tools/llvm-objdump/XCOFFDump.cpp | 6 ++---- llvm/tools/llvm-objdump/llvm-objdump.cpp | 18 ++++++------------ llvm/tools/llvm-readobj/ELFDumper.cpp | 2 +- .../llvm-tli-checker/llvm-tli-checker.cpp | 2 +- 16 files changed, 45 insertions(+), 52 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp index 784ff77c6172..b93477fdbf0a 100644 --- a/clang/lib/CodeGen/CodeGenAction.cpp +++ b/clang/lib/CodeGen/CodeGenAction.cpp @@ -635,9 +635,8 @@ BackendConsumer::StackSizeDiagHandler(const llvm::DiagnosticInfoStackSize &D) { return false; Diags.Report(*Loc, diag::warn_fe_frame_larger_than) - << D.getStackSize() - << D.getStackLimit() - << llvm::demangle(D.getFunction().getName().str()); + << D.getStackSize() << D.getStackLimit() + << llvm::demangle(D.getFunction().getName()); return true; } @@ -651,7 +650,7 @@ bool BackendConsumer::ResourceLimitDiagHandler( Diags.Report(*Loc, DiagID) << D.getResourceName() << D.getResourceSize() << D.getResourceLimit() - << llvm::demangle(D.getFunction().getName().str()); + << llvm::demangle(D.getFunction().getName()); return true; } @@ -856,7 +855,7 @@ void BackendConsumer::DontCallDiagHandler(const DiagnosticInfoDontCall &D) { Diags.Report(LocCookie, D.getSeverity() == DiagnosticSeverity::DS_Error ? diag::err_fe_backend_error_attr : diag::warn_fe_backend_warning_attr) - << llvm::demangle(D.getFunctionName().str()) << D.getNote(); + << llvm::demangle(D.getFunctionName()) << D.getNote(); } void BackendConsumer::MisExpectDiagHandler( diff --git a/lld/COFF/Symbols.cpp b/lld/COFF/Symbols.cpp index c042386e0106..f4efcf2266cd 100644 --- a/lld/COFF/Symbols.cpp +++ b/lld/COFF/Symbols.cpp @@ -38,9 +38,9 @@ static std::string maybeDemangleSymbol(const COFFLinkerContext &ctx, StringRef demangleInput = prefixless; if (ctx.config.machine == I386) demangleInput.consume_front("_"); - std::string demangled = demangle(demangleInput.str()); + std::string demangled = demangle(demangleInput); if (demangled != demangleInput) - return prefix + demangle(demangleInput.str()); + return prefix + demangled; return (prefix + prefixless).str(); } return std::string(symName); diff --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp index 8ffc8ee7696b..0437095c8638 100644 --- a/lld/ELF/SymbolTable.cpp +++ b/lld/ELF/SymbolTable.cpp @@ -145,13 +145,16 @@ StringMap> &SymbolTable::getDemangledSyms() { if (canBeVersioned(*sym)) { StringRef name = sym->getName(); size_t pos = name.find('@'); + std::string substr; if (pos == std::string::npos) - demangled = demangle(name.str()); - else if (pos + 1 == name.size() || name[pos + 1] == '@') - demangled = demangle(name.substr(0, pos).str()); - else - demangled = - (demangle(name.substr(0, pos).str()) + name.substr(pos)).str(); + demangled = demangle(name); + else if (pos + 1 == name.size() || name[pos + 1] == '@') { + substr = name.substr(0, pos); + demangled = demangle(substr); + } else { + substr = name.substr(0, pos); + demangled = (demangle(substr) + name.substr(pos)).str(); + } (*demangledSyms)[demangled].push_back(sym); } } diff --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp index 62a8a3c30664..840385aabea3 100644 --- a/lld/ELF/Symbols.cpp +++ b/lld/ELF/Symbols.cpp @@ -45,9 +45,7 @@ LLVM_ATTRIBUTE_UNUSED static inline void assertSymbols() { // Returns a symbol for an error message. static std::string maybeDemangleSymbol(StringRef symName) { - if (elf::config->demangle) - return demangle(symName.str()); - return symName.str(); + return elf::config->demangle ? demangle(symName.str()) : symName.str(); } std::string lld::toString(const elf::Symbol &sym) { diff --git a/lld/MachO/Symbols.cpp b/lld/MachO/Symbols.cpp index 7efede390414..4428bd4d9dc9 100644 --- a/lld/MachO/Symbols.cpp +++ b/lld/MachO/Symbols.cpp @@ -32,7 +32,7 @@ static_assert(sizeof(SymbolUnion) == sizeof(Defined), static std::string maybeDemangleSymbol(StringRef symName) { if (config->demangle) { symName.consume_front("_"); - return demangle(symName.str()); + return demangle(symName); } return symName.str(); } diff --git a/lld/wasm/Symbols.cpp b/lld/wasm/Symbols.cpp index 567ff49dfa44..2adf72b6965c 100644 --- a/lld/wasm/Symbols.cpp +++ b/lld/wasm/Symbols.cpp @@ -35,7 +35,7 @@ std::string maybeDemangleSymbol(StringRef name) { if (name == "__main_argc_argv") return "main"; if (wasm::config->demangle) - return demangle(name.str()); + return demangle(name); return name.str(); } diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst index 99f3a5c8ed6d..1dd259095ab7 100644 --- a/llvm/docs/ReleaseNotes.rst +++ b/llvm/docs/ReleaseNotes.rst @@ -344,6 +344,11 @@ Changes to Sanitizers Other Changes ------------- +* ``llvm::demangle`` now takes a ``std::string_view`` rather than a + ``const std::string&``. Be careful passing temporaries into + ``llvm::demangle`` that don't outlive the expression using + ``llvm::demangle``. + External Open Source Projects Using LLVM 15 =========================================== diff --git a/llvm/include/llvm/Demangle/Demangle.h b/llvm/include/llvm/Demangle/Demangle.h index 9140c94ac3cc..e1f73c422db8 100644 --- a/llvm/include/llvm/Demangle/Demangle.h +++ b/llvm/include/llvm/Demangle/Demangle.h @@ -65,7 +65,7 @@ char *dlangDemangle(std::string_view MangledName); /// \param MangledName - reference to string to demangle. /// \returns - the demangled string, or a copy of the input string if no /// demangling occurred. -std::string demangle(const std::string &MangledName); +std::string demangle(std::string_view MangledName); bool nonMicrosoftDemangle(std::string_view MangledName, std::string &Result); diff --git a/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp b/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp index 2139124873ae..c3ca6df91933 100644 --- a/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp +++ b/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp @@ -1605,7 +1605,7 @@ Error LVSymbolVisitor::visitKnownRecord(CVSymbol &Record, ProcSym &Proc) { // We don't have a way to see if the symbol is compiler generated. Use // the linkage name, to detect `scalar deleting destructor' functions. - std::string DemangledSymbol = demangle(std::string(LinkageName)); + std::string DemangledSymbol = demangle(LinkageName); if (DemangledSymbol.find("scalar deleting dtor") != std::string::npos) { Function->setIsArtificial(); } else { diff --git a/llvm/lib/Demangle/Demangle.cpp b/llvm/lib/Demangle/Demangle.cpp index f575cd61e4bf..f2aa571d685f 100644 --- a/llvm/lib/Demangle/Demangle.cpp +++ b/llvm/lib/Demangle/Demangle.cpp @@ -13,27 +13,27 @@ #include "llvm/Demangle/Demangle.h" #include "llvm/Demangle/StringViewExtras.h" #include -#include +#include using llvm::itanium_demangle::starts_with; -std::string llvm::demangle(const std::string &MangledName) { +std::string llvm::demangle(std::string_view MangledName) { std::string Result; - const char *S = MangledName.c_str(); if (nonMicrosoftDemangle(MangledName, Result)) return Result; - if (S[0] == '_' && nonMicrosoftDemangle(MangledName.substr(1), Result)) + if (starts_with(MangledName, '_') && + nonMicrosoftDemangle(MangledName.substr(1), Result)) return Result; - if (char *Demangled = microsoftDemangle(S, nullptr, nullptr)) { + if (char *Demangled = microsoftDemangle(MangledName, nullptr, nullptr)) { Result = Demangled; std::free(Demangled); - return Result; + } else { + Result = MangledName; } - - return MangledName; + return Result; } static bool isItaniumEncoding(std::string_view S) { diff --git a/llvm/lib/IR/DiagnosticInfo.cpp b/llvm/lib/IR/DiagnosticInfo.cpp index a19d9d607f4b..342c4cbbc39d 100644 --- a/llvm/lib/IR/DiagnosticInfo.cpp +++ b/llvm/lib/IR/DiagnosticInfo.cpp @@ -441,8 +441,7 @@ void llvm::diagnoseDontCall(const CallInst &CI) { } void DiagnosticInfoDontCall::print(DiagnosticPrinter &DP) const { - DP << "call to " << demangle(getFunctionName().str()) - << " marked \"dontcall-"; + DP << "call to " << demangle(getFunctionName()) << " marked \"dontcall-"; if (getSeverity() == DiagnosticSeverity::DS_Error) DP << "error\""; else diff --git a/llvm/tools/llvm-objdump/ELFDump.cpp b/llvm/tools/llvm-objdump/ELFDump.cpp index b0dd0dccfe8f..6e422042c982 100644 --- a/llvm/tools/llvm-objdump/ELFDump.cpp +++ b/llvm/tools/llvm-objdump/ELFDump.cpp @@ -108,10 +108,7 @@ static Error getRelocationValueString(const ELFObjectFile *Obj, Expected SymName = SI->getName(); if (!SymName) return SymName.takeError(); - if (Demangle) - Fmt << demangle(std::string(*SymName)); - else - Fmt << *SymName; + Fmt << (Demangle ? demangle(*SymName) : *SymName); } } else { Fmt << "*ABS*"; diff --git a/llvm/tools/llvm-objdump/XCOFFDump.cpp b/llvm/tools/llvm-objdump/XCOFFDump.cpp index 7171e2eb6eb3..4849b20f1166 100644 --- a/llvm/tools/llvm-objdump/XCOFFDump.cpp +++ b/llvm/tools/llvm-objdump/XCOFFDump.cpp @@ -32,10 +32,8 @@ Error objdump::getXCOFFRelocationValueString(const XCOFFObjectFile &Obj, if (!SymNameOrErr) return SymNameOrErr.takeError(); - std::string SymName = (*SymNameOrErr).str(); - if (Demangle) - SymName = demangle(SymName); - + std::string SymName = + Demangle ? demangle(*SymNameOrErr) : SymNameOrErr->str(); if (SymbolDescription) SymName = getXCOFFSymbolDescription(createSymbolInfo(Obj, *SymI), SymName); diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp index 4267ce660355..7b6edb3d05b6 100644 --- a/llvm/tools/llvm-objdump/llvm-objdump.cpp +++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp @@ -1545,7 +1545,7 @@ static void disassembleObject(const Target *TheTarget, ObjectFile &Obj, if (Demangle) { // Fetch the demangled names and store them locally. for (const SymbolInfoTy &Symbol : SymbolsHere) - DemangledSymNamesHere.push_back(demangle(Symbol.Name.str())); + DemangledSymNamesHere.push_back(demangle(Symbol.Name)); // Now we've finished modifying that vector, it's safe to make // a vector of StringRefs pointing into it. SymNamesHere.insert(SymNamesHere.begin(), DemangledSymNamesHere.begin(), @@ -1906,9 +1906,8 @@ static void disassembleObject(const Target *TheTarget, ObjectFile &Obj, if (TargetSym != nullptr) { uint64_t TargetAddress = TargetSym->Addr; uint64_t Disp = Target - TargetAddress; - std::string TargetName = TargetSym->Name.str(); - if (Demangle) - TargetName = demangle(TargetName); + std::string TargetName = Demangle ? demangle(TargetSym->Name) + : TargetSym->Name.str(); *TargetOS << " <"; if (!Disp) { @@ -2508,10 +2507,8 @@ void objdump::printSymbol(const ObjectFile &O, const SymbolRef &Symbol, if (NameOrErr) { outs() << " (csect:"; - std::string SymName(NameOrErr.get()); - - if (Demangle) - SymName = demangle(SymName); + std::string SymName = + Demangle ? demangle(*NameOrErr) : NameOrErr->str(); if (SymbolDescription) SymName = getXCOFFSymbolDescription(createSymbolInfo(O, *SymRef), @@ -2565,10 +2562,7 @@ void objdump::printSymbol(const ObjectFile &O, const SymbolRef &Symbol, outs() << " .hidden"; } - std::string SymName(Name); - if (Demangle) - SymName = demangle(SymName); - + std::string SymName = Demangle ? demangle(Name) : Name.str(); if (O.isXCOFF() && SymbolDescription) SymName = getXCOFFSymbolDescription(createSymbolInfo(O, Symbol), SymName); diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp index 40b33a425354..a5092a84be11 100644 --- a/llvm/tools/llvm-readobj/ELFDumper.cpp +++ b/llvm/tools/llvm-readobj/ELFDumper.cpp @@ -908,7 +908,7 @@ ELFDumper::getShndxTable(const Elf_Shdr *Symtab) const { } static std::string maybeDemangle(StringRef Name) { - return opts::Demangle ? demangle(std::string(Name)) : Name.str(); + return opts::Demangle ? demangle(Name) : Name.str(); } template diff --git a/llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp b/llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp index 71946f438475..9cc18f80910d 100644 --- a/llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp +++ b/llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp @@ -107,7 +107,7 @@ static std::string getPrintableName(StringRef Name) { std::string OutputName = "'"; OutputName += Name; OutputName += "'"; - std::string DemangledName(demangle(Name.str())); + std::string DemangledName(demangle(Name)); if (Name != DemangledName) { OutputName += " aka "; OutputName += DemangledName;