Use mremap to extend the output file mapping

Previously, data appended to the output file, such as .gdb_index,
was staged in a heap buffer or a separate memory mapping. We can
instead just grow the existing mapping with mremap(MREMAP_MAYMOVE).
Consumers of the output buffer derive pointers from ctx.buf, so the
mapping is free to move, and since filesize now includes appended
data, the build ID and the .gnu_debuglink CRC naturally cover the
entire file.

Co-Authored-By: Claude Fable 5 <[email protected]>

src/gdb-index.cc

@@ -635,7 +635,7 @@ std::span<u8> get_buffer(Context<E> &ctx, Chunk<E> *chunk) {
 }
 
 template <typename E>
-std::span<u8> write_gdb_index(Context<E> &ctx) {
+void write_gdb_index(Context<E> &ctx) {
   Timer t(ctx, "write_gdb_index");
 
   // Find debug info sections
@@ -654,7 +654,7 @@ std::span<u8> write_gdb_index(Context<E> &ctx) {
   }
 
   if (ctx.debug_info.empty())
-    return {};
+    return;
 
   // Read debug info
   std::vector<Compunit> cus = read_compunits(ctx);
@@ -797,12 +797,10 @@ std::span<u8> write_gdb_index(Context<E> &ctx) {
     ctx.gdb_index->shdr.sh_size = bufsize;
     ctx.shdr->copy_buf(ctx);
   }
-
-  return {buf, (size_t)bufsize};
 }
 
 using E = MOLD_TARGET;
 
-template std::span<u8> write_gdb_index(Context<E> &);
+template void write_gdb_index(Context<E> &);
 
 } // namespace mold

src/mold.h

@@ -1608,18 +1608,14 @@ public:
   open(Context<E> &ctx, std::string path, i64 filesize, int perm);
 
   virtual void close(Context<E> &ctx) = 0;
-  virtual ~OutputFile() { free(trailer); }
+  virtual ~OutputFile() = default;
 
-  // Returns a buffer for `size` bytes of additional data to be
-  // appended to the end of the output file. We use it to append
-  // .gdb_index, whose size is not known until all other sections
-  // have been written. By default, appended data is buffered on
-  // the heap and written out on close().
-  virtual u8 *extend(Context<E> &ctx, i64 size) {
-    trailer = (u8 *)realloc(trailer, trailer_size + size);
-    trailer_size += size;
-    return trailer + trailer_size - size;
-  }
+  // Appends `size` bytes to the output file and returns a pointer to
+  // the newly-allocated space, bumping `filesize` accordingly. We use
+  // it for .gdb_index, whose size is not known until all other
+  // sections have been written. The new space is zero-initialized.
+  // `buf` and `ctx.buf` may move as a result of this call.
+  virtual u8 *extend(Context<E> &ctx, i64 size) = 0;
 
   u8 *buf = nullptr;
   std::string path;
@@ -1631,18 +1627,31 @@ public:
 protected:
   OutputFile(std::string path, i64 filesize, bool is_mmapped)
     : path(path), filesize(filesize), is_mmapped(is_mmapped) {}
-
-  u8 *trailer = nullptr;
-  i64 trailer_size = 0;
 };
 
 template <typename E>
 class MallocOutputFile : public OutputFile<E> {
 public:
   MallocOutputFile(Context<E> &ctx, std::string path, i64 filesize, int perm)
-    : OutputFile<E>(path, filesize, false), ptr(new u8[filesize]),
-      perm(perm) {
-    this->buf = ptr.get();
+    : OutputFile<E>(path, filesize, false), perm(perm) {
+    this->buf = (u8 *)calloc(filesize, 1);
+    if (!this->buf)
+      Fatal(ctx) << "calloc failed: " << errno_string();
+  }
+
+  ~MallocOutputFile() { free(this->buf); }
+
+  u8 *extend(Context<E> &ctx, i64 size) override {
+    this->buf = (u8 *)realloc(this->buf, this->filesize + size);
+    if (!this->buf)
+      Fatal(ctx) << "realloc failed: " << errno_string();
+
+    u8 *space = this->buf + this->filesize;
+    memset(space, 0, size);
+
+    this->filesize += size;
+    ctx.buf = this->buf;
+    return space;
   }
 
   void close(Context<E> &ctx) override {
@@ -1668,13 +1677,10 @@ public:
     }
 
     fwrite(this->buf, this->filesize, 1, fp);
-    if (this->trailer)
-      fwrite(this->trailer, this->trailer_size, 1, fp);
     fclose(fp);
   }
 
 private:
-  std::unique_ptr<u8[]> ptr;
   int perm;
 };
 
