From 3e3c6f24ff85ea52ed67d4c26f1d3d0eacd1ad1b Mon Sep 17 00:00:00 2001 From: Nick Desaulniers Date: Tue, 2 May 2023 15:54:09 -0700 Subject: [PATCH] Revert "[Demangle] make llvm::demangle take std::string_view rather than const std::string&" This reverts commit c117c2c8ba4afd45a006043ec6dd858652b2ffcc. itaniumDemangle calls std::strlen with the results of std::string_view::data() which may not be NUL-terminated. This causes lld/test/wasm/why-extract.s to fail when "expensive checks" are enabled via -DLLVM_ENABLE_EXPENSIVE_CHECKS=ON. See D149675 for further discussion. Back this out until the individual demanglers are converted to use std::string_view. --- 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 | 3 +-- .../LogicalView/Readers/LVCodeViewVisitor.cpp | 2 +- llvm/lib/Demangle/Demangle.cpp | 30 ++++++++++++---------- 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/tools/llvm-tli-checker/llvm-tli-checker.cpp | 2 +- 16 files changed, 60 insertions(+), 52 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp index b054147..29adf88 100644 --- a/clang/lib/CodeGen/CodeGenAction.cpp +++ b/clang/lib/CodeGen/CodeGenAction.cpp @@ -633,8 +633,9 @@ 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()); + << D.getStackSize() + << D.getStackLimit() + << llvm::demangle(D.getFunction().getName().str()); return true; } @@ -648,7 +649,7 @@ bool BackendConsumer::ResourceLimitDiagHandler( Diags.Report(*Loc, DiagID) << D.getResourceName() << D.getResourceSize() << D.getResourceLimit() - << llvm::demangle(D.getFunction().getName()); + << llvm::demangle(D.getFunction().getName().str()); return true; } @@ -853,7 +854,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()) << D.getNote(); + << llvm::demangle(D.getFunctionName().str()) << D.getNote(); } void BackendConsumer::MisExpectDiagHandler( diff --git a/lld/COFF/Symbols.cpp b/lld/COFF/Symbols.cpp index f4efcf2..c042386 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); + std::string demangled = demangle(demangleInput.str()); if (demangled != demangleInput) - return prefix + demangled; + return prefix + demangle(demangleInput.str()); return (prefix + prefixless).str(); } return std::string(symName); diff --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp index c5ef6d3..f09d0d7 100644 --- a/lld/ELF/SymbolTable.cpp +++ b/lld/ELF/SymbolTable.cpp @@ -145,16 +145,13 @@ 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); - 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(); - } + 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(); (*demangledSyms)[demangled].push_back(sym); } } diff --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp index 840385a..62a8a3c 100644 --- a/lld/ELF/Symbols.cpp +++ b/lld/ELF/Symbols.cpp @@ -45,7 +45,9 @@ LLVM_ATTRIBUTE_UNUSED static inline void assertSymbols() { // Returns a symbol for an error message. static std::string maybeDemangleSymbol(StringRef symName) { - return elf::config->demangle ? demangle(symName.str()) : symName.str(); + if (elf::config->demangle) + return demangle(symName.str()); + return symName.str(); } std::string lld::toString(const elf::Symbol &sym) { diff --git a/lld/MachO/Symbols.cpp b/lld/MachO/Symbols.cpp index 660826e..cb3b271 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); + return demangle(symName.str()); } return symName.str(); } diff --git a/lld/wasm/Symbols.cpp b/lld/wasm/Symbols.cpp index 2adf72b..567ff49 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); + return demangle(name.str()); return name.str(); } diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst index 100a424..c764a50 100644 --- a/llvm/docs/ReleaseNotes.rst +++ b/llvm/docs/ReleaseNotes.rst @@ -292,11 +292,6 @@ 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 567d721..855ab1f 100644 --- a/llvm/include/llvm/Demangle/Demangle.h +++ b/llvm/include/llvm/Demangle/Demangle.h @@ -11,7 +11,6 @@ #include #include -#include namespace llvm { /// This is a llvm local version of __cxa_demangle. Other than the name and @@ -63,7 +62,7 @@ char *dlangDemangle(const char *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_view MangledName); +std::string demangle(const std::string &MangledName); bool nonMicrosoftDemangle(const char *MangledName, std::string &Result); diff --git a/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp b/llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp index c3ca6df..2139124 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(LinkageName); + std::string DemangledSymbol = demangle(std::string(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 c7cfae7..68eddde 100644 --- a/llvm/lib/Demangle/Demangle.cpp +++ b/llvm/lib/Demangle/Demangle.cpp @@ -11,14 +11,24 @@ //===----------------------------------------------------------------------===// #include "llvm/Demangle/Demangle.h" -#include "llvm/Demangle/StringViewExtras.h" #include +#include -using llvm::itanium_demangle::starts_with; +static bool isItaniumEncoding(const char *S) { + // Itanium encoding requires 1 or 3 leading underscores, followed by 'Z'. + return std::strncmp(S, "_Z", 2) == 0 || std::strncmp(S, "___Z", 4) == 0; +} + +static bool isRustEncoding(const char *S) { return S[0] == '_' && S[1] == 'R'; } + +static bool isDLangEncoding(const std::string &MangledName) { + return MangledName.size() >= 2 && MangledName[0] == '_' && + MangledName[1] == 'D'; +} -std::string llvm::demangle(const std::string_view MangledName) { +std::string llvm::demangle(const std::string &MangledName) { std::string Result; - const char *S = MangledName.data(); + const char *S = MangledName.c_str(); if (nonMicrosoftDemangle(S, Result)) return Result; @@ -29,20 +39,12 @@ std::string llvm::demangle(const std::string_view MangledName) { if (char *Demangled = microsoftDemangle(S, nullptr, nullptr)) { Result = Demangled; std::free(Demangled); - } else { - Result = MangledName; + return Result; } - return Result; -} -static bool isItaniumEncoding(std::string_view S) { - return starts_with(S, "_Z") || starts_with(S, "___Z"); + return MangledName; } -static bool isRustEncoding(std::string_view S) { return starts_with(S, "_R"); } - -static bool isDLangEncoding(std::string_view S) { return starts_with(S, "_D"); } - bool llvm::nonMicrosoftDemangle(const char *MangledName, std::string &Result) { char *Demangled = nullptr; if (isItaniumEncoding(MangledName)) diff --git a/llvm/lib/IR/DiagnosticInfo.cpp b/llvm/lib/IR/DiagnosticInfo.cpp index 342c4cb..a19d9d6 100644 --- a/llvm/lib/IR/DiagnosticInfo.cpp +++ b/llvm/lib/IR/DiagnosticInfo.cpp @@ -441,7 +441,8 @@ void llvm::diagnoseDontCall(const CallInst &CI) { } void DiagnosticInfoDontCall::print(DiagnosticPrinter &DP) const { - DP << "call to " << demangle(getFunctionName()) << " marked \"dontcall-"; + DP << "call to " << demangle(getFunctionName().str()) + << " 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 6e42204..b0dd0dc 100644 --- a/llvm/tools/llvm-objdump/ELFDump.cpp +++ b/llvm/tools/llvm-objdump/ELFDump.cpp @@ -108,7 +108,10 @@ static Error getRelocationValueString(const ELFObjectFile *Obj, Expected SymName = SI->getName(); if (!SymName) return SymName.takeError(); - Fmt << (Demangle ? demangle(*SymName) : *SymName); + if (Demangle) + Fmt << demangle(std::string(*SymName)); + else + Fmt << *SymName; } } else { Fmt << "*ABS*"; diff --git a/llvm/tools/llvm-objdump/XCOFFDump.cpp b/llvm/tools/llvm-objdump/XCOFFDump.cpp index 4849b20..7171e2e 100644 --- a/llvm/tools/llvm-objdump/XCOFFDump.cpp +++ b/llvm/tools/llvm-objdump/XCOFFDump.cpp @@ -32,8 +32,10 @@ Error objdump::getXCOFFRelocationValueString(const XCOFFObjectFile &Obj, if (!SymNameOrErr) return SymNameOrErr.takeError(); - std::string SymName = - Demangle ? demangle(*SymNameOrErr) : SymNameOrErr->str(); + std::string SymName = (*SymNameOrErr).str(); + if (Demangle) + SymName = demangle(SymName); + 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 9d4a2d4..5abcdcc 100644 --- a/llvm/tools/llvm-objdump/llvm-objdump.cpp +++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp @@ -1548,7 +1548,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)); + DemangledSymNamesHere.push_back(demangle(Symbol.Name.str())); // 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(), @@ -1909,8 +1909,9 @@ static void disassembleObject(const Target *TheTarget, ObjectFile &Obj, if (TargetSym != nullptr) { uint64_t TargetAddress = TargetSym->Addr; uint64_t Disp = Target - TargetAddress; - std::string TargetName = Demangle ? demangle(TargetSym->Name) - : TargetSym->Name.str(); + std::string TargetName = TargetSym->Name.str(); + if (Demangle) + TargetName = demangle(TargetName); *TargetOS << " <"; if (!Disp) { @@ -2510,8 +2511,10 @@ void objdump::printSymbol(const ObjectFile &O, const SymbolRef &Symbol, if (NameOrErr) { outs() << " (csect:"; - std::string SymName = - Demangle ? demangle(*NameOrErr) : NameOrErr->str(); + std::string SymName(NameOrErr.get()); + + if (Demangle) + SymName = demangle(SymName); if (SymbolDescription) SymName = getXCOFFSymbolDescription(createSymbolInfo(O, *SymRef), @@ -2565,7 +2568,10 @@ void objdump::printSymbol(const ObjectFile &O, const SymbolRef &Symbol, outs() << " .hidden"; } - std::string SymName = Demangle ? demangle(Name) : Name.str(); + std::string SymName(Name); + if (Demangle) + SymName = demangle(SymName); + 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 7627bed..43d9a0f 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(Name) : Name.str(); + return opts::Demangle ? demangle(std::string(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 9cc18f8..71946f4 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)); + std::string DemangledName(demangle(Name.str())); if (Name != DemangledName) { OutputName += " aka "; OutputName += DemangledName; -- 2.7.4