dexonsmith created this revision.
dexonsmith added reviewers: arphaman, Bigcheese, jansvoboda11.
Herald added subscribers: ributzka, JDevlieghere.
Herald added a project: clang.
dexonsmith requested review of this revision.
This reverts commit b34632201987eed369bb7ef4646f341b901c95b8
<https://reviews.llvm.org/rGb34632201987eed369bb7ef4646f341b901c95b8>,
reapplying 3b18a594c7717a328c33b9c1eba675e9f4bd367c
<https://reviews.llvm.org/rG3b18a594c7717a328c33b9c1eba675e9f4bd367c> with an
additional
change to SourceManager.
SourceManager checks if the file entry's size matches the eventually
loaded buffer. This doesn't make sense for named pipes so I've disabled
that check. Since we can't trust `ContentsEntry->getSize()`, we also need
shift the check for files that are too large until after the buffer is
loaded... and load the buffer immediately in `createFileID` so that no
client gets a bad value from `ContentCache::getSize`.
My main goal with this commit is to remove a use of
`SourceManager::overrideFileContents`, whose existance blocks sinking
the content cache down to `FileManager`.
The original commit message follows:
Frontend: Sink named pipe logic from CompilerInstance down to FileManager
Remove compilicated logic from CompilerInstance::InitializeSourceManager
to deal with named pipes, updating FileManager::getBufferForFile to
handle it in a more straightforward way. The existing test at
clang/test/Misc/dev-fd-fs.c covers the new behaviour (just like it did
the old behaviour).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D92531
Files:
clang/lib/Basic/FileManager.cpp
clang/lib/Basic/SourceManager.cpp
clang/lib/Frontend/CompilerInstance.cpp
Index: clang/lib/Frontend/CompilerInstance.cpp
===================================================================
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -858,30 +858,8 @@
}
FileEntryRef File = *FileOrErr;
- // The natural SourceManager infrastructure can't currently handle named
- // pipes, but we would at least like to accept them for the main
- // file. Detect them here, read them with the volatile flag so FileMgr will
- // pick up the correct size, and simply override their contents as we do for
- // STDIN.
- if (File.getFileEntry().isNamedPipe()) {
- auto MB =
- FileMgr.getBufferForFile(&File.getFileEntry(), /*isVolatile=*/true);
- if (MB) {
- // Create a new virtual file that will have the correct size.
- const FileEntry *FE =
- FileMgr.getVirtualFile(InputFile, (*MB)->getBufferSize(), 0);
- SourceMgr.overrideFileContents(FE, std::move(*MB));
- SourceMgr.setMainFileID(
- SourceMgr.createFileID(FE, SourceLocation(), Kind));
- } else {
- Diags.Report(diag::err_cannot_open_file) << InputFile
- << MB.getError().message();
- return false;
- }
- } else {
- SourceMgr.setMainFileID(
- SourceMgr.createFileID(File, SourceLocation(), Kind));
- }
+ SourceMgr.setMainFileID(
+ SourceMgr.createFileID(File, SourceLocation(), Kind));
} else {
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> SBOrErr =
llvm::MemoryBuffer::getSTDIN();
Index: clang/lib/Basic/SourceManager.cpp
===================================================================
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -120,8 +120,10 @@
// Clang (including elsewhere in this file!) use 'unsigned' to represent file
// offsets, line numbers, string literal lengths, and so on, and fail
// miserably on large source files.
- if ((uint64_t)ContentsEntry->getSize() >=
- std::numeric_limits<unsigned>::max()) {
+ auto diagnoseFileTooLarge = [&](uint64_t Size) {
+ if (Size < std::numeric_limits<unsigned>::max())
+ return false;
+
if (Diag.isDiagnosticInFlight())
Diag.SetDelayedDiagnostic(diag::err_file_too_large,
ContentsEntry->getName());
@@ -129,8 +131,14 @@
Diag.Report(Loc, diag::err_file_too_large)
<< ContentsEntry->getName();
+ return true;
+ };
+
+ // Check ContentsEntry's size, unless it's a named pipe (in which case we
+ // need to load the buffer first).
+ if (!ContentsEntry->isNamedPipe() &&
+ diagnoseFileTooLarge(ContentsEntry->getSize()))
return None;
- }
auto BufferOrError = FM.getBufferForFile(ContentsEntry, IsFileVolatile);
@@ -153,9 +161,14 @@
Buffer = std::move(*BufferOrError);
- // Check that the file's size is the same as in the file entry (which may
- // have come from a stat cache).
- if (Buffer->getBufferSize() != (size_t)ContentsEntry->getSize()) {
+ // If this is a named pipe, check the buffer to see if it's too big.
+ // Otherwise, check that the file's size is the same as in the file entry
+ // (which may have come from a stat cache).
+ if (ContentsEntry->isNamedPipe()) {
+ // Check the buffer size directly if this is a named pipe.
+ if (diagnoseFileTooLarge(Buffer->getBufferSize()))
+ return None;
+ } else if (Buffer->getBufferSize() != (size_t)ContentsEntry->getSize()) {
if (Diag.isDiagnosticInFlight())
Diag.SetDelayedDiagnostic(diag::err_file_modified,
ContentsEntry->getName());
@@ -531,6 +544,12 @@
assert(SourceFile && "Null source file!");
SrcMgr::ContentCache &IR =
getOrCreateContentCache(SourceFile, isSystem(FileCharacter));
+
+ // If this is a named pipe, immediately load the buffer to ensure subsequent
+ // calls to ContentCache::getSize are accurate.
+ if (IR.ContentsEntry->isNamedPipe())
+ (void)IR.getBufferOrNone(Diag, getFileManager(), SourceLocation());
+
return createFileIDImpl(IR, SourceFile->getName(), IncludePos, FileCharacter,
LoadedID, LoadedOffset);
}
@@ -541,6 +560,12 @@
int LoadedID, unsigned LoadedOffset) {
SrcMgr::ContentCache &IR = getOrCreateContentCache(&SourceFile.getFileEntry(),
isSystem(FileCharacter));
+
+ // If this is a named pipe, immediately load the buffer to ensure subsequent
+ // calls to ContentCache::getSize are accurate.
+ if (IR.ContentsEntry->isNamedPipe())
+ (void)IR.getBufferOrNone(Diag, getFileManager(), SourceLocation());
+
return createFileIDImpl(IR, SourceFile.getName(), IncludePos, FileCharacter,
LoadedID, LoadedOffset);
}
Index: clang/lib/Basic/FileManager.cpp
===================================================================
--- clang/lib/Basic/FileManager.cpp
+++ clang/lib/Basic/FileManager.cpp
@@ -489,7 +489,7 @@
uint64_t FileSize = Entry->getSize();
// If there's a high enough chance that the file have changed since we
// got its size, force a stat before opening it.
- if (isVolatile)
+ if (isVolatile || Entry->isNamedPipe())
FileSize = -1;
StringRef Filename = Entry->getName();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
