[llvm-branch-commits] [clang] 245218b - Basic: Support named pipes natively in SourceManager and FileManager

Duncan P. N. Exon Smith via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Wed Dec 23 15:02:26 PST 2020


Author: Duncan P. N. Exon Smith
Date: 2020-12-23T14:57:41-08:00
New Revision: 245218bb355599771ba43a0fe1449d1670f2666c

URL: https://github.com/llvm/llvm-project/commit/245218bb355599771ba43a0fe1449d1670f2666c
DIFF: https://github.com/llvm/llvm-project/commit/245218bb355599771ba43a0fe1449d1670f2666c.diff

LOG: 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 3b18a594c7717a328c33b9c1eba675e9f4bd367c, which
was reverted in b34632201987eed369bb7ef4646f341b901c95b8 since it was
missing the `SourceManager` changes.

Differential Revision: https://reviews.llvm.org/D92531

Added: 
    

Modified: 
    clang/lib/Basic/FileManager.cpp
    clang/lib/Basic/SourceManager.cpp
    clang/lib/Frontend/CompilerInstance.cpp
    clang/test/Misc/dev-fd-fs.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp
index c0d3685001ee..f3afe6dd5f48 100644
--- a/clang/lib/Basic/FileManager.cpp
+++ b/clang/lib/Basic/FileManager.cpp
@@ -489,7 +489,7 @@ FileManager::getBufferForFile(const FileEntry *Entry, bool isVolatile,
   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();

diff  --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index b71b2be0cc41..d2c0de5006c4 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -115,23 +115,6 @@ ContentCache::getBufferOrNone(DiagnosticsEngine &Diag, FileManager &FM,
   // return paths.
   IsBufferInvalid = true;
 
-  // Check that the file's size fits in an 'unsigned' (with room for a
-  // past-the-end value). This is deeply regrettable, but various parts of
-  // 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()) {
-    if (Diag.isDiagnosticInFlight())
-      Diag.SetDelayedDiagnostic(diag::err_file_too_large,
-                                ContentsEntry->getName());
-    else
-      Diag.Report(Loc, diag::err_file_too_large)
-        << ContentsEntry->getName();
-
-    return None;
-  }
-
   auto BufferOrError = FM.getBufferForFile(ContentsEntry, IsFileVolatile);
 
   // If we were unable to open the file, then we are in an inconsistent
@@ -153,9 +136,31 @@ ContentCache::getBufferOrNone(DiagnosticsEngine &Diag, FileManager &FM,
 
   Buffer = std::move(*BufferOrError);
 
-  // Check that the file's size is the same as in the file entry (which may
+  // Check that the file's size fits in an 'unsigned' (with room for a
+  // past-the-end value). This is deeply regrettable, but various parts of
+  // 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.
+  //
+  // Note: ContentsEntry could be a named pipe, in which case
+  // ContentsEntry::getSize() could have the wrong size. Use
+  // MemoryBuffer::getBufferSize() instead.
+  if (Buffer->getBufferSize() >= std::numeric_limits<unsigned>::max()) {
+    if (Diag.isDiagnosticInFlight())
+      Diag.SetDelayedDiagnostic(diag::err_file_too_large,
+                                ContentsEntry->getName());
+    else
+      Diag.Report(Loc, diag::err_file_too_large)
+        << ContentsEntry->getName();
+
+    return None;
+  }
+
+  // Unless this is a named pipe (in which case we can handle a mismatch),
+  // 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 (!ContentsEntry->isNamedPipe() &&
+      Buffer->getBufferSize() != (size_t)ContentsEntry->getSize()) {
     if (Diag.isDiagnosticInFlight())
       Diag.SetDelayedDiagnostic(diag::err_file_modified,
                                 ContentsEntry->getName());
@@ -541,6 +546,12 @@ FileID SourceManager::createFileID(FileEntryRef SourceFile,
                                    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);
 }

diff  --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 69e2e554d018..d96afae44296 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -856,32 +856,9 @@ bool CompilerInstance::InitializeSourceManager(const FrontendInputFile &Input,
       Diags.Report(diag::err_fe_error_reading) << InputFile;
       return false;
     }
-    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.
-        FileEntryRef FE =
-            FileMgr.getVirtualFileRef(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(*FileOrErr, SourceLocation(), Kind));
   } else {
     llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> SBOrErr =
         llvm::MemoryBuffer::getSTDIN();

diff  --git a/clang/test/Misc/dev-fd-fs.c b/clang/test/Misc/dev-fd-fs.c
index f04b059a6436..ea94d950b071 100644
--- a/clang/test/Misc/dev-fd-fs.c
+++ b/clang/test/Misc/dev-fd-fs.c
@@ -8,7 +8,13 @@
 // RUN: cat %s | %clang -x c /dev/fd/0 -E > %t
 // RUN: FileCheck --check-prefix DEV-FD-INPUT < %t %s
 //
+// RUN: cat %s | %clang -x c %s -E -DINCLUDE_FROM_STDIN > %t
+// RUN: FileCheck --check-prefix DEV-FD-INPUT \
+// RUN:           --check-prefix DEV-FD-INPUT-INCLUDE < %t %s
+//
+// DEV-FD-INPUT-INCLUDE: int w;
 // DEV-FD-INPUT: int x;
+// DEV-FD-INPUT-INCLUDE: int y;
 
 
 // Check writing to /dev/fd named pipes. We use cat here as before to ensure we
@@ -27,4 +33,11 @@
 //
 // DEV-FD-REG-OUTPUT: int x;
 
+#ifdef INCLUDE_FROM_STDIN
+#undef INCLUDE_FROM_STDIN
+int w;
+#include "/dev/fd/0"
+int y;
+#else
 int x;
+#endif


        


More information about the llvm-branch-commits mailing list