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.
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits