[libcxx-commits] [libcxxabi] 5b3ca24 - [demangler] Simplify OutputBuffer initialization

Nathan Sidwell via libcxx-commits libcxx-commits at lists.llvm.org
Tue Apr 26 04:23:24 PDT 2022


Author: Nathan Sidwell
Date: 2022-04-26T04:23:12-07:00
New Revision: 5b3ca24a35e91bf9c19af856e7f92c69b17f989e

URL: https://github.com/llvm/llvm-project/commit/5b3ca24a35e91bf9c19af856e7f92c69b17f989e
DIFF: https://github.com/llvm/llvm-project/commit/5b3ca24a35e91bf9c19af856e7f92c69b17f989e.diff

LOG: [demangler] Simplify OutputBuffer initialization

Every non-testcase use of OutputBuffer contains code to allocate an
initial buffer (using either 128 or 1024 as initial guesses). There's
now no need to do that, given recent changes to the buffer extension
heuristics -- it allocates a 1k(ish) buffer on first need.

Just pass in a buffer (if any) to the constructor.  Thus the
OutputBuffer's ownership of the buffer starts at its own lifetime
start. We can reduce the lifetime of this object in several cases.

That new constructor takes a 'size_t *' for the size argument, as all
uses with a non-null buffer are passing through a malloc'd buffer from
their own caller in this manner.

The buffer reset member function is never used, and is deleted.

The original buffer initialization code would return a failure code if
that first malloc failed.  Existing code either ignored that, called
std::terminate with a FIXME, or returned an error code.

But that's not foolproof anyway, as a subsequent buffer extension
failure ends up calling std::terminate. I am working on addressing
that unfortunate failure mode in a manner more consistent with the C++
ABI design.

Reviewed By: dblaikie

Differential Revision: https://reviews.llvm.org/D122604

Added: 
    

Modified: 
    libcxxabi/src/cxa_demangle.cpp
    libcxxabi/src/demangle/Utility.h
    llvm/include/llvm/Demangle/Utility.h
    llvm/lib/Demangle/DLangDemangle.cpp
    llvm/lib/Demangle/ItaniumDemangle.cpp
    llvm/lib/Demangle/MicrosoftDemangle.cpp
    llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
    llvm/lib/Demangle/RustDemangle.cpp

Removed: 
    


