jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith.
Herald added subscribers: ormris, steven_wu, hiraditya.
Herald added a reviewer: alexander-shaposhnikov.
Herald added a reviewer: shafik.
Herald added a reviewer: rupprecht.
Herald added a reviewer: jhenderson.
jansvoboda11 requested review of this revision.
Herald added subscribers: llvm-commits, lldb-commits, cfe-commits, MaskRay.
Herald added projects: clang, LLDB, LLVM.
Most of `MemoryBuffer` interfaces expose a `RequiresNullTerminator` parameter
that's being used to:
- determine how to open a file (`mmap` vs `open`),
- assert newly initialized buffer indeed has an implicit null terminator.
This patch adds the paramater to the `SmallVectorMemoryBuffer` constructors,
meaning:
- null terminator can now be added to `SmallVector`s that didn't have one
before,
- `SmallVectors` that had a null terminator before keep it even after the move.
In line with existing code, the new parameter is defaulted to `true`. This
patch makes sure all calls to the `SmallVectorMemoryBuffer` constructor set it
to `false` to preserve the current semantics.
As a drive-by fix, this patch removes unused `#include`s of
`SmallVectorMemoryBuffer.h` and ensures all callers use the `auto Var =
std::make_unique<SmallVectorMemoryBuffer>(...);` pattern.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D115331
Files:
clang/unittests/AST/ASTImporterTest.cpp
clang/unittests/Frontend/TextDiagnosticTest.cpp
lldb/unittests/Expression/CppModuleConfigurationTest.cpp
llvm/include/llvm/Support/SmallVectorMemoryBuffer.h
llvm/lib/ExecutionEngine/MCJIT/MCJIT.cpp
llvm/lib/ExecutionEngine/Orc/CompileUtils.cpp
llvm/lib/LTO/LTOBackend.cpp
llvm/lib/LTO/ThinLTOCodeGenerator.cpp
llvm/lib/Object/ArchiveWriter.cpp
llvm/lib/Object/MachOUniversalWriter.cpp
llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp
llvm/tools/llvm-objcopy/llvm-objcopy.cpp
llvm/unittests/Support/MemoryBufferTest.cpp
Index: llvm/unittests/Support/MemoryBufferTest.cpp
===================================================================
--- llvm/unittests/Support/MemoryBufferTest.cpp
+++ llvm/unittests/Support/MemoryBufferTest.cpp
@@ -11,6 +11,7 @@
//===----------------------------------------------------------------------===//
#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/SmallVectorMemoryBuffer.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/FileUtilities.h"
@@ -408,4 +409,27 @@
EXPECT_EQ(MB->getBufferSize(), std::size_t(FileWrites * 8));
EXPECT_TRUE(MB->getBuffer().startswith("01234567"));
}
+
+// Test that SmallVector without a null terminator gets one.
+TEST(SmallVectorMemoryBufferTest, WithoutNullTerminatorRequiresNullTerminator) {
+ SmallString<0> Data("some data");
+
+ SmallVectorMemoryBuffer MB(std::move(Data),
+ /*RequiresNullTerminator=*/true);
+ EXPECT_EQ(MB.getBufferSize(), 9u);
+ EXPECT_EQ(MB.getBufferEnd()[0], '\0');
+}
+
+// Test that SmallVector with a null terminator keeps it.
+TEST(SmallVectorMemoryBufferTest, WithNullTerminatorRequiresNullTerminator) {
+ SmallString<0> Data("some data");
+ Data.push_back('\0');
+ Data.pop_back();
+
+ SmallVectorMemoryBuffer MB(std::move(Data),
+ /*RequiresNullTerminator=*/true);
+ EXPECT_EQ(MB.getBufferSize(), 9u);
+ EXPECT_EQ(MB.getBufferEnd()[0], '\0');
}
+
+} // namespace
Index: llvm/tools/llvm-objcopy/llvm-objcopy.cpp
===================================================================
--- llvm/tools/llvm-objcopy/llvm-objcopy.cpp
+++ llvm/tools/llvm-objcopy/llvm-objcopy.cpp
@@ -236,7 +236,8 @@
return createFileError(Ar.getFileName(), Member.takeError());
Member->Buf = std::make_unique<SmallVectorMemoryBuffer>(
- std::move(Buffer), ChildNameOrErr.get());
+ std::move(Buffer), ChildNameOrErr.get(),
+ /*RequiresNullTerminator=*/false);
Member->MemberName = Member->Buf->getBufferIdentifier();
NewArchiveMembers.push_back(std::move(*Member));
}
Index: llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp
===================================================================
--- llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp
+++ llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp
@@ -476,9 +476,8 @@
**ObjOrErr, MemStream))
return E;
- std::unique_ptr<MemoryBuffer> MB =
- std::make_unique<SmallVectorMemoryBuffer>(std::move(Buffer),
- ArchFlagName);
+ auto MB = std::make_unique<SmallVectorMemoryBuffer>(
+ std::move(Buffer), ArchFlagName, /*RequiresNullTerminator=*/false);
Expected<std::unique_ptr<Binary>> BinaryOrErr = object::createBinary(*MB);
if (!BinaryOrErr)
return BinaryOrErr.takeError();
Index: llvm/lib/Object/MachOUniversalWriter.cpp
===================================================================
--- llvm/lib/Object/MachOUniversalWriter.cpp
+++ llvm/lib/Object/MachOUniversalWriter.cpp
@@ -19,7 +19,6 @@
#include "llvm/Object/IRObjectFile.h"
#include "llvm/Object/MachO.h"
#include "llvm/Object/MachOUniversal.h"
-#include "llvm/Support/SmallVectorMemoryBuffer.h"
using namespace llvm;
using namespace object;
Index: llvm/lib/Object/ArchiveWriter.cpp
===================================================================
--- llvm/lib/Object/ArchiveWriter.cpp
+++ llvm/lib/Object/ArchiveWriter.cpp
@@ -696,7 +696,7 @@
return std::move(E);
return std::make_unique<SmallVectorMemoryBuffer>(
- std::move(ArchiveBufferVector));
+ std::move(ArchiveBufferVector), /*RequiresNullTerminator=*/false);
}
} // namespace llvm
Index: llvm/lib/LTO/ThinLTOCodeGenerator.cpp
===================================================================
--- llvm/lib/LTO/ThinLTOCodeGenerator.cpp
+++ llvm/lib/LTO/ThinLTOCodeGenerator.cpp
@@ -378,7 +378,8 @@
// Run codegen now. resulting binary is in OutputBuffer.
PM.run(TheModule);
}
- return std::make_unique<SmallVectorMemoryBuffer>(std::move(OutputBuffer));
+ return std::make_unique<SmallVectorMemoryBuffer>(
+ std::move(OutputBuffer), /*RequiresNullTerminator=*/false);
}
/// Manage caching for a single Module.
@@ -541,7 +542,8 @@
auto Index = buildModuleSummaryIndex(TheModule, nullptr, &PSI);
WriteBitcodeToFile(TheModule, OS, true, &Index);
}
- return std::make_unique<SmallVectorMemoryBuffer>(std::move(OutputBuffer));
+ return std::make_unique<SmallVectorMemoryBuffer>(
+ std::move(OutputBuffer), /*RequiresNullTerminator=*/false);
}
return codegenModule(TheModule, TM);
Index: llvm/lib/LTO/LTOBackend.cpp
===================================================================
--- llvm/lib/LTO/LTOBackend.cpp
+++ llvm/lib/LTO/LTOBackend.cpp
@@ -37,7 +37,6 @@
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Program.h"
-#include "llvm/Support/SmallVectorMemoryBuffer.h"
#include "llvm/Support/ThreadPool.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/Target/TargetMachine.h"
Index: llvm/lib/ExecutionEngine/Orc/CompileUtils.cpp
===================================================================
--- llvm/lib/ExecutionEngine/Orc/CompileUtils.cpp
+++ llvm/lib/ExecutionEngine/Orc/CompileUtils.cpp
@@ -53,7 +53,8 @@
}
auto ObjBuffer = std::make_unique<SmallVectorMemoryBuffer>(
- std::move(ObjBufferSV), M.getModuleIdentifier() + "-jitted-objectbuffer");
+ std::move(ObjBufferSV), M.getModuleIdentifier() + "-jitted-objectbuffer",
+ /*RequiresNullTerminator=*/false);
auto Obj = object::ObjectFile::createObjectFile(ObjBuffer->getMemBufferRef());
Index: llvm/lib/ExecutionEngine/MCJIT/MCJIT.cpp
===================================================================
--- llvm/lib/ExecutionEngine/MCJIT/MCJIT.cpp
+++ llvm/lib/ExecutionEngine/MCJIT/MCJIT.cpp
@@ -170,8 +170,8 @@
PM.run(*M);
// Flush the output buffer to get the generated code into memory
- std::unique_ptr<MemoryBuffer> CompiledObjBuffer(
- new SmallVectorMemoryBuffer(std::move(ObjBufferSV)));
+ auto CompiledObjBuffer = std::make_unique<SmallVectorMemoryBuffer>(
+ std::move(ObjBufferSV), /*RequiresNullTerminator=*/false);
// If we have an object cache, tell it about the new object.
// Note that we're using the compiled image, not the loaded image (as below).
Index: llvm/include/llvm/Support/SmallVectorMemoryBuffer.h
===================================================================
--- llvm/include/llvm/Support/SmallVectorMemoryBuffer.h
+++ llvm/include/llvm/Support/SmallVectorMemoryBuffer.h
@@ -28,17 +28,21 @@
/// MemoryBuffer).
class SmallVectorMemoryBuffer : public MemoryBuffer {
public:
- /// Construct an SmallVectorMemoryBuffer from the given SmallVector
- /// r-value.
- SmallVectorMemoryBuffer(SmallVectorImpl<char> &&SV)
- : SV(std::move(SV)), BufferName("<in-memory object>") {
- init(this->SV.begin(), this->SV.end(), false);
- }
-
- /// Construct a named SmallVectorMemoryBuffer from the given
- /// SmallVector r-value and StringRef.
- SmallVectorMemoryBuffer(SmallVectorImpl<char> &&SV, StringRef Name)
+ /// Construct a SmallVectorMemoryBuffer from the given SmallVector r-value.
+ SmallVectorMemoryBuffer(SmallVectorImpl<char> &&SV,
+ bool RequiresNullTerminator = true)
+ : SmallVectorMemoryBuffer(std::move(SV), "<in-memory object>",
+ RequiresNullTerminator) {}
+
+ /// Construct a named SmallVectorMemoryBuffer from the given SmallVector
+ /// r-value and StringRef.
+ SmallVectorMemoryBuffer(SmallVectorImpl<char> &&SV, StringRef Name,
+ bool RequiresNullTerminator = true)
: SV(std::move(SV)), BufferName(std::string(Name)) {
+ if (RequiresNullTerminator) {
+ this->SV.push_back('\0');
+ this->SV.pop_back();
+ }
init(this->SV.begin(), this->SV.end(), false);
}
Index: lldb/unittests/Expression/CppModuleConfigurationTest.cpp
===================================================================
--- lldb/unittests/Expression/CppModuleConfigurationTest.cpp
+++ lldb/unittests/Expression/CppModuleConfigurationTest.cpp
@@ -11,7 +11,6 @@
#include "TestingSupport/SubsystemRAII.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/HostInfo.h"
-#include "llvm/Support/SmallVectorMemoryBuffer.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
Index: clang/unittests/Frontend/TextDiagnosticTest.cpp
===================================================================
--- clang/unittests/Frontend/TextDiagnosticTest.cpp
+++ clang/unittests/Frontend/TextDiagnosticTest.cpp
@@ -54,7 +54,7 @@
llvm::SmallVector<char, 64> buffer;
buffer.append(main_file_contents.begin(), main_file_contents.end());
auto file_contents = std::make_unique<llvm::SmallVectorMemoryBuffer>(
- std::move(buffer), file_path);
+ std::move(buffer), file_path, /*RequiresNullTerminator=*/false);
SrcMgr.overrideFileContents(fe, std::move(file_contents));
// Create the actual file id and use it as the main file.
Index: clang/unittests/AST/ASTImporterTest.cpp
===================================================================
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -6452,8 +6452,8 @@
llvm::SmallVector<char, 64> Buffer;
Buffer.append(Contents.begin(), Contents.end());
- auto FileContents =
- std::make_unique<llvm::SmallVectorMemoryBuffer>(std::move(Buffer), Path);
+ auto FileContents = std::make_unique<llvm::SmallVectorMemoryBuffer>(
+ std::move(Buffer), Path, /*RequiresNullTerminator=*/false);
FromSM.overrideFileContents(&FE, std::move(FileContents));
// Import the VarDecl to trigger the importing of the FileID.
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
