Fix msync() flags on FreeBSD

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;
 }

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
 #

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

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

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

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_ */

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));
+}