################################################################################
diff  --git a/libcxxabi/src/cxa_demangle.cpp b/libcxxabi/src/cxa_demangle.cpp
index ddab6d33358a5..7baac680074ce 100644
--- a/libcxxabi/src/cxa_demangle.cpp
+++ b/libcxxabi/src/cxa_demangle.cpp
@@ -386,15 +386,12 @@ __cxa_demangle(const char *MangledName, char *Buf, size_t *N, int *Status) {
 
   int InternalStatus = demangle_success;
   Demangler Parser(MangledName, MangledName + std::strlen(MangledName));
-  OutputBuffer O;
-
   Node *AST = Parser.parse();
 
   if (AST == nullptr)
     InternalStatus = demangle_invalid_mangled_name;
-  else if (!initializeOutputBuffer(Buf, N, O, 1024))
-    InternalStatus = demangle_memory_alloc_failure;
   else {
+    OutputBuffer O(Buf, N);
     assert(Parser.ForwardTemplateRefs.empty());
     AST->print(O);
     O += '\0';

diff  --git a/libcxxabi/src/demangle/Utility.h b/libcxxabi/src/demangle/Utility.h
index 6e0fa38632c89..00572880237a0 100644
--- a/libcxxabi/src/demangle/Utility.h
+++ b/libcxxabi/src/demangle/Utility.h
@@ -69,7 +69,9 @@ class OutputBuffer {
 
 public:
   OutputBuffer(char *StartBuf, size_t Size)
-      : Buffer(StartBuf), CurrentPosition(0), BufferCapacity(Size) {}
+      : Buffer(StartBuf), BufferCapacity(Size) {}
+  OutputBuffer(char *StartBuf, size_t *SizePtr)
+      : OutputBuffer(StartBuf, StartBuf ? *SizePtr : 0) {}
   OutputBuffer() = default;
   // Non-copyable
   OutputBuffer(const OutputBuffer &) = delete;
@@ -77,12 +79,6 @@ class OutputBuffer {
 
   operator StringView() const { return StringView(Buffer, CurrentPosition); }
 
-  void reset(char *Buffer_, size_t BufferCapacity_) {
-    CurrentPosition = 0;
-    Buffer = Buffer_;
-    BufferCapacity = BufferCapacity_;
-  }
-
   /// If a ParameterPackExpansion (or similar type) is encountered, the offset
   /// into the pack that we're currently printing.
   unsigned CurrentPackIndex = std::numeric_limits<unsigned>::max();
@@ -197,21 +193,6 @@ template <class T> class ScopedOverride {
   ScopedOverride &operator=(const ScopedOverride &) = delete;
 };
 
-inline bool initializeOutputBuffer(char *Buf, size_t *N, OutputBuffer &OB,
-                                   size_t InitSize) {
-  size_t BufferSize;
-  if (Buf == nullptr) {
-    Buf = static_cast<char *>(std::malloc(InitSize));
-    if (Buf == nullptr)
-      return false;
-    BufferSize = InitSize;
-  } else
-    BufferSize = *N;
-
-  OB.reset(Buf, BufferSize);
-  return true;
-}
-
 DEMANGLE_NAMESPACE_END
 
 #endif

diff  --git a/llvm/include/llvm/Demangle/Utility.h b/llvm/include/llvm/Demangle/Utility.h
index b17eadcfa3bee..7025469d03531 100644
--- a/llvm/include/llvm/Demangle/Utility.h
+++ b/llvm/include/llvm/Demangle/Utility.h
@@ -69,7 +69,9 @@ class OutputBuffer {
 
 public:
   OutputBuffer(char *StartBuf, size_t Size)
-      : Buffer(StartBuf), CurrentPosition(0), BufferCapacity(Size) {}
+      : Buffer(StartBuf), BufferCapacity(Size) {}
+  OutputBuffer(char *StartBuf, size_t *SizePtr)
+      : OutputBuffer(StartBuf, StartBuf ? *SizePtr : 0) {}
   OutputBuffer() = default;
   // Non-copyable
   OutputBuffer(const OutputBuffer &) = delete;
@@ -77,12 +79,6 @@ class OutputBuffer {
 
   operator StringView() const { return StringView(Buffer, CurrentPosition); }
 
-  void reset(char *Buffer_, size_t BufferCapacity_) {
-    CurrentPosition = 0;
-    Buffer = Buffer_;
-    BufferCapacity = BufferCapacity_;
-  }
-
   /// If a ParameterPackExpansion (or similar type) is encountered, the offset
   /// into the pack that we're currently printing.
   unsigned CurrentPackIndex = std::numeric_limits<unsigned>::max();
@@ -197,21 +193,6 @@ template <class T> class ScopedOverride {
   ScopedOverride &operator=(const ScopedOverride &) = delete;
 };
 
-inline bool initializeOutputBuffer(char *Buf, size_t *N, OutputBuffer &OB,
-                                   size_t InitSize) {
-  size_t BufferSize;
-  if (Buf == nullptr) {
-    Buf = static_cast<char *>(std::malloc(InitSize));
-    if (Buf == nullptr)
-      return false;
-    BufferSize = InitSize;
-  } else
-    BufferSize = *N;
-
-  OB.reset(Buf, BufferSize);
-  return true;
-}
-
 DEMANGLE_NAMESPACE_END
 
 #endif

diff  --git a/llvm/lib/Demangle/DLangDemangle.cpp b/llvm/lib/Demangle/DLangDemangle.cpp
index 7cecd80070878..b747b0f9cc677 100644
--- a/llvm/lib/Demangle/DLangDemangle.cpp
+++ b/llvm/lib/Demangle/DLangDemangle.cpp
@@ -548,9 +548,6 @@ char *llvm::dlangDemangle(const char *MangledName) {
     return nullptr;
 
   OutputBuffer Demangled;
-  if (!initializeOutputBuffer(nullptr, nullptr, Demangled, 1024))
-    return nullptr;
-
   if (strcmp(MangledName, "_Dmain") == 0) {
     Demangled << "D main";
   } else {

diff  --git a/llvm/lib/Demangle/ItaniumDemangle.cpp b/llvm/lib/Demangle/ItaniumDemangle.cpp
index 1c9209d8f3693..9b646ea800aa6 100644
--- a/llvm/lib/Demangle/ItaniumDemangle.cpp
+++ b/llvm/lib/Demangle/ItaniumDemangle.cpp
@@ -375,15 +375,12 @@ char *llvm::itaniumDemangle(const char *MangledName, char *Buf,
 
   int InternalStatus = demangle_success;
   Demangler Parser(MangledName, MangledName + std::strlen(MangledName));
-  OutputBuffer OB;
-
   Node *AST = Parser.parse();
 
   if (AST == nullptr)
     InternalStatus = demangle_invalid_mangled_name;
-  else if (!initializeOutputBuffer(Buf, N, OB, 1024))
-    InternalStatus = demangle_memory_alloc_failure;
   else {
+    OutputBuffer OB(Buf, N);
     assert(Parser.ForwardTemplateRefs.empty());
     AST->print(OB);
     OB += '\0';
@@ -427,9 +424,7 @@ bool ItaniumPartialDemangler::partialDemangle(const char *MangledName) {
 }
 
 static char *printNode(const Node *RootNode, char *Buf, size_t *N) {
-  OutputBuffer OB;
-  if (!initializeOutputBuffer(Buf, N, OB, 128))
-    return nullptr;
+  OutputBuffer OB(Buf, N);
   RootNode->print(OB);
   OB += '\0';
   if (N != nullptr)
@@ -472,9 +467,7 @@ char *ItaniumPartialDemangler::getFunctionDeclContextName(char *Buf,
     return nullptr;
   const Node *Name = static_cast<const FunctionEncoding *>(RootNode)->getName();
 
-  OutputBuffer OB;
-  if (!initializeOutputBuffer(Buf, N, OB, 128))
-    return nullptr;
+  OutputBuffer OB(Buf, N);
 
  KeepGoingLocalFunction:
   while (true) {
@@ -525,9 +518,7 @@ char *ItaniumPartialDemangler::getFunctionParameters(char *Buf,
     return nullptr;
   NodeArray Params = static_cast<FunctionEncoding *>(RootNode)->getParams();
 
-  OutputBuffer OB;
-  if (!initializeOutputBuffer(Buf, N, OB, 128))
-    return nullptr;
+  OutputBuffer OB(Buf, N);
 
   OB += '(';
   Params.printWithComma(OB);
@@ -543,9 +534,7 @@ char *ItaniumPartialDemangler::getFunctionReturnType(
   if (!isFunction())
     return nullptr;
 
-  OutputBuffer OB;
-  if (!initializeOutputBuffer(Buf, N, OB, 128))
-    return nullptr;
+  OutputBuffer OB(Buf, N);
 
   if (const Node *Ret =
           static_cast<const FunctionEncoding *>(RootNode)->getReturnType())

diff  --git a/llvm/lib/Demangle/MicrosoftDemangle.cpp b/llvm/lib/Demangle/MicrosoftDemangle.cpp
index aca8cf7a0a58c..26087a5b444c2 100644
--- a/llvm/lib/Demangle/MicrosoftDemangle.cpp
+++ b/llvm/lib/Demangle/MicrosoftDemangle.cpp
@@ -966,9 +966,6 @@ void Demangler::memorizeIdentifier(IdentifierNode *Identifier) {
   // Render this class template name into a string buffer so that we can
   // memorize it for the purpose of back-referencing.
   OutputBuffer OB;
-  if (!initializeOutputBuffer(nullptr, nullptr, OB, 1024))
-    // FIXME: Propagate out-of-memory as an error?
-    std::terminate();
   Identifier->output(OB, OF_Default);
   StringView Owned = copyString(OB);
   memorizeString(Owned);
@@ -1279,11 +1276,6 @@ Demangler::demangleStringLiteral(StringView &MangledName) {
 
   EncodedStringLiteralNode *Result = Arena.alloc<EncodedStringLiteralNode>();
 
-  // Must happen before the first `goto StringLiteralError`.
-  if (!initializeOutputBuffer(nullptr, nullptr, OB, 1024))
-    // FIXME: Propagate out-of-memory as an error?
-    std::terminate();
-
   // Prefix indicating the beginning of a string literal
   if (!MangledName.consumeFront("@_"))
     goto StringLiteralError;
@@ -1442,9 +1434,6 @@ Demangler::demangleLocallyScopedNamePiece(StringView &MangledName) {
 
   // Render the parent symbol's name into a buffer.
   OutputBuffer OB;
-  if (!initializeOutputBuffer(nullptr, nullptr, OB, 1024))
-    // FIXME: Propagate out-of-memory as an error?
-    std::terminate();
   OB << '`';
   Scope->output(OB, OF_Default);
   OB << '\'';
@@ -2307,8 +2296,6 @@ void Demangler::dumpBackReferences() {
 
   // Create an output stream so we can render each type.
   OutputBuffer OB;
-  if (!initializeOutputBuffer(nullptr, nullptr, OB, 1024))
-    std::terminate();
   for (size_t I = 0; I < Backrefs.FunctionParamCount; ++I) {
     OB.setCurrentPosition(0);
 
@@ -2335,7 +2322,6 @@ char *llvm::microsoftDemangle(const char *MangledName, size_t *NMangled,
                               char *Buf, size_t *N,
                               int *Status, MSDemangleFlags Flags) {
   Demangler D;
-  OutputBuffer OB;
 
   StringView Name{MangledName};
   SymbolNode *AST = D.parse(Name);
@@ -2360,9 +2346,8 @@ char *llvm::microsoftDemangle(const char *MangledName, size_t *NMangled,
   int InternalStatus = demangle_success;
   if (D.Error)
     InternalStatus = demangle_invalid_mangled_name;
-  else if (!initializeOutputBuffer(Buf, N, OB, 1024))
-    InternalStatus = demangle_memory_alloc_failure;
   else {
+    OutputBuffer OB(Buf, N);
     AST->output(OB, OF);
     OB += '\0';
     if (N != nullptr)

diff  --git a/llvm/lib/Demangle/MicrosoftDemangleNodes.cpp b/llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
index 494cdabad41f3..975649f28ad21 100644
--- a/llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
+++ b/llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
@@ -119,7 +119,6 @@ static void outputCallingConvention(OutputBuffer &OB, CallingConv CC) {
 
 std::string Node::toString(OutputFlags Flags) const {
   OutputBuffer OB;
-  initializeOutputBuffer(nullptr, nullptr, OB, 1024);
   this->output(OB, Flags);
   StringView SV = OB;
   std::string Owned(SV.begin(), SV.end());

diff  --git a/llvm/lib/Demangle/RustDemangle.cpp b/llvm/lib/Demangle/RustDemangle.cpp
index 32b10db2a968d..8c01155127d85 100644
--- a/llvm/lib/Demangle/RustDemangle.cpp
+++ b/llvm/lib/Demangle/RustDemangle.cpp
@@ -157,9 +157,6 @@ char *llvm::rustDemangle(const char *MangledName) {
     return nullptr;
 
   Demangler D;
-  if (!initializeOutputBuffer(nullptr, nullptr, D.Output, 1024))
-    return nullptr;
-
   if (!D.demangle(Mangled)) {
     std::free(D.Output.getBuffer());
     return nullptr;


        


More information about the libcxx-commits mailing list