[llvm] [MCParser] .incbin: Don't retain the buffer, don't require NUL termination (PR #196696)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-support

Author: Fangrui Song (MaskRay)

<details>
<summary>Changes</summary>

processIncbinFile uses SourceMgr::AddIncludeFile, which

* sets `RequiresNullTerminator=true` and disable `mmap` when the file size is a multiple of the page size,
* and unnecessarily retains the throwaway buffer in `Buffers`.

Switch to OpenIncludeFile so the buffer is freed when processIncbinFile
returns, and pass RequiresNullTerminator=false. The buffer is consumed
only by emitBytes; the lexer never scans it, so it does not need a
trailing '\0'. Without that requirement, MemoryBuffer mmaps the file and
RSS tracks only the touched pages.

Stress test (1000 .incbin "blob.bin", 0, 16 against a 1 MiB blob):

```
                  Maximum RSS
  Before          1042944 KiB
  After             15360 KiB
```

This matches the result of #<!-- -->194254 without introducing the
SourceMgr::OpenSliceIncludeFile / vfs::File::getSliceBuffer /
FileSystem::getSliceBufferForFile API surface.

Fix #<!-- -->62339


---
Full diff: https://github.com/llvm/llvm-project/pull/196696.diff


3 Files Affected:

- (modified) llvm/include/llvm/Support/SourceMgr.h (+2-1) 
- (modified) llvm/lib/MC/MCParser/AsmParser.cpp (+7-4) 
- (modified) llvm/lib/Support/SourceMgr.cpp (+7-3) 


``````````diff
diff --git a/llvm/include/llvm/Support/SourceMgr.h b/llvm/include/llvm/Support/SourceMgr.h
index 02e694cad8697..1bfbd1e7eaae7 100644
--- a/llvm/include/llvm/Support/SourceMgr.h
+++ b/llvm/include/llvm/Support/SourceMgr.h
@@ -200,7 +200,8 @@ class SourceMgr {
   /// buffer of the stacked file. The full path to the included file can be
   /// found in \p IncludedFile.
   LLVM_ABI ErrorOr<std::unique_ptr<MemoryBuffer>>
-  OpenIncludeFile(const std::string &Filename, std::string &IncludedFile);
+  OpenIncludeFile(const std::string &Filename, std::string &IncludedFile,
+                  bool RequiresNullTerminator = true);
 
   /// Return the ID of the buffer containing the specified location.
   ///
diff --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp
index 0d517fbd57472..482abd5bbbd55 100644
--- a/llvm/lib/MC/MCParser/AsmParser.cpp
+++ b/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -860,14 +860,17 @@ bool AsmParser::processIncbinFile(const std::string &Filename, int64_t Skip,
   if (SymbolScanningMode)
     return false;
 
+  // The buffer is consumed only by emitBytes. Skip the NUL termination to
+  // enable mmap in more cases, reading only the touched pages instead of the
+  // whole file.
   std::string IncludedFile;
-  unsigned NewBuf =
-      SrcMgr.AddIncludeFile(Filename, Lexer.getLoc(), IncludedFile);
-  if (!NewBuf)
+  ErrorOr<std::unique_ptr<MemoryBuffer>> BufOrErr = SrcMgr.OpenIncludeFile(
+      Filename, IncludedFile, /*RequiresNullTerminator=*/false);
+  if (!BufOrErr)
     return true;
 
   // Pick up the bytes from the file and emit them.
-  StringRef Bytes = SrcMgr.getMemoryBuffer(NewBuf)->getBuffer();
+  StringRef Bytes = (*BufOrErr)->getBuffer();
   Bytes = Bytes.drop_front(Skip);
   if (Count) {
     int64_t Res;
diff --git a/llvm/lib/Support/SourceMgr.cpp b/llvm/lib/Support/SourceMgr.cpp
index 299615a6c8041..486537c7b0171 100644
--- a/llvm/lib/Support/SourceMgr.cpp
+++ b/llvm/lib/Support/SourceMgr.cpp
@@ -68,9 +68,13 @@ unsigned SourceMgr::AddIncludeFile(const std::string &Filename,
 
 ErrorOr<std::unique_ptr<MemoryBuffer>>
 SourceMgr::OpenIncludeFile(const std::string &Filename,
-                           std::string &IncludedFile) {
-  auto GetFile = [this](StringRef Path) {
-    return FS ? FS->getBufferForFile(Path) : MemoryBuffer::getFile(Path);
+                           std::string &IncludedFile,
+                           bool RequiresNullTerminator) {
+  auto GetFile = [this, RequiresNullTerminator](StringRef Path) {
+    return FS ? FS->getBufferForFile(Path, /*FileSize=*/-1,
+                                     RequiresNullTerminator)
+              : MemoryBuffer::getFile(Path, /*IsText=*/false,
+                                      RequiresNullTerminator);
   };
 
   ErrorOr<std::unique_ptr<MemoryBuffer>> NewBufOrErr = GetFile(Filename);

``````````

</details>


https://github.com/llvm/llvm-project/pull/196696