From fabf7f9f0200f666480b11a20baad17731b2c0bf Mon Sep 17 00:00:00 2001 From: Justine Tunney Date: Sat, 1 May 2021 17:13:48 -0700 Subject: [PATCH] Fix popen_test in MODE=dbg ASAN and vfork() don't appear to play well together. Maybe in later versions of GCC it'll be better. But vfork() is flirting with danger after all and that probably doesn't make sense in ASAN mode anyway. --- libc/runtime/vfork.S | 7 +++++++ libc/stdio/fgetc.c | 6 +++--- libc/stdio/popen.c | 33 ++++++++++++++++++++++++--------- libc/stdio/stdio.h | 2 +- test/libc/stdio/popen_test.c | 1 - 5 files changed, 35 insertions(+), 14 deletions(-) diff --git a/libc/runtime/vfork.S b/libc/runtime/vfork.S index 8af42732..197d6f76 100644 --- a/libc/runtime/vfork.S +++ b/libc/runtime/vfork.S @@ -20,6 +20,11 @@ #include "libc/macros.internal.h" .privileged +#ifdef __FSANITIZE_ADDRESS__ +vfork: jmp fork # TODO: asan and vfork don't mix? + .endfn vfork,globl +#else + // Forks process without copying page tables. // // This is the same as fork() except it's optimized for the case @@ -75,3 +80,5 @@ vfork.bsd: jmp 0b .endfn vfork.bsd #endif /* BSD */ + +#endif /* __FSANITIZE_ADDRESS__ */ diff --git a/libc/stdio/fgetc.c b/libc/stdio/fgetc.c index 034b9678..731e4b3e 100644 --- a/libc/stdio/fgetc.c +++ b/libc/stdio/fgetc.c @@ -23,11 +23,11 @@ * @return byte in range 0..255, or -1 w/ errno */ int fgetc(FILE *f) { - unsigned char b; + unsigned char b[1]; if (f->beg < f->end) { return f->buf[f->beg++] & 0xff; } else { - if (!fread(&b, 1, 1, f)) return -1; - return b; + if (!fread(b, 1, 1, f)) return -1; + return b[0]; } } diff --git a/libc/stdio/popen.c b/libc/stdio/popen.c index 50abc39c..d99bba67 100644 --- a/libc/stdio/popen.c +++ b/libc/stdio/popen.c @@ -33,7 +33,7 @@ */ FILE *popen(const char *cmdline, const char *mode) { FILE *f; - int dir, flags, pipefds[2]; + int e, pid, dir, flags, pipefds[2]; flags = fopenflags(mode); if ((flags & O_ACCMODE) == O_RDONLY) { dir = 0; @@ -45,13 +45,28 @@ FILE *popen(const char *cmdline, const char *mode) { } if (pipe(pipefds) == -1) return NULL; fcntl(pipefds[dir], F_SETFD, FD_CLOEXEC); - if (!(f = fdopen(pipefds[dir], mode))) abort(); - if ((f->pid = vfork()) == -1) abort(); - if (!f->pid) { - dup2(pipefds[!dir], !dir); - systemexec(cmdline); - _exit(127); + if ((f = fdopen(pipefds[dir], mode))) { + switch ((pid = vfork())) { + case 0: + dup2(pipefds[!dir], !dir); + systemexec(cmdline); + _exit(127); + default: + f->pid = pid; + close(pipefds[!dir]); + return f; + case -1: + e = errno; + fclose(f); + close(pipefds[!dir]); + errno = e; + return NULL; + } + } else { + e = errno; + close(pipefds[0]); + close(pipefds[1]); + errno = e; + return NULL; } - close(pipefds[!dir]); - return f; } diff --git a/libc/stdio/stdio.h b/libc/stdio/stdio.h index 268913be..7d3d0649 100644 --- a/libc/stdio/stdio.h +++ b/libc/stdio/stdio.h @@ -14,7 +14,7 @@ COSMOPOLITAN_C_START_ typedef struct FILE { uint8_t bufmode; /* 0x00 _IOFBF, etc. (ignored if fd=-1) */ - bool noclose; /* 0x01 for fake dup() */ + bool noclose; /* 0x01 for fake dup() todo delete! */ uint32_t iomode; /* 0x04 O_RDONLY, etc. (ignored if fd=-1) */ int32_t state; /* 0x08 0=OK, -1=EOF, >0=errno */ int fd; /* 0x0c ≥0=fd, -1=closed|buffer */ diff --git a/test/libc/stdio/popen_test.c b/test/libc/stdio/popen_test.c index 752b1131..c7d37b8e 100644 --- a/test/libc/stdio/popen_test.c +++ b/test/libc/stdio/popen_test.c @@ -23,7 +23,6 @@ TEST(popen, test) { int ws; FILE *f; - /* TODO(jart): look into popen() asan error */ f = popen("echo hi", "r"); ASSERT_NE(NULL, f); EXPECT_EQ('h', fgetc(f));