[clang] 3e3c6f2 - Revert "[Demangle] make llvm::demangle take std::string_view rather than const std::string&"

Nick Desaulniers via cfe-commits cfe-commits at lists.llvm.org
Tue May 2 15:55:36 PDT 2023


Author: Nick Desaulniers
Date: 2023-05-02T15:54:09-07:00
New Revision: 3e3c6f24ff85ea52ed67d4c26f1d3d0eacd1ad1b

URL: https://github.com/llvm/llvm-project/commit/3e3c6f24ff85ea52ed67d4c26f1d3d0eacd1ad1b
DIFF: https://github.com/llvm/llvm-project/commit/3e3c6f24ff85ea52ed67d4c26f1d3d0eacd1ad1b.diff

LOG: 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.

Added: 
    

Modified: 
    clang/lib/CodeGen/CodeGenAction.cpp
    lld/COFF/Symbols.cpp
    lld/ELF/SymbolTable.cpp
    lld/ELF/Symbols.cpp
    lld/MachO/Symbols.cpp
    lld/wasm/Symbols.cpp
    llvm/docs/ReleaseNotes.rst
    llvm/include/llvm/Demangle/Demangle.h
    llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
    llvm/lib/Demangle/Demangle.cpp
    llvm/lib/IR/DiagnosticInfo.cpp
    llvm/tools/llvm-objdump/ELFDump.cpp
    llvm/tools/llvm-objdump/XCOFFDump.cpp
    llvm/tools/llvm-objdump/llvm-objdump.cpp
    llvm/tools/llvm-readobj/ELFDumper.cpp
    llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp
index b054147e2d0ce..29adf88acd704 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 f4efcf2266cd6..c042386e01064 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 c5ef6d3f0cbf3..f09d0d7f90958 100644
--- a/lld/ELF/SymbolTable.cpp
+++ b/lld/ELF/SymbolTable.cpp
@@ -145,16 +145,13 @@ StringMap<SmallVector<Symbol *, 0>> &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 840385aabea35..62a8a3c30664e 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 660826edc4f86..cb3b271a19124 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 2adf72b6965ca..567ff49dfa444 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 100a424ed7717..c764a50f88b22 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 567d72168a642..855ab1f57512f 100644
--- a/llvm/include/llvm/Demangle/Demangle.h
+++ b/llvm/include/llvm/Demangle/Demangle.h
@@ -11,7 +11,6 @@
 
 #include <cstddef>
 #include <string>
-#include <string_view>
 
 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 c3ca6df91933f..2139124873aed 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 c7cfae79ea839..68eddde056021 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 <cstdlib>
+#include <cstring>
 
-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 342c4cbbc39d6..a19d9d607f4b6 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 6e422042c982a..b0dd0dccfe8f4 100644
--- a/llvm/tools/llvm-objdump/ELFDump.cpp
+++ b/llvm/tools/llvm-objdump/ELFDump.cpp
@@ -108,7 +108,10 @@ static Error getRelocationValueString(const ELFObjectFile<ELFT> *Obj,
       Expected<StringRef> 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 4849b20f1166b..7171e2eb6eb37 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 9d4a2d4f1eef2..5abcdcc788441 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 7627bed243edd..43d9a0f576aca 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -908,7 +908,7 @@ ELFDumper<ELFT>::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 <typename ELFT>

diff  --git a/llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp b/llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
index 9cc18f80910d4..71946f438475d 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;


        


More information about the cfe-commits mailing list