[Demangle] remove unused status param of itaniumDemangle

[nickdesaulniers]

Summary

No call sites interpreted this value meaningfully. Simplify this
interface.

[MaskRay]

I think the signature llvm::itaniumDemangle was to match __cxxabiv1::__cxa_demangle: https://itanium-cxx-abi.github.io/cxx-abi/abi.html#demangler

I agree that the status parameter is unneeded by most users. The error categories distinguished by status become unuseful after the buf and n parameters were removed.

This change looks good. I am unfamiliar with how LLVMDemangle code is copied to libcxxabi. Does this change affect libcxxabi/src/cxa_demangle.cpp? If no, I think the patch is good to go.

[MaskRay]

llvm/include/llvm/Demangle/Demangle.h

31–34

Place non-null and a pointer to a NUL-terminated C style string together? I.e. move if successful before or after them.

Also explain when the return value may be a null.

[nickdesaulniers]

• update comment, change code to return early to reduce indentation

[nickdesaulniers]

In D149707#4316149, @MaskRay wrote:

I think the signature llvm::itaniumDemangle was to match __cxxabiv1::__cxa_demangle: https://itanium-cxx-abi.github.io/cxx-abi/abi.html#demangler

Ah, that makes sense for "historically, why did this interface look like this?"

I agree that the status parameter is unneeded by most users. The error categories distinguished by status become unuseful after the buf and n parameters were removed.

This change looks good. I am unfamiliar with how LLVMDemangle code is copied to libcxxabi. Does this change affect libcxxabi/src/cxa_demangle.cpp? If no, I think the patch is good to go.

Good question; I believe it does not. In my change to llvm/lib/Demangle/ItaniumDemangle.cpp, you can see in the context on phab on line 363 the comment:

// Code beyond this point should not be synchronized with libc++abi.

(My changes are beyond that point).

libcxxabi's demangler only concerns itself with itanium style demangling, not microsoft, rust or D demangling. llvm::itaniumDemangle uses a ManglingParser which is the first point of interface into code that's synchronized between llvm/ and libcxxabi/. This change is not something that is downstream of libcxxabi (and thus has no need to be copied back upstream).

[MaskRay]

Nice!

[barannikov88]

@nickdesaulniers This function is used in a couple more places:

/mnt/d/llvm-project/llvm/lib/Target/SPIRV/SPIRVUtils.cpp:298:25: error: no matching function for call to 'itaniumDemangle'

char *DemangledName = itaniumDemangle(Name.data(), nullptr, &n, &Status);
                      ^~~~~~~~~~~~~~~

/mnt/d/llvm-project/llvm/lib/Target/SPIRV/SPIRVRegularizer.cpp:176:19: error: no matching function for call to 'itaniumDemangle'

char *NameStr = itaniumDemangle(F->getName().data(), nullptr, &n, &status);
                ^~~~~~~~~~~~~~~

[nickdesaulniers]

In D149707#4320929, @barannikov88 wrote:

@nickdesaulniers This function is used in a couple more places:

/mnt/d/llvm-project/llvm/lib/Target/SPIRV/SPIRVUtils.cpp:298:25: error: no matching function for call to 'itaniumDemangle'

char *DemangledName = itaniumDemangle(Name.data(), nullptr, &n, &Status);
                      ^~~~~~~~~~~~~~~

/mnt/d/llvm-project/llvm/lib/Target/SPIRV/SPIRVRegularizer.cpp:176:19: error: no matching function for call to 'itaniumDemangle'

char *NameStr = itaniumDemangle(F->getName().data(), nullptr, &n, &status);
                ^~~~~~~~~~~~~~~

Thanks for the report. Looks like my code indexer is only indexing backends I build; none of the experimental ones were enabled. I'm working on a fix immediately, and will triple check for other callers using grep.

[nickdesaulniers]

In D149707#4322536, @nickdesaulniers wrote:

In D149707#4320929, @barannikov88 wrote:

@nickdesaulniers This function is used in a couple more places:

/mnt/d/llvm-project/llvm/lib/Target/SPIRV/SPIRVUtils.cpp:298:25: error: no matching function for call to 'itaniumDemangle'

char *DemangledName = itaniumDemangle(Name.data(), nullptr, &n, &Status);
                      ^~~~~~~~~~~~~~~

/mnt/d/llvm-project/llvm/lib/Target/SPIRV/SPIRVRegularizer.cpp:176:19: error: no matching function for call to 'itaniumDemangle'

char *NameStr = itaniumDemangle(F->getName().data(), nullptr, &n, &status);
                ^~~~~~~~~~~~~~~

Thanks for the report. Looks like my code indexer is only indexing backends I build; none of the experimental ones were enabled. I'm working on a fix immediately, and will triple check for other callers using grep.

Nevermind, @MaskRay fixed this up in 8e6b3cc4aeedb864a983d849498c03b074570406. Thanks @MaskRay !

[MaskRay]

I missed SPIRV in my testing of this patch as well. I now use -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD='ARC;AVR;DirectX;LoongArch;M68k;SPIRV;Xtensa' :)