@@ -1683,6 +1689,7 @@ class LockingOutputFile : public OutputFile<E> {
 public:
   LockingOutputFile(Context<E> &ctx, std::string path, int perm);
   void resize(Context<E> &ctx, i64 filesize);
+  u8 *extend(Context<E> &ctx, i64 size) override;
   void close(Context<E> &ctx) override;
 };
 
@@ -1690,7 +1697,7 @@ public:
 // gdb-index.cc
 //
 
-template <typename E> std::span<u8> write_gdb_index(Context<E> &ctx);
+template <typename E> void write_gdb_index(Context<E> &ctx);
 
 //
 // input-files.cc

src/output-file-unix.cc

@@ -84,32 +84,38 @@ public:
       ::close(fd2);
   }
 
-  // Extend the file and memory-map the new range so that the caller
-  // can fill the appended data in place. The new range gets its own
-  // mapping because growing the first one with mremap could move it,
-  // invalidating pointers into it that our caller holds.
+  // Extend the file and grow its memory mapping so that the caller
+  // can fill the appended data in place. mremap may move the
+  // mapping; we update ctx.buf, and the output buffer is normally
+  // accessed as ctx.buf plus an offset, but a caller that holds raw
+  // pointers into the buffer across this call must rebase them.
   u8 *extend(Context<E> &ctx, i64 size) override {
-    i64 offset = align_down(this->filesize, sysconf(_SC_PAGESIZE));
+    i64 mapsize = this->filesize;
 
-    if (ftruncate(this->fd, this->filesize + size) == -1)
+    if (ftruncate(this->fd, mapsize + size) == -1)
       Fatal(ctx) << "ftruncate failed: " << errno_string();
 
 #if HAVE_FALLOCATE
     if (struct statfs fs;
         fstatfs(this->fd, &fs) || fs.f_type != TMPFS_MAGIC)
-      fallocate(this->fd, 0, this->filesize, size);
+      fallocate(this->fd, 0, mapsize, size);
 #endif
 
-    extension_size = this->filesize + size - offset;
-    extension = (u8 *)mmap(nullptr, extension_size, PROT_READ | PROT_WRITE,
-                           MAP_SHARED, this->fd, offset);
-    if (extension == MAP_FAILED)
-      Fatal(ctx) << this->path << ": mmap failed: " << errno_string();
+    u8 *buf =
+      (u8 *)mremap(this->buf, mapsize, mapsize + size, MREMAP_MAYMOVE);
+    if (buf == MAP_FAILED)
+      Fatal(ctx) << this->path << ": mremap failed: " << errno_string();
+
+    this->filesize += size;
+    this->buf = buf;
+    ctx.buf = buf;
+    mold::output_buffer_start = buf;
+    mold::output_buffer_end = buf + mapsize + size;
 
 #ifdef MADV_HUGEPAGE
-    madvise(extension, extension_size, MADV_HUGEPAGE);
+    madvise(buf + mapsize, size, MADV_HUGEPAGE);
 #endif
-    return extension + (this->filesize - offset);
+    return buf + mapsize;
   }
 
   void close(Context<E> &ctx) override {
@@ -117,8 +123,6 @@ public:
 
     if (!this->is_unmapped)
       munmap(this->buf, this->filesize);
-    if (extension)
-      munmap(extension, extension_size);
     ::close(this->fd);
 
     // If an output file already exists, open a file and then remove it.
@@ -135,8 +139,6 @@ public:
 
 private:
   int fd2 = -1;
-  u8 *extension = nullptr;
-  i64 extension_size = 0;
 };
 
 template <typename E>
@@ -201,18 +203,30 @@ void LockingOutputFile<E>::resize(Context<E> &ctx, i64 filesize) {
   mold::output_buffer_end = this->buf + filesize;
 }
 
