RPCS3/llvm-mirror
1
0
Fork 0
mirror of https://github.com/RPCS3/llvm-mirror.git synced 2025-01-31 20:51:52 +01:00
Code Issues Projects Releases Wiki Activity

[Clang] Expose RequiresNullTerminator in FileManager.

Browse Source 
This is needed to fix the reason
0a2be46cfdb698fe (Modules: Invalidate out-of-date PCMs as they're
discovered) and 5b44a4b07fc1d ([modules] Do not cache invalid state for
modules that we attempted to load.) were reverted.

These patches changed Clang to use `isVolatile` when loading modules.
This had the side effect of not using mmap when loading modules, and
thus greatly increased memory usage.

The reason it wasn't using mmap is because `MemoryBuffer` plays some
games with file size when you request null termination, and it has to
disable these when `isVolatile` is set as the size may change by the
time it's mmapped. Clang by default passes
`RequiresNullTerminator = true`, and `shouldUseMmap` ignored if
`RequiresNullTerminator` was even requested.

This patch adds `RequiresNullTerminator` to the `FileManager` interface
so Clang can use it when loading modules, and changes `shouldUseMmap` to
only take volatility into account if `RequiresNullTerminator` is true.
This is fine as both `mmap` and a `read` loop are vulnerable to
modifying the file while reading, but are immune to the rename Clang
does when replacing a module file.

Differential Revision: https://reviews.llvm.org/D77772
This commit is contained in:
Michael Spencer 2020-04-08 20:29:39 -07:00
parent e5640d8442
commit 86423aa8b7
2 changed files with 29 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
lib/Support/MemoryBuffer.cpp
View File

@ -329,7 +329,7 @@ static bool shouldUseMmap(sys::fs::file_t FD,
// mmap may leave the buffer without null terminator if the file size changed
// by the time the last page is mapped in, so avoid it if the file size is
// likely to change.
if (IsVolatile)
if (IsVolatile && RequiresNullTerminator)
return false;
// We don't use mmap for small files because this can severely fragment our

28
unittests/Support/MemoryBufferTest.cpp
View File

@ -380,4 +380,32 @@ TEST_F(MemoryBufferTest, writeThroughFile) {
ASSERT_EQ(16u, MB.getBufferSize());
EXPECT_EQ("xxxxxxxxxxxxxxxx", MB.getBuffer());
}
TEST_F(MemoryBufferTest, mmapVolatileNoNull) {
// Verify that `MemoryBuffer::getOpenFile` will use mmap when
// `RequiresNullTerminator = false`, `IsVolatile = true`, and the file is
// large enough to use mmap.
//
// This is done because Clang should use this mode to open module files, and
// falling back to malloc for them causes a huge memory usage increase.
int FD;
SmallString<64> TestPath;
ASSERT_NO_ERROR(sys::fs::createTemporaryFile(
"MemoryBufferTest_mmapVolatileNoNull", "temp", FD, TestPath));
FileRemover Cleanup(TestPath);
raw_fd_ostream OF(FD, true);
// Create a file large enough to mmap. A 32KiB file should be enough.
for (unsigned i = 0; i < 0x1000; ++i)
OF << "01234567";
OF.flush();
auto MBOrError = MemoryBuffer::getOpenFile(FD, TestPath,
/*FileSize=*/-1, /*RequiresNullTerminator=*/false, /*IsVolatile=*/true);
ASSERT_NO_ERROR(MBOrError.getError())
OwningBuffer MB = std::move(*MBOrError);
EXPECT_EQ(MB->getBufferKind(), MemoryBuffer::MemoryBuffer_MMap);
EXPECT_EQ(MB->getBufferSize(), std::size_t(0x8000));
EXPECT_TRUE(MB->getBuffer().startswith("01234567"));
}
}