llvm/include/llvm/Demangle/Demangle.h

 enum : int {
   demangle_unknown_error = -4,
   demangle_invalid_args = -3,
   demangle_invalid_mangled_name = -2,
   demangle_memory_alloc_failure = -1,
   demangle_success = 0,
 };
 
-char *itaniumDemangle(const char *mangled_name, int *status);
+/// Returns a non-NULL pointer to a NUL-terminated C style string
+/// that should be explicitly freed, if successful. Otherwise, may return
+/// nullptr if mangled_name is not a valid mangling or is nullptr.
+char *itaniumDemangle(const char *mangled_name);
 
 enum MSDemangleFlags {
   MSDF_None = 0,
   MSDF_DumpBackrefs = 1 << 0,
   MSDF_NoAccessSpecifier = 1 << 1,
   MSDF_NoCallingConvention = 1 << 2,
   MSDF_NoReturnType = 1 << 3,
   MSDF_NoMemberType = 1 << 4,

llvm/lib/Demangle/Demangle.cpp

std::string llvm::demangle(const std::string &MangledName) {
   }
 
   return MangledName;
 }
 
 bool llvm::nonMicrosoftDemangle(const char *MangledName, std::string &Result) {
   char *Demangled = nullptr;
   if (isItaniumEncoding(MangledName))
-    Demangled = itaniumDemangle(MangledName, nullptr);
+    Demangled = itaniumDemangle(MangledName);
   else if (isRustEncoding(MangledName))
     Demangled = rustDemangle(MangledName);
   else if (isDLangEncoding(MangledName))
     Demangled = dlangDemangle(MangledName);
 
   if (!Demangled)
     return false;
 
   Result = Demangled;
   std::free(Demangled);
   return true;
 }

llvm/lib/Demangle/ItaniumDemangle.cpp

 }  // unnamed namespace
 
 //===----------------------------------------------------------------------===//
 // Code beyond this point should not be synchronized with libc++abi.
 //===----------------------------------------------------------------------===//
 
 using Demangler = itanium_demangle::ManglingParser<DefaultAllocator>;
 