+template <typename E>
+u8 *LockingOutputFile<E>::extend(Context<E> &ctx, i64 size) {
+  i64 mapsize = this->filesize;
+
+  if (ftruncate(this->fd, mapsize + size) == -1)
+    Fatal(ctx) << "ftruncate failed: " << errno_string();
+
+  u8 *buf =
+    (u8 *)mremap(this->buf, mapsize, mapsize + size, MREMAP_MAYMOVE);
+  if (buf == MAP_FAILED)
+    Fatal(ctx) << this->path << ": mremap failed: " << errno_string();
+
+  this->filesize += size;
+  this->buf = buf;
+  ctx.buf = buf;
+  mold::output_buffer_start = buf;
+  mold::output_buffer_end = buf + mapsize + size;
+  return buf + mapsize;
+}
+
 template <typename E>
 void LockingOutputFile<E>::close(Context<E> &ctx) {
   if (!this->is_unmapped)
     munmap(this->buf, this->filesize);
-
-  if (this->trailer) {
-    FILE *out = fdopen(this->fd, "w");
-    fseek(out, 0, SEEK_END);
-    fwrite(this->trailer, this->trailer_size, 1, out);
-    fclose(out);
-  }
-
   ::close(this->fd);
 }
 

src/output-file-win32.cc

@@ -39,22 +39,36 @@ public:
       CloseHandle(handle);
   }
 
+  u8 *extend(Context<E> &ctx, i64 size) override {
+    UnmapViewOfFile(this->buf);
+
+    HANDLE map = CreateFileMapping(handle, nullptr, PAGE_READWRITE, 0,
+                                   this->filesize + size, nullptr);
+    if (!map)
+      Fatal(ctx) << this->path << ": CreateFileMapping failed: "
+                 << GetLastError();
+
+    this->buf = (u8 *)MapViewOfFile(map, FILE_MAP_WRITE, 0, 0,
+                                    this->filesize + size);
+    if (!this->buf)
+      Fatal(ctx) << this->path << ": MapViewOfFile failed: " << GetLastError();
+
+    CloseHandle(map);
+
+    u8 *space = this->buf + this->filesize;
+    memset(space, 0, size);
+
+    this->filesize += size;
+    ctx.buf = this->buf;
+    mold::output_buffer_start = this->buf;
+    mold::output_buffer_end = this->buf + this->filesize;
+    return space;
+  }
+
   void close(Context<E> &ctx) override {
     Timer t(ctx, "close_file");
 
     UnmapViewOfFile(this->buf);
-
-    if (this->trailer) {
-      if (SetFilePointer(handle, 0, nullptr, FILE_END) == INVALID_SET_FILE_POINTER)
-        Fatal(ctx) << this->path << ": SetFilePointer failed: "
-                   << GetLastError();
-
-      DWORD written;
-      if (!WriteFile(handle, this->trailer, this->trailer_size, &written,
-                     nullptr))
-        Fatal(ctx) << this->path << ": WriteFile failed: " << GetLastError();
-    }
-
     CloseHandle(handle);
     handle = INVALID_HANDLE_VALUE;
   }
@@ -106,6 +120,11 @@ LockingOutputFile<E>::LockingOutputFile(Context<E> &ctx, std::string path,
 template <typename E>
 void LockingOutputFile<E>::resize(Context<E> &ctx, i64 filesize) {}
 
+template <typename E>
+u8 *LockingOutputFile<E>::extend(Context<E> &ctx, i64 size) {
+  return nullptr;
+}
+
 template <typename E>
 void LockingOutputFile<E>::close(Context<E> &ctx) {}
 

src/passes.cc

@@ -3527,16 +3527,13 @@ void write_separate_debug_file(Context<E> &ctx) {
 
   copy_chunks(ctx);
 
-  std::span<u8> gdb_index;
   if (ctx.gdb_index)
-    gdb_index = write_gdb_index(ctx);
+    write_gdb_index(ctx);
 
   // Reverse-compute a CRC32 value so that the CRC32 checksum embedded to
   // the .gnu_debuglink section in the main executable matches with the
   // debug info file's CRC32 checksum.
   u32 crc = compute_crc32(0, ctx.buf, ctx.output_file->filesize);
-  if (!gdb_index.empty())
-    crc = compute_crc32(crc, gdb_index.data(), gdb_index.size());
 
   std::vector<u8> trailer = crc32_solve(crc, ctx.gnu_debuglink->crc32);
   memcpy(ctx.output_file->extend(ctx, trailer.size()),