[PATCH] D124524: [demangler] Avoid nullptr UB

[Nathan Sidwell via Phabricator via llvm-commits]

urnathan created this revision.
urnathan added reviewers: dblaikie, kstoimenov.
Herald added a subscriber: hiraditya.
Herald added a project: All.
urnathan requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

this resolves a UB introduced by D122604 <https://reviews.llvm.org/D122604>

The microsoft demangler demangles string literal contents (sans
quotes), and can therefore demangle the empty string to nothing.  With
lazy buffer allocation, this results in a null buffer and consequent
detected UB with a memcpy.  However, the fault is earlier, with taking
a StringView of that null buffer.  This adds an allocateBuffer member,
to ensure that there is a non-null buffer, even when it is not needed.

A unit test also requires this, as it commences by inserting
no-characters and then checking for an empty string.

An assert is added on theOutputBuffer's conversion to StringView, to
catch future cases.

(My assumption that demangling either failed or always produced at
least one character turned out to be false in this one cases.  I do
not know why the Microsoft demangler behaves this way, as it appears
to decorate the demangled strings with quotes before presentation to
the user.)


https://reviews.llvm.org/D124524

Files:
  libcxxabi/src/demangle/Utility.h
  llvm/include/llvm/Demangle/Utility.h
  llvm/lib/Demangle/MicrosoftDemangle.cpp
  llvm/unittests/Demangle/OutputBufferTest.cpp


Index: llvm/unittests/Demangle/OutputBufferTest.cpp
===================================================================
--- llvm/unittests/Demangle/OutputBufferTest.cpp
+++ llvm/unittests/Demangle/OutputBufferTest.cpp
@@ -44,6 +44,8 @@
 TEST(OutputBufferTest, Insert) {
   OutputBuffer OB;
 
+  OB.allocateBuffer();
+
   OB.insert(0, "", 0);
   EXPECT_EQ("", toString(OB));
 
Index: llvm/lib/Demangle/MicrosoftDemangle.cpp
===================================================================
--- llvm/lib/Demangle/MicrosoftDemangle.cpp
+++ llvm/lib/Demangle/MicrosoftDemangle.cpp
@@ -1301,6 +1301,10 @@
     goto StringLiteralError;
   }
 
+  // We can demangle to an empty string, and need to avoid a null buffer in that
+  // case.
+  OB.allocateBuffer();
+
   // Encoded Length
   std::tie(StringByteSize, IsNegative) = demangleNumber(MangledName);
   if (Error || IsNegative || StringByteSize < (IsWcharT ? 2 : 1))
Index: llvm/include/llvm/Demangle/Utility.h
===================================================================
--- llvm/include/llvm/Demangle/Utility.h
+++ llvm/include/llvm/Demangle/Utility.h
@@ -75,7 +75,10 @@
   OutputBuffer(const OutputBuffer &) = delete;
   OutputBuffer &operator=(const OutputBuffer &) = delete;
 
-  operator StringView() const { return StringView(Buffer, CurrentPosition); }
+  operator StringView() const {
+    assert(Buffer && "Buffer cannot by nullptr");
+    return StringView(Buffer, CurrentPosition);
+  }
 
   void reset(char *Buffer_, size_t BufferCapacity_) {
     CurrentPosition = 0;
@@ -174,6 +177,8 @@
     return CurrentPosition ? Buffer[CurrentPosition - 1] : '\0';
   }
 
+  // Some uses require a buffer even when they write nothing.
+  void allocateBuffer() { grow(1); }
   bool empty() const { return CurrentPosition == 0; }
 
   char *getBuffer() { return Buffer; }
Index: libcxxabi/src/demangle/Utility.h
===================================================================
--- libcxxabi/src/demangle/Utility.h
+++ libcxxabi/src/demangle/Utility.h
@@ -75,7 +75,10 @@
   OutputBuffer(const OutputBuffer &) = delete;
   OutputBuffer &operator=(const OutputBuffer &) = delete;
 
-  operator StringView() const { return StringView(Buffer, CurrentPosition); }
+  operator StringView() const {
+    assert(Buffer && "Buffer cannot by nullptr");
+    return StringView(Buffer, CurrentPosition);
+  }
 
   void reset(char *Buffer_, size_t BufferCapacity_) {
     CurrentPosition = 0;
@@ -174,6 +177,8 @@
     return CurrentPosition ? Buffer[CurrentPosition - 1] : '\0';
   }
 
+  // Some uses require a buffer even when they write nothing.
+  void allocateBuffer() { grow(1); }
   bool empty() const { return CurrentPosition == 0; }
 
   char *getBuffer() { return Buffer; }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124524.425493.patch
Type: text/x-patch
Size: 2732 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220427/6095aeff/attachment.bin>