[llvm] dcac5a2 - Revert "Cache memory buffer's name length"

[Florian Hahn via llvm-commits]

Author: Florian Hahn
Date: 2022-11-29T13:22:52Z
New Revision: dcac5a2f7a58f6c66520a1147c0bf3d24177c30b

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

LOG: Revert "Cache memory buffer's name length"

This reverts commit 1824432174b3166b40bce59477beb5821170748e.

The change triggers an assertion when building clang on macOS:

FAILED: tools/clang/include/clang/Tooling/Syntax/Nodes.inc
cd /Users/florianhahn/projects/llvm-project/builds/release-with-assertions/ccache-stage1 && /Users/florianhahn/projects/llvm-project/builds/release-with-assertions/ccache-stage1/bin/clang-tblgen -gen-clang-syntax-node-list -I /Users/florianhahn/projects/llvm-project/clang/include/clang/Tooling/Syntax -I/Users/florianhahn/projects/llvm-project/clang/include -I/Users/florianhahn/projects/llvm-project/builds/release-with-assertions/ccache-stage1/tools/clang/include -I/Users/florianhahn/projects/llvm-project/builds/release-with-assertions/ccache-stage1/include -I/Users/florianhahn/projects/llvm-project/llvm/include /Users/florianhahn/projects/llvm-project/clang/include/clang/Tooling/Syntax/Nodes.td --write-if-changed -o tools/clang/include/clang/Tooling/Syntax/Nodes.inc -d tools/clang/include/clang/Tooling/Syntax/Nodes.inc.d
Assertion failed: ((!RequiresNullTerminator || BufEnd[0] == 0) && "Buffer is not null terminated!"), function init, file MemoryBuffer.cpp, line 52.
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
0  clang-tblgen             0x000000010466b68c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 56
1  clang-tblgen             0x000000010466a808 llvm::sys::RunSignalHandlers() + 64
2  clang-tblgen             0x000000010466bd60 SignalHandler(int) + 344
3  libsystem_platform.dylib 0x00000001877a92a4 _sigtramp + 56
4  libsystem_pthread.dylib  0x000000018777acec pthread_kill + 288
5  libsystem_c.dylib        0x00000001876b42c8 abort + 180
6  libsystem_c.dylib        0x00000001876b3620 err + 0
7  clang-tblgen             0x00000001046ce150 llvm::ErrorOr<std::__1::unique_ptr<llvm::MemoryBuffer, std::__1::default_delete<llvm::MemoryBuffer>>> getOpenFileImpl<llvm::MemoryBuffer>(int, llvm::Twine const&, unsigned long long, unsigned long long, long long, bool, bool, llvm::Optional<llvm::Align>) (.cold.1) + 0
8  clang-tblgen             0x000000010464a548 llvm::WritableMemoryBuffer::getNewUninitMemBuffer(unsigned long, llvm::Twine const&, llvm::Optional<llvm::Align>) + 496
9  clang-tblgen             0x000000010464a7a0 llvm::ErrorOr<std::__1::unique_ptr<llvm::MemoryBuffer, std::__1::default_delete<llvm::MemoryBuffer>>> getOpenFileImpl<llvm::MemoryBuffer>(int, llvm::Twine const&, unsigned long long, unsigned long long, long long, bool, bool, llvm::Optional<llvm::Align>) + 528
10 clang-tblgen             0x000000010464a254 llvm::ErrorOr<std::__1::unique_ptr<llvm::MemoryBuffer, std::__1::default_delete<llvm::MemoryBuffer>>> getFileAux<llvm::MemoryBuffer>(llvm::Twine const&, unsigned long long, unsigned long long, bool, bool, bool, llvm::Optional<llvm::Align>) + 176
11 clang-tblgen             0x000000010466fbbc llvm::TableGenMain(char const*, bool (*)(llvm::raw_ostream&, llvm::RecordKeeper&)) + 1428
12 clang-tblgen             0x0000000104626114 main + 156
13 dyld                     0x0000000187453e50 start + 2544

Added: 
    

Modified: 
    llvm/lib/Support/MemoryBuffer.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Support/MemoryBuffer.cpp b/llvm/lib/Support/MemoryBuffer.cpp
