Author: Jan Svoboda Date: 2021-12-09T11:32:13+01:00 New Revision: d0262c2394f46bb7da2a75529413d625c70908e5
URL: https://github.com/llvm/llvm-project/commit/d0262c2394f46bb7da2a75529413d625c70908e5 DIFF: https://github.com/llvm/llvm-project/commit/d0262c2394f46bb7da2a75529413d625c70908e5.diff LOG: [llvm] Add null-termination capability to SmallVectorMemoryBuffer 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. Reviewed By: dexonsmith Differential Revision: https://reviews.llvm.org/D115331 Added: Modified: clang/unittests/AST/ASTImporterTest.cpp clang/unittests/Frontend/TextDiagnosticTest.cpp llvm/include/llvm/Support/SmallVectorMemoryBuffer.h llvm/lib/ExecutionEngine/MCJIT/MCJIT.cpp llvm/lib/ExecutionEngine/Orc/CompileUtils.cpp llvm/lib/LTO/ThinLTOCodeGenerator.cpp llvm/lib/Object/ArchiveWriter.cpp llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp llvm/tools/llvm-objcopy/llvm-objcopy.cpp llvm/unittests/Support/MemoryBufferTest.cpp Removed: ################################################################################ diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 83ccdb5caeaff..9e8bee9c10b3b 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -6452,8 +6452,8 @@ TEST_P(ImportSourceLocations, OverwrittenFileBuffer) { 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. diff --git a/clang/unittests/Frontend/TextDiagnosticTest.cpp b/clang/unittests/Frontend/TextDiagnosticTest.cpp index 220cff6643d50..3acd89a985ab8 100644 --- a/clang/unittests/Frontend/TextDiagnosticTest.cpp +++ b/clang/unittests/Frontend/TextDiagnosticTest.cpp @@ -54,7 +54,7 @@ TEST(TextDiagnostic, ShowLine) { 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. diff --git a/llvm/include/llvm/Support/SmallVectorMemoryBuffer.h b/llvm/include/llvm/Support/SmallVectorMemoryBuffer.h index af748cf962b00..f7f2d4e54e705 100644 --- a/llvm/include/llvm/Support/SmallVectorMemoryBuffer.h +++ b/llvm/include/llvm/Support/SmallVectorMemoryBuffer.h @@ -28,17 +28,21 @@ namespace llvm { /// 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); } diff --git a/llvm/lib/ExecutionEngine/MCJIT/MCJIT.cpp b/llvm/lib/ExecutionEngine/MCJIT/MCJIT.cpp index 200f42aec067a..ed912280ac826 100644 --- a/llvm/lib/ExecutionEngine/MCJIT/MCJIT.cpp +++ b/llvm/lib/ExecutionEngine/MCJIT/MCJIT.cpp @@ -170,8 +170,8 @@ std::unique_ptr<MemoryBuffer> MCJIT::emitObject(Module *M) { 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). diff --git a/llvm/lib/ExecutionEngine/Orc/CompileUtils.cpp b/llvm/lib/ExecutionEngine/Orc/CompileUtils.cpp index f8efed15edea3..f342470052581 100644 --- a/llvm/lib/ExecutionEngine/Orc/CompileUtils.cpp +++ b/llvm/lib/ExecutionEngine/Orc/CompileUtils.cpp @@ -53,7 +53,8 @@ Expected<SimpleCompiler::CompileResult> SimpleCompiler::operator()(Module &M) { } 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()); diff --git a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp index 9474d8c9dafbf..9aea27f0fdbaf 100644 --- a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp +++ b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp @@ -378,7 +378,8 @@ std::unique_ptr<MemoryBuffer> codegenModule(Module &TheModule, // 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 @@ ProcessThinLTOModule(Module &TheModule, ModuleSummaryIndex &Index, 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); diff --git a/llvm/lib/Object/ArchiveWriter.cpp b/llvm/lib/Object/ArchiveWriter.cpp index ce997464caa73..da8bcec7f3d42 100644 --- a/llvm/lib/Object/ArchiveWriter.cpp +++ b/llvm/lib/Object/ArchiveWriter.cpp @@ -696,7 +696,7 @@ writeArchiveToBuffer(ArrayRef<NewArchiveMember> NewMembers, bool WriteSymtab, return std::move(E); return std::make_unique<SmallVectorMemoryBuffer>( - std::move(ArchiveBufferVector)); + std::move(ArchiveBufferVector), /*RequiresNullTerminator=*/false); } } // namespace llvm diff --git a/llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp b/llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp index 9e7b91d730579..915394b65b12d 100644 --- a/llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp +++ b/llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp @@ -476,9 +476,8 @@ Error objcopy::macho::executeObjcopyOnMachOUniversalBinary( **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(); diff --git a/llvm/tools/llvm-objcopy/llvm-objcopy.cpp b/llvm/tools/llvm-objcopy/llvm-objcopy.cpp index ad166487eb78b..a5963985f78ab 100644 --- a/llvm/tools/llvm-objcopy/llvm-objcopy.cpp +++ b/llvm/tools/llvm-objcopy/llvm-objcopy.cpp @@ -236,7 +236,8 @@ createNewArchiveMembers(const MultiFormatConfig &Config, const Archive &Ar) { 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)); } diff --git a/llvm/unittests/Support/MemoryBufferTest.cpp b/llvm/unittests/Support/MemoryBufferTest.cpp index 97bb62df393bb..c3e7b3c926a6c 100644 --- a/llvm/unittests/Support/MemoryBufferTest.cpp +++ b/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 @@ TEST_F(MemoryBufferTest, mmapVolatileNoNull) { 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits