From fffcd98b0e6f18ce1b8ec1ad097e4e38ed880ece Mon Sep 17 00:00:00 2001 From: Justine Tunney Date: Mon, 21 Aug 2023 04:15:05 -0700 Subject: [PATCH] Fix msync() flags on FreeBSD --- libc/runtime/msync.c | 54 ++++++++++++++++++++++++++++---- libc/sysv/consts.sh | 6 ++-- libc/sysv/consts/MS_ASYNC.S | 2 +- libc/sysv/consts/MS_INVALIDATE.S | 2 +- libc/sysv/consts/MS_SYNC.S | 2 +- libc/sysv/consts/msync.h | 3 +- test/libc/runtime/msync_test.c | 41 +++++++++++++++++++++++- 7 files changed, 95 insertions(+), 15 deletions(-) diff --git a/libc/runtime/msync.c b/libc/runtime/msync.c index a5b0cc91f..a04539182 100644 --- a/libc/runtime/msync.c +++ b/libc/runtime/msync.c @@ -25,6 +25,7 @@ #include "libc/dce.h" #include "libc/intrin/strace.internal.h" #include "libc/macros.internal.h" +#include "libc/sysv/errfuns.h" /** * Synchronize memory mapping changes to disk. @@ -37,20 +38,61 @@ * @return 0 on success or -1 w/ errno * @raise ECANCELED if thread was cancelled in masked mode * @raise EINTR if we needed to block and a signal was delivered instead + * @raise EINVAL if `MS_SYNC` and `MS_ASYNC` were both specified + * @raise EINVAL if unknown `flags` were passed * @cancellationpoint */ int msync(void *addr, size_t size, int flags) { int rc; - BEGIN_CANCELLATION_POINT; - unassert(((flags & MS_SYNC) ^ (flags & MS_ASYNC)) || !(MS_SYNC && MS_ASYNC)); - if (!IsWindows()) { - rc = sys_msync(addr, size, flags); - } else { - rc = sys_msync_nt(addr, size, flags); + if ((flags & ~(MS_SYNC | MS_ASYNC | MS_INVALIDATE)) || + (flags & (MS_SYNC | MS_ASYNC)) == (MS_SYNC | MS_ASYNC)) { + rc = einval(); + goto Finished; } + // According to POSIX, either MS_SYNC or MS_ASYNC must be specified + // in flags, and indeed failure to include one of these flags will + // cause msync() to fail on some systems. However, Linux permits a + // call to msync() that specifies neither of these flags, with + // semantics that are (currently) equivalent to specifying MS_ASYNC. + // ──Quoth msync(2) of Linux Programmer's Manual + int sysflags = flags; + sysflags = flags; + if (flags & MS_ASYNC) { + sysflags = MS_ASYNC; + } else if (flags & MS_SYNC) { + sysflags = MS_SYNC; + } else { + sysflags = MS_ASYNC; + } + if (flags & MS_INVALIDATE) { + sysflags |= MS_INVALIDATE; + } + + // FreeBSD's manual says "The flags argument was both MS_ASYNC and + // MS_INVALIDATE. Only one of these flags is allowed." which makes + // following the POSIX recommendation somewhat difficult. + if (IsFreebsd()) { + if (sysflags == (MS_ASYNC | MS_INVALIDATE)) { + sysflags = MS_INVALIDATE; + } + } + + // FreeBSD specifies MS_SYNC as 0 so we shift the Cosmo constants + if (IsFreebsd()) { + sysflags >>= 1; + } + + BEGIN_CANCELLATION_POINT; + if (!IsWindows()) { + rc = sys_msync(addr, size, sysflags); + } else { + rc = sys_msync_nt(addr, size, sysflags); + } END_CANCELLATION_POINT; + +Finished: STRACE("msync(%p, %'zu, %#x) → %d% m", addr, size, flags, rc); return rc; } diff --git a/libc/sysv/consts.sh b/libc/sysv/consts.sh index 8902cb3d3..2c1b3f867 100755 --- a/libc/sysv/consts.sh +++ b/libc/sysv/consts.sh @@ -959,9 +959,9 @@ syscon pf PF_X25 9 9 0 0 0 0 0 0 # msync() flags # # group name GNU/Systemd GNU/Systemd (Aarch64) XNU's Not UNIX! MacOS (Arm64) FreeBSD OpenBSD NetBSD The New Technology Commentary -syscon ms MS_SYNC 4 4 16 16 0 2 4 4 # faked nt -syscon ms MS_ASYNC 1 1 1 1 1 1 1 1 # consensus (faked nt) -syscon ms MS_INVALIDATE 2 2 2 2 2 4 2 0 +syscon ms MS_SYNC 4 4 16 16 1 2 4 4 # faked nt; actually 0 on freebsd +syscon ms MS_ASYNC 1 1 1 1 2 1 1 1 # faked nt; actually 1 on freebsd +syscon ms MS_INVALIDATE 2 2 2 2 4 4 2 0 # actually 2 on freebsd # statfs() flags # diff --git a/libc/sysv/consts/MS_ASYNC.S b/libc/sysv/consts/MS_ASYNC.S index cf2bd6e42..0de89617b 100644 --- a/libc/sysv/consts/MS_ASYNC.S +++ b/libc/sysv/consts/MS_ASYNC.S @@ -1,2 +1,2 @@ #include "libc/sysv/consts/syscon.internal.h" -.syscon ms,MS_ASYNC,1,1,1,1,1,1,1,1 +.syscon ms,MS_ASYNC,1,1,1,1,2,1,1,1 diff --git a/libc/sysv/consts/MS_INVALIDATE.S b/libc/sysv/consts/MS_INVALIDATE.S index 623dd9982..059037dfb 100644 --- a/libc/sysv/consts/MS_INVALIDATE.S +++ b/libc/sysv/consts/MS_INVALIDATE.S @@ -1,2 +1,2 @@ #include "libc/sysv/consts/syscon.internal.h" -.syscon ms,MS_INVALIDATE,2,2,2,2,2,4,2,0 +.syscon ms,MS_INVALIDATE,2,2,2,2,4,4,2,0 diff --git a/libc/sysv/consts/MS_SYNC.S b/libc/sysv/consts/MS_SYNC.S index a17a60782..94ded8eef 100644 --- a/libc/sysv/consts/MS_SYNC.S +++ b/libc/sysv/consts/MS_SYNC.S @@ -1,2 +1,2 @@ #include "libc/sysv/consts/syscon.internal.h" -.syscon ms,MS_SYNC,4,4,16,16,0,2,4,4 +.syscon ms,MS_SYNC,4,4,16,16,1,2,4,4 diff --git a/libc/sysv/consts/msync.h b/libc/sysv/consts/msync.h index a0d69d21e..576ecfa83 100644 --- a/libc/sysv/consts/msync.h +++ b/libc/sysv/consts/msync.h @@ -7,11 +7,10 @@ extern const int MS_SYNC; extern const int MS_ASYNC; extern const int MS_INVALIDATE; -#define MS_ASYNC 1 #define MS_SYNC MS_SYNC +#define MS_ASYNC MS_ASYNC #define MS_INVALIDATE MS_INVALIDATE - COSMOPOLITAN_C_END_ #endif /* !(__ASSEMBLER__ + __LINKER__ + 0) */ #endif /* COSMOPOLITAN_LIBC_SYSV_CONSTS_MSYNC_H_ */ diff --git a/test/libc/runtime/msync_test.c b/test/libc/runtime/msync_test.c index 6fa6b022c..5d818dff9 100644 --- a/test/libc/runtime/msync_test.c +++ b/test/libc/runtime/msync_test.c @@ -16,13 +16,17 @@ │ TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR │ │ PERFORMANCE OF THIS SOFTWARE. │ ╚─────────────────────────────────────────────────────────────────────────────*/ +#include "libc/sysv/consts/msync.h" +#include "libc/atomic.h" #include "libc/calls/calls.h" +#include "libc/intrin/kprintf.h" #include "libc/runtime/runtime.h" #include "libc/sysv/consts/map.h" -#include "libc/sysv/consts/msync.h" #include "libc/sysv/consts/prot.h" #include "libc/testlib/testlib.h" +// msync() is a difficult function to test because it doesn't do much + TEST(msync, changeFileMappingAndExit) { int ws; char byte; @@ -33,6 +37,7 @@ TEST(msync, changeFileMappingAndExit) { (map = mmap(0, 4, PROT_READ | PROT_WRITE, MAP_SHARED, 3, 0))); if (!fork()) { map[0] = 1; + // commenting this out will fail on openbsd ASSERT_SYS(0, 0, msync(map, 4, MS_SYNC)); _Exit(123); } @@ -45,3 +50,37 @@ TEST(msync, changeFileMappingAndExit) { ASSERT_SYS(0, 0, munmap(map, 4)); ASSERT_SYS(0, 0, close(3)); } + +TEST(msync, changeFileMappingAndWakeSpinLockWaiter) { + int ws; + char byte; + char *map; + atomic_char *sem = _mapshared(1); + ASSERT_SYS(0, 3, tmpfd()); + ASSERT_SYS(0, 0, ftruncate(3, 1)); + ASSERT_NE(MAP_FAILED, + (map = mmap(0, 1, PROT_READ | PROT_WRITE, MAP_SHARED, 3, 0))); + if (!fork()) { + // wait for other process to enter spin sem + while (*sem == 0) donothing; + // change the file mapping + map[0] = 1; + // openbsd fails with this line commented out + ASSERT_SYS(0, 0, msync(map, 1, MS_ASYNC)); + // immediately wake other process + *sem = 2; + _Exit(123); + } + *sem = 1; + while (*sem == 1) donothing; + ASSERT_EQ(1, map[0]); + ASSERT_SYS(0, 1, pread(3, &byte, 1, 0)); + ASSERT_EQ(1, byte); + ASSERT_EQ(1, map[0]); + ASSERT_NE(-1, wait(&ws)); + ASSERT_TRUE(WIFEXITED(ws)); + ASSERT_EQ(123, WEXITSTATUS(ws)); + ASSERT_SYS(0, 0, munmap(map, 1)); + ASSERT_SYS(0, 0, close(3)); + ASSERT_SYS(0, 0, munmap(sem, 1)); +}