-char *llvm::itaniumDemangle(const char *MangledName, int *Status) {
-  if (MangledName == nullptr) {
-    if (Status)
-      *Status = demangle_invalid_args;
+char *llvm::itaniumDemangle(const char *MangledName) {
+  if (!MangledName)
     return nullptr;
-  }
 
-  int InternalStatus = demangle_success;
   Demangler Parser(MangledName, MangledName + std::strlen(MangledName));
   Node *AST = Parser.parse();
+  if (!AST)
+    return nullptr;
 
-  char *Buf;
-  if (AST == nullptr)
-    InternalStatus = demangle_invalid_mangled_name;
-  else {
-    OutputBuffer OB;
-    assert(Parser.ForwardTemplateRefs.empty());
-    AST->print(OB);
-    OB += '\0';
-    Buf = OB.getBuffer();
-  }
-
-  if (Status)
-    *Status = InternalStatus;
-  return InternalStatus == demangle_success ? Buf : nullptr;
+  return OB.getBuffer();
 }
 
 ItaniumPartialDemangler::ItaniumPartialDemangler()
     : RootNode(nullptr), Context(new Demangler{nullptr, nullptr}) {}
 
 ItaniumPartialDemangler::~ItaniumPartialDemangler() {
   delete static_cast<Demangler *>(Context);
 }

llvm/lib/ProfileData/GCOV.cpp

 // GCOVFunction implementation.
 
 StringRef GCOVFunction::getName(bool demangle) const {
   if (!demangle)
     return Name;
   if (demangled.empty()) {
     do {
       if (Name.startswith("_Z")) {
-        int status = 0;
         // Name is guaranteed to be NUL-terminated.
-        char *res = itaniumDemangle(Name.data(), &status);
-        if (status == 0) {
+        if (char *res = itaniumDemangle(Name.data())) {
           demangled = res;
           free(res);
           break;
         }
       }
       demangled = Name;
     } while (false);
   }

llvm/lib/Support/Unix/Signals.inc

for (int i = 0; i < depth; ++i) {
     else
       OS << format(" %-*s", width, name + 1);
 
     OS << format(" %#0*lx", (int)(sizeof(void *) * 2) + 2,
                  (unsigned long)StackTrace[i]);
 
     if (dlinfo.dli_sname != nullptr) {
       OS << ' ';
-      int res;
-      char *d = itaniumDemangle(dlinfo.dli_sname, &res);
-      if (!d)
-        OS << dlinfo.dli_sname;
-      else
+      if (char *d = itaniumDemangle(dlinfo.dli_sname)) {
         OS << d;
         free(d);
+      } else {
+        OS << dlinfo.dli_sname;
+      }
 
       OS << format(" + %tu", (static_cast<const char *>(StackTrace[i]) -
                               static_cast<const char *>(dlinfo.dli_saddr)));
     }
     OS << '\n';
   }
 #elif defined(HAVE_BACKTRACE)
   backtrace_symbols_fd(StackTrace, Depth, STDERR_FILENO);

llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp

static std::string demangle(const std::string &Mangled) {
   std::string Result;
   if (nonMicrosoftDemangle(DecoratedStr, Result))
     return Result;
 
   std::string Prefix;
   char *Undecorated = nullptr;
 
   if (Types)
-    Undecorated = itaniumDemangle(DecoratedStr, nullptr);
+    Undecorated = itaniumDemangle(DecoratedStr);
 
   if (!Undecorated && strncmp(DecoratedStr, "__imp_", 6) == 0) {
     Prefix = "import thunk for ";
-    Undecorated = itaniumDemangle(DecoratedStr + 6, nullptr);
+    Undecorated = itaniumDemangle(DecoratedStr + 6);
   }
 
   Result = Undecorated ? Prefix + Undecorated : Mangled;
   free(Undecorated);
   return Result;
 }
 
 // Split 'Source' on any character that fails to pass 'IsLegalChar'.  The

llvm/tools/llvm-itanium-demangle-fuzzer/llvm-itanium-demangle-fuzzer.cpp

 //===--- llvm-demangle-fuzzer.cpp - Fuzzer for the Itanium Demangler ------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Demangle/Demangle.h"
 
 #include <cstdint>
 #include <cstdlib>
 #include <string>
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
   std::string NullTerminatedString((const char *)Data, Size);
-  int status = 0;
-  if (char *demangle =
-          llvm::itaniumDemangle(NullTerminatedString.c_str(), &status))
+  if (char *demangle = llvm::itaniumDemangle(NullTerminatedString.c_str()))
     free(demangle);
 
   return 0;
 }

llvm/tools/llvm-objdump/MachODump.cpp

if (*ReferenceType == LLVMDisassembler_ReferenceType_In_Branch) {
     *ReferenceName = GuessIndirectSymbol(ReferenceValue, info);
     if (*ReferenceName != nullptr) {
       method_reference(info, ReferenceType, ReferenceName);
       if (*ReferenceType != LLVMDisassembler_ReferenceType_Out_Objc_Message)
         *ReferenceType = LLVMDisassembler_ReferenceType_Out_SymbolStub;
     } else if (SymbolName != nullptr && strncmp(SymbolName, "__Z", 3) == 0) {
       if (info->demangled_name != nullptr)
         free(info->demangled_name);
-      int status;
-      info->demangled_name = itaniumDemangle(SymbolName + 1, &status);
+      info->demangled_name = itaniumDemangle(SymbolName + 1);
       if (info->demangled_name != nullptr) {
         *ReferenceName = info->demangled_name;
         *ReferenceType = LLVMDisassembler_ReferenceType_DeMangled_Name;
       } else
         *ReferenceType = LLVMDisassembler_ReferenceType_InOut_None;
     } else
       *ReferenceType = LLVMDisassembler_ReferenceType_InOut_None;
   } else if (*ReferenceType == LLVMDisassembler_ReferenceType_In_PCrel_Load) {
else if (info->O->getArch() == Triple::aarch64 &&
             *ReferenceType == LLVMDisassembler_ReferenceType_In_ARM64_ADR)) {
     *ReferenceName =
         GuessLiteralPointer(ReferenceValue, ReferencePC, ReferenceType, info);
     if (*ReferenceName == nullptr)
       *ReferenceType = LLVMDisassembler_ReferenceType_InOut_None;
   } else if (SymbolName != nullptr && strncmp(SymbolName, "__Z", 3) == 0) {
     if (info->demangled_name != nullptr)
       free(info->demangled_name);
-    int status;
-    info->demangled_name = itaniumDemangle(SymbolName + 1, &status);
+    info->demangled_name = itaniumDemangle(SymbolName + 1);
     if (info->demangled_name != nullptr) {
       *ReferenceName = info->demangled_name;
       *ReferenceType = LLVMDisassembler_ReferenceType_DeMangled_Name;
     }
   }
   else {
     *ReferenceName = nullptr;
     *ReferenceType = LLVMDisassembler_ReferenceType_InOut_None;

llvm/tools/llvm-opt-report/OptReport.cpp

for (line_iterator LI(*Buf.get(), false); LI != line_iterator(); ++LI) {
           for (const auto &FuncName : FuncNameSet) {
             if (FirstFunc)
               FirstFunc = false;
             else
               OS << ", ";
 
             bool Printed = false;
             if (!NoDemangle) {
-              int Status = 0;
-              char *Demangled = itaniumDemangle(FuncName.c_str(), &Status);
-              if (Demangled && Status == 0) {
+              if (char *Demangled = itaniumDemangle(FuncName.c_str())) {
                 OS << Demangled;
                 Printed = true;
-              }
-
-              if (Demangled)
                 std::free(Demangled);
               }
+            }
 
             if (!Printed)
               OS << FuncName;
           }
 
           OS << ":\n";
         }