index 8f123a41473e..6bb046e9b3db 100644
--- a/llvm/lib/Support/MemoryBuffer.cpp
+++ b/llvm/lib/Support/MemoryBuffer.cpp
@@ -58,10 +58,12 @@ void MemoryBuffer::init(const char *BufStart, const char *BufEnd,
 // MemoryBufferMem implementation.
 //===----------------------------------------------------------------------===//
 
-/// CopyStringRef - Copies contents of a StringRef into a block of memory.
+/// CopyStringRef - Copies contents of a StringRef into a block of memory and
+/// null-terminates it.
 static void CopyStringRef(char *Memory, StringRef Data) {
   if (!Data.empty())
     memcpy(Memory, Data.data(), Data.size());
+  Memory[Data.size()] = 0; // Null terminate string.
 }
 
 namespace {
@@ -75,10 +77,8 @@ void *operator new(size_t N, const NamedBufferAlloc &Alloc) {
   SmallString<256> NameBuf;
   StringRef NameRef = Alloc.Name.toStringRef(NameBuf);
 
-  char *Mem =
-      static_cast<char *>(operator new(N + sizeof(size_t) + NameRef.size()));
-  *reinterpret_cast<size_t *>(Mem + N) = NameRef.size();
-  CopyStringRef(Mem + N + sizeof(size_t), NameRef);
+  char *Mem = static_cast<char *>(operator new(N + NameRef.size() + 1));
+  CopyStringRef(Mem + N, NameRef);
   return Mem;
 }
 
@@ -98,8 +98,7 @@ class MemoryBufferMem : public MB {
 
   StringRef getBufferIdentifier() const override {
     // The name is stored after the class itself.
-    return StringRef(reinterpret_cast<const char *>(this + 1) + sizeof(size_t),
-                     *reinterpret_cast<const size_t *>(this + 1));
+    return StringRef(reinterpret_cast<const char *>(this + 1));
   }
 
   MemoryBuffer::BufferKind getBufferKind() const override {
@@ -222,8 +221,7 @@ class MemoryBufferMMapFile : public MB {
 
   StringRef getBufferIdentifier() const override {
     // The name is stored after the class itself.
-    return StringRef(reinterpret_cast<const char *>(this + 1) + sizeof(size_t),
-                     *reinterpret_cast<const size_t *>(this + 1));
+    return StringRef(reinterpret_cast<const char *>(this + 1));
   }
 
   MemoryBuffer::BufferKind getBufferKind() const override {
@@ -303,8 +301,7 @@ WritableMemoryBuffer::getNewUninitMemBuffer(size_t Size,
   // that MemoryBuffer and data are aligned so PointerIntPair works with them.
   SmallString<256> NameBuf;
   StringRef NameRef = BufferName.toStringRef(NameBuf);
-
-  size_t StringLen = sizeof(MemBuffer) + sizeof(size_t) + NameRef.size();
+  size_t StringLen = sizeof(MemBuffer) + NameRef.size() + 1;
   size_t RealLen = StringLen + Size + 1 + BufAlign.value();
   if (RealLen <= Size) // Check for rollover.
     return nullptr;
@@ -313,15 +310,13 @@ WritableMemoryBuffer::getNewUninitMemBuffer(size_t Size,
     return nullptr;
 
   // The name is stored after the class itself.
-  *reinterpret_cast<size_t *>(Mem + sizeof(MemBuffer)) =
-      NameRef.size(); // Null terminate buffer.
-  CopyStringRef(Mem + sizeof(MemBuffer) + sizeof(size_t), NameRef);
+  CopyStringRef(Mem + sizeof(MemBuffer), NameRef);
 
   // The buffer begins after the name and must be aligned.
   char *Buf = (char *)alignAddr(Mem + StringLen, BufAlign);
+  Buf[Size] = 0; // Null terminate buffer.
 
   auto *Ret = new (Mem) MemBuffer(StringRef(Buf, Size), true);
-  Buf[Size] = 0; // Null terminate buffer.
   return std::unique_ptr<WritableMemoryBuffer>(Ret);
 }