From 4a734070cd2838e49658464003de5b92271d8b9e Mon Sep 17 00:00:00 2001
From: Gabriella Gonzalez <gabriella@mercury.com>
Date: Mon, 30 Jan 2023 16:54:26 -0800
Subject: [PATCH] Fix `ld`'s response files support for special files
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Currently `ld` uses `mmap` to read in the contents of the
response file (presumably for performance reasons), but `mmap`
sometimes doesn't work on special files like pipes.  For example,
if you do something like:

```
$ ld @<(echo --help)
```

… that will currently fail with a segmentation fault.

You might wonder why one would want to generate a response file
from process substitution.  The rationale behind this is that
I'm currently in the process of fixing a longstanding issue
with the linker failing in Nixpkgs on macOS due to hitting
command-line length limits and the fix entails the use of
process substitution to generate the process file.

Specifically, what I was doing was building upon the work from
this PR:

https://github.com/NixOS/nixpkgs/pull/112449

… which modified the `cc-wrapper` in Nixpkgs to use a
response file generate from process substitution.  I was
going to do essentially the same for the `ld-wrapper` in
Nixpkgs, but that failed with a segmentation fault (for the
reasons outlined above).

There are other possible ways to work around that, but using
process substitution is the "leanest" way of generating the
response file for `ld` in Nixpkgs, so I wanted to push on
getting that working here instead of working around the problem
downstream.

So the way I fixed it was to fall back to using `read` instead
of `mmap` if the `mmap` failed.  After this change, the above
sample command now works correctly.

This also fixes another small issue along the way: this now
correctly detects when the `mmap` fails.  Previously, the `mmap`
logic was detecting failure by looking for a `NULL`/`0` return
value, but that is not the correct error-handling behavior.
`mmap` returns `MAP_FAILED` on failure, which is `-1` in practice,
and not `0`.  That's the reason why the code was failing with
a segmentation fault before because it wasn't detecting the failure
and proceeding to read from the invalid buffer anyway.
---
 cctools/ld64/src/ld/ResponseFiles.cpp | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/cctools/ld64/src/ld/ResponseFiles.cpp b/cctools/ld64/src/ld/ResponseFiles.cpp
index d274797a..2b536b6e 100644
--- a/cctools/ld64/src/ld/ResponseFiles.cpp
+++ b/cctools/ld64/src/ld/ResponseFiles.cpp
@@ -209,18 +209,25 @@ struct string_list* at_paths, int *hint_p)
       }
 
       char* addr = NULL;
+      size_t size = sb.st_size;
+      bool used_mmap = true;
       if (sb.st_size) {
 	addr = (char*)mmap(0, (size_t)sb.st_size, PROT_READ | PROT_WRITE,
 		    MAP_FILE | MAP_PRIVATE, fd, 0);
-	if (!addr) {
-	  close(fd);
-	  throwf("can't mmap %s: %s\n", at_path, strerror(errno));
-	  return EXPAND_ERROR;
+	if (MAP_FAILED == addr) {
+	  used_mmap = false;
+	  addr = (char *)malloc(size + 1);
+	  if (-1 == read(fd, addr, size)) {
+	    close(fd);
+	    throwf("can't read %s: %s\n", at_path, strerror(errno));
+	    return EXPAND_ERROR;
+	  }
+	  addr[size] = '\0';
 	}
       }
 
       if (close(fd)) {
-	if (munmap(addr, (size_t)sb.st_size))
+	if (used_mmap && munmap(addr, (size_t)sb.st_size))
 	  throwf("can't munmap %s: %s\n", at_path, strerror(errno));
 	throwf("can't close %s: %s\n", at_path, strerror(errno));
 	return EXPAND_ERROR;
@@ -245,7 +252,7 @@ struct string_list* at_paths, int *hint_p)
 
       // unmap the file
       if (addr) {
-	if (munmap(addr, (size_t)sb.st_size)) {
+	if (used_mmap && munmap(addr, (size_t)sb.st_size)) {
 	  throwf("can't munmap %s: %s\n", at_path,
 		  strerror(errno));
 	  return EXPAND_ERROR;