Skip to content

Commit 245218b

Browse files
committed
Basic: Support named pipes natively in SourceManager and FileManager
Handle named pipes natively in SourceManager and FileManager, removing a call to `SourceManager::overrideFileContents` in `CompilerInstance::InitializeSourceManager` (removing a blocker for sinking the content cache to FileManager (which will incidently sink this new named pipe logic with it)). SourceManager usually checks if the file entry's size matches the eventually loaded buffer, but that's now skipped for named pipes since the `stat` won't reflect the full size. 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`. `FileManager::getBufferForFile` also needs to treat these files as volatile when loading the buffer. Native support in SourceManager / FileManager means that named pipes can also be `#include`d, and clang/test/Misc/dev-fd-fs.c was expanded to check for that. This is a new version of 3b18a59, which was reverted in b346322 since it was missing the `SourceManager` changes. Differential Revision: https://reviews.llvm.org/D92531
1 parent 747f67e commit 245218b

File tree

4 files changed

+47
-46
lines changed

4 files changed

+47
-46
lines changed

clang/lib/Basic/FileManager.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,7 @@ FileManager::getBufferForFile(const FileEntry *Entry, bool isVolatile,
489489
uint64_t FileSize = Entry->getSize();
490490
// If there's a high enough chance that the file have changed since we
491491
// got its size, force a stat before opening it.
492-
if (isVolatile)
492+
if (isVolatile || Entry->isNamedPipe())
493493
FileSize = -1;
494494

495495
StringRef Filename = Entry->getName();

clang/lib/Basic/SourceManager.cpp

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -115,23 +115,6 @@ ContentCache::getBufferOrNone(DiagnosticsEngine &Diag, FileManager &FM,
115115
// return paths.
116116
IsBufferInvalid = true;
117117

118-
// Check that the file's size fits in an 'unsigned' (with room for a
119-
// past-the-end value). This is deeply regrettable, but various parts of
120-
// Clang (including elsewhere in this file!) use 'unsigned' to represent file
121-
// offsets, line numbers, string literal lengths, and so on, and fail
122-
// miserably on large source files.
123-
if ((uint64_t)ContentsEntry->getSize() >=
124-
std::numeric_limits<unsigned>::max()) {
125-
if (Diag.isDiagnosticInFlight())
126-
Diag.SetDelayedDiagnostic(diag::err_file_too_large,
127-
ContentsEntry->getName());
128-
else
129-
Diag.Report(Loc, diag::err_file_too_large)
130-
<< ContentsEntry->getName();
131-
132-
return None;
133-
}
134-
135118
auto BufferOrError = FM.getBufferForFile(ContentsEntry, IsFileVolatile);
136119

137120
// If we were unable to open the file, then we are in an inconsistent
@@ -153,9 +136,31 @@ ContentCache::getBufferOrNone(DiagnosticsEngine &Diag, FileManager &FM,
153136

154137
Buffer = std::move(*BufferOrError);
155138

156-
// Check that the file's size is the same as in the file entry (which may
139+
// Check that the file's size fits in an 'unsigned' (with room for a
140+
// past-the-end value). This is deeply regrettable, but various parts of
141+
// Clang (including elsewhere in this file!) use 'unsigned' to represent file
142+
// offsets, line numbers, string literal lengths, and so on, and fail
143+
// miserably on large source files.
144+
//
145+
// Note: ContentsEntry could be a named pipe, in which case
146+
// ContentsEntry::getSize() could have the wrong size. Use
147+
// MemoryBuffer::getBufferSize() instead.
148+
if (Buffer->getBufferSize() >= std::numeric_limits<unsigned>::max()) {
149+
if (Diag.isDiagnosticInFlight())
150+
Diag.SetDelayedDiagnostic(diag::err_file_too_large,
151+
ContentsEntry->getName());
152+
else
153+
Diag.Report(Loc, diag::err_file_too_large)
154+
<< ContentsEntry->getName();
155+
156+
return None;
157+
}
158+
159+
// Unless this is a named pipe (in which case we can handle a mismatch),
160+
// check that the file's size is the same as in the file entry (which may
157161
// have come from a stat cache).
158-
if (Buffer->getBufferSize() != (size_t)ContentsEntry->getSize()) {
162+
if (!ContentsEntry->isNamedPipe() &&
163+
Buffer->getBufferSize() != (size_t)ContentsEntry->getSize()) {
159164
if (Diag.isDiagnosticInFlight())
160165
Diag.SetDelayedDiagnostic(diag::err_file_modified,
161166
ContentsEntry->getName());
@@ -541,6 +546,12 @@ FileID SourceManager::createFileID(FileEntryRef SourceFile,
541546
int LoadedID, unsigned LoadedOffset) {
542547
SrcMgr::ContentCache &IR = getOrCreateContentCache(&SourceFile.getFileEntry(),
543548
isSystem(FileCharacter));
549+
550+
// If this is a named pipe, immediately load the buffer to ensure subsequent
551+
// calls to ContentCache::getSize() are accurate.
552+
if (IR.ContentsEntry->isNamedPipe())
553+
(void)IR.getBufferOrNone(Diag, getFileManager(), SourceLocation());
554+
544555
return createFileIDImpl(IR, SourceFile.getName(), IncludePos, FileCharacter,
545556
LoadedID, LoadedOffset);
546557
}

clang/lib/Frontend/CompilerInstance.cpp

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -856,32 +856,9 @@ bool CompilerInstance::InitializeSourceManager(const FrontendInputFile &Input,
856856
Diags.Report(diag::err_fe_error_reading) << InputFile;
857857
return false;
858858
}
859-
FileEntryRef File = *FileOrErr;
860-
861-
// The natural SourceManager infrastructure can't currently handle named
862-
// pipes, but we would at least like to accept them for the main
863-
// file. Detect them here, read them with the volatile flag so FileMgr will
864-
// pick up the correct size, and simply override their contents as we do for
865-
// STDIN.
866-
if (File.getFileEntry().isNamedPipe()) {
867-
auto MB =
868-
FileMgr.getBufferForFile(&File.getFileEntry(), /*isVolatile=*/true);
869-
if (MB) {
870-
// Create a new virtual file that will have the correct size.
871-
FileEntryRef FE =
872-
FileMgr.getVirtualFileRef(InputFile, (*MB)->getBufferSize(), 0);
873-
SourceMgr.overrideFileContents(FE, std::move(*MB));
874-
SourceMgr.setMainFileID(
875-
SourceMgr.createFileID(FE, SourceLocation(), Kind));
876-
} else {
877-
Diags.Report(diag::err_cannot_open_file) << InputFile
878-
<< MB.getError().message();
879-
return false;
880-
}
881-
} else {
882-
SourceMgr.setMainFileID(
883-
SourceMgr.createFileID(File, SourceLocation(), Kind));
884-
}
859+
860+
SourceMgr.setMainFileID(
861+
SourceMgr.createFileID(*FileOrErr, SourceLocation(), Kind));
885862
} else {
886863
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> SBOrErr =
887864
llvm::MemoryBuffer::getSTDIN();

clang/test/Misc/dev-fd-fs.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,13 @@
88
// RUN: cat %s | %clang -x c /dev/fd/0 -E > %t
99
// RUN: FileCheck --check-prefix DEV-FD-INPUT < %t %s
1010
//
11+
// RUN: cat %s | %clang -x c %s -E -DINCLUDE_FROM_STDIN > %t
12+
// RUN: FileCheck --check-prefix DEV-FD-INPUT \
13+
// RUN: --check-prefix DEV-FD-INPUT-INCLUDE < %t %s
14+
//
15+
// DEV-FD-INPUT-INCLUDE: int w;
1116
// DEV-FD-INPUT: int x;
17+
// DEV-FD-INPUT-INCLUDE: int y;
1218

1319

1420
// Check writing to /dev/fd named pipes. We use cat here as before to ensure we
@@ -27,4 +33,11 @@
2733
//
2834
// DEV-FD-REG-OUTPUT: int x;
2935

36+
#ifdef INCLUDE_FROM_STDIN
37+
#undef INCLUDE_FROM_STDIN
38+
int w;
39+
#include "/dev/fd/0"
40+
int y;
41+
#else
3042
int x;
43+
#endif

0 commit comments

Comments
 (0)