Introduce system call page table entry locking

mmap() and munmap() will now block if asked to unmap a page that's being used by a system call. This way there's no fear that calling free() will cause a read() in another thread to segfault, particularly if strace was enabled. Eliminating that weakness took some revamping and revealed many other issues that have also been addressed by this change. One particularly nasty longstanding issue, is rt_sigreturn would clobber 128 bytes of struct Machine memory. Overusing intptr_t was a mistake due to how it would sign extend unsigned addresses on 32-bit platforms. This solves the memstat invariant mystery we encountered last week, which was due to not using atomic integers. The page fault handler is now completely lockless, since we're using CAS operations now when modifying page table entries. A bug in IsValidMemory has been fixed, where it wasn't clearing the 12 bottom bits of the page. Crash reports are now more durable to edge runtime states, e.g g_machine being null, crashes that happen outside sigsetjmp() or page tables gone.
jart · Feb 27, 2023 · ffa83e5 · ffa83e5
1 parent 329dbb4
commit ffa83e5
Show file tree

Hide file tree

Showing 40 changed files with 737 additions and 387 deletions.
diff --git a/README.md b/README.md
@@ -827,10 +827,8 @@ reasons of performance is defined to include pushing and popping.
 
 ### Threads
 
-System calls do not explicitly lock the memory pages they're accessing;
-we haven't determined yet if that makes calling munmap() unsafe. Blink
-also lacks support right now for unlocking robust mutexes when a guest
-program crashes; this too is something we'd like to fix.
+Blink doesn't have a working implementation of `set_robust_list()` yet,
+which means robust mutexes might not get unlocked if a process crashes.
 
 ### Coherency
 

diff --git a/blink/alu1.c b/blink/alu1.c
@@ -72,7 +72,7 @@ static void AluEvqp(P, const aluop_f ops[4]) {
   if (Rexw(rde)) {
     p = GetModrmRegisterWordPointerWrite8(A);
 #if CAN_64BIT
-    if (Lock(rde) && !((intptr_t)p & 7)) {
+    if (Lock(rde) && !((uintptr_t)p & 7)) {
       u64 x, z;
       x = atomic_load_explicit((_Atomic(u64) *)p, memory_order_acquire);
       do {
@@ -93,7 +93,7 @@ static void AluEvqp(P, const aluop_f ops[4]) {
   } else if (!Osz(rde)) {
     u32 x, z;
     p = GetModrmRegisterWordPointerWrite4(A);
-    if (Lock(rde) && !((intptr_t)p & 3)) {
+    if (Lock(rde) && !((uintptr_t)p & 3)) {
       x = atomic_load_explicit((_Atomic(u32) *)p, memory_order_acquire);
       do {
         z = Little32(f(m, Little32(x), 0));
@@ -110,7 +110,7 @@ static void AluEvqp(P, const aluop_f ops[4]) {
     }
   } else {
     p = GetModrmRegisterWordPointerWrite2(A);
-    if (Lock(rde) && !((intptr_t)p & 1)) {
+    if (Lock(rde) && !((uintptr_t)p & 1)) {
       u16 x, z;
       x = atomic_load_explicit((_Atomic(u16) *)p, memory_order_acquire);
       do {

diff --git a/blink/alu2.c b/blink/alu2.c
@@ -97,7 +97,7 @@ void OpAluw(P) {
   if (Rexw(rde)) {
     p = GetModrmRegisterWordPointerWrite8(A);
 #if CAN_64BIT
-    if (Lock(rde) && !((intptr_t)p & 7)) {
+    if (Lock(rde) && !((uintptr_t)p & 7)) {
       u64 x, y, z;
       x = atomic_load_explicit((_Atomic(u64) *)p, memory_order_acquire);
       y = atomic_load_explicit((_Atomic(u64) *)q, memory_order_relaxed);
@@ -132,7 +132,7 @@ void OpAluw(P) {
     if (IsModrmRegister(rde)) {
       Put32(p + 4, 0);
     }
-    if (Lock(rde) && !((intptr_t)p & 3)) {
+    if (Lock(rde) && !((uintptr_t)p & 3)) {
       x = atomic_load_explicit((_Atomic(u32) *)p, memory_order_acquire);
       y = atomic_load_explicit((_Atomic(u32) *)q, memory_order_relaxed);
       y = Little32(y);
@@ -152,7 +152,7 @@ void OpAluw(P) {
   } else {
     u16 x, y, z;
     p = GetModrmRegisterWordPointerWrite2(A);
-    if (Lock(rde) && !((intptr_t)p & 1)) {
+    if (Lock(rde) && !((uintptr_t)p & 1)) {
       x = atomic_load_explicit((_Atomic(u16) *)p, memory_order_acquire);
       y = atomic_load_explicit((_Atomic(u16) *)q, memory_order_relaxed);
       y = Little16(y);

diff --git a/blink/alui.c b/blink/alui.c
@@ -101,7 +101,7 @@ static void AluiLocked(P, u8 *p, aluop_f op) {
   switch (RegLog2(rde)) {
     case 3:
 #if CAN_64BIT
-      if (!((intptr_t)p & 7)) {
+      if (!((uintptr_t)p & 7)) {
         u64 x, z;
         x = atomic_load_explicit((_Atomic(u64) *)p, memory_order_acquire);
         do {
@@ -117,7 +117,7 @@ static void AluiLocked(P, u8 *p, aluop_f op) {
       UnlockBus(p);
       break;
     case 2:
-      if (!((intptr_t)p & 3)) {
+      if (!((uintptr_t)p & 3)) {
         u32 x, z;
         x = atomic_load_explicit((_Atomic(u32) *)p, memory_order_acquire);
         do {
@@ -132,7 +132,7 @@ static void AluiLocked(P, u8 *p, aluop_f op) {
       UnlockBus(p);
       break;
     case 1:
-      if (!((intptr_t)p & 1)) {
+      if (!((uintptr_t)p & 1)) {
         u16 x, z;
         x = atomic_load_explicit((_Atomic(u16) *)p, memory_order_acquire);
         do {

diff --git a/blink/assert.c b/blink/assert.c
@@ -17,34 +17,31 @@
 │ PERFORMANCE OF THIS SOFTWARE.                                                │
 ╚─────────────────────────────────────────────────────────────────────────────*/
 #include <errno.h>
-#include <limits.h>
 #include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
 
 #include "blink/assert.h"
 #include "blink/debug.h"
-#include "blink/endian.h"
+#include "blink/flag.h"
 #include "blink/log.h"
 #include "blink/machine.h"
-#include "blink/macros.h"
 #include "blink/util.h"
 
 void AssertFailed(const char *file, int line, const char *msg) {
   _Thread_local static bool noreentry;
-  char b[512];
-  FLAG_nologstderr = false;
-  snprintf(b, sizeof(b), "%s:%d: assertion failed: %s (%s)\n", file, line, msg,
-           DescribeHostErrno(errno));
-  b[sizeof(b) - 1] = 0;
-  WriteErrorString(b);
-  if (g_machine && !noreentry) {
+  _Thread_local static char bp[20000];
+  WriteErrorString("assertion failed\n");
+  if (!noreentry) {
     noreentry = true;
+    FLAG_nologstderr = false;
     RestoreIp(g_machine);
-    WriteErrorString("\t");
-    WriteErrorString(GetBacktrace(g_machine));
-    WriteErrorString("\n");
+    snprintf(bp, sizeof(bp),
+             "%s:%d:%d assertion failed: %s (%s)\n"
+             "\t%s\n"
+             "\t%s\n",
+             file, line, g_machine ? g_machine->tid : 666, msg,
+             DescribeHostErrno(errno), GetBacktrace(g_machine),
+             GetBlinkBacktrace());
+    WriteErrorString(bp);
   }
-  PrintBacktrace();
   Abort();
 }
diff --git a/blink/blink.c b/blink/blink.c
@@ -136,11 +136,9 @@ static void OnSigSys(int sig) {
 static void PrintDiagnostics(struct Machine *m) {
   ERRF("additional information\n"
        "\t%s\n"
+       "%s\n"
        "%s",
-       GetBacktrace(m), FormatPml4t(m));
-#ifdef DEBUG
-  PrintBacktrace();
-#endif
+       GetBacktrace(m), FormatPml4t(m), GetBlinkBacktrace());
 }
 
 void TerminateSignal(struct Machine *m, int sig) {
@@ -170,6 +168,8 @@ void TerminateSignal(struct Machine *m, int sig) {
 
 static void OnFatalSystemSignal(int sig, siginfo_t *si, void *ptr) {
   g_siginfo = *si;
+  unassert(g_machine);
+  unassert(g_machine->canhalt);
   siglongjmp(g_machine->onhalt, kMachineFatalSystemSignal);
 }
 
@@ -191,6 +191,8 @@ static int Exec(char *execfn, char *prog, char **argv, char **envp) {
       AddStdFd(&m->system->fds, i);
     }
   } else {
+    unassert(!m->sysdepth);
+    unassert(!m->pagelocks.i);
     unassert(!FreeVirtual(old->system, -0x800000000000, 0x1000000000000));
     for (i = 1; i <= 64; ++i) {
       if (Read64(old->system->hands[i - 1].handler) == SIG_IGN_LINUX) {

diff --git a/blink/bus.c b/blink/bus.c
@@ -96,7 +96,7 @@ i64 Load16(const u8 p[2]) {
     !defined(__SANITIZE_UNDEFINED__)
   z = atomic_load_explicit((_Atomic(u16) *)p, memory_order_relaxed);
 #else
-  if (!((intptr_t)p & 1)) {
+  if (!((uintptr_t)p & 1)) {
     z = Little16(atomic_load_explicit((_Atomic(u16) *)p, memory_order_acquire));
   } else {
     z = Read16(p);
@@ -111,7 +111,7 @@ i64 Load32(const u8 p[4]) {
     !defined(__SANITIZE_UNDEFINED__)
   z = atomic_load_explicit((_Atomic(u32) *)p, memory_order_relaxed);
 #else
-  if (!((intptr_t)p & 3)) {
+  if (!((uintptr_t)p & 3)) {
     z = Little32(atomic_load_explicit((_Atomic(u32) *)p, memory_order_acquire));
   } else {
     z = Read32(p);
@@ -126,7 +126,7 @@ i64 Load64(const u8 p[8]) {
 #if defined(__x86_64__) && !defined(__SANITIZE_UNDEFINED__)
   z = atomic_load_explicit((_Atomic(u64) *)p, memory_order_relaxed);
 #else
-  if (!((intptr_t)p & 7)) {
+  if (!((uintptr_t)p & 7)) {
     z = Little64(atomic_load_explicit((_Atomic(u64) *)p, memory_order_acquire));
   } else {
     z = Read64(p);
@@ -159,7 +159,7 @@ void Store16(u8 p[2], u64 x) {
     !defined(__SANITIZE_UNDEFINED__)
   atomic_store_explicit((_Atomic(u16) *)p, x, memory_order_relaxed);
 #else
-  if (!((intptr_t)p & 1)) {
+  if (!((uintptr_t)p & 1)) {
     atomic_store_explicit((_Atomic(u16) *)p, Little16(x), memory_order_release);
   } else {
     Write16(p, x);
@@ -172,7 +172,7 @@ void Store32(u8 p[4], u64 x) {
     !defined(__SANITIZE_UNDEFINED__)
   atomic_store_explicit((_Atomic(u32) *)p, x, memory_order_relaxed);
 #else
-  if (!((intptr_t)p & 3)) {
+  if (!((uintptr_t)p & 3)) {
     atomic_store_explicit((_Atomic(u32) *)p, Little32(x), memory_order_release);
   } else {
     Write32(p, x);
@@ -185,7 +185,7 @@ void Store64(u8 p[8], u64 x) {
 #if defined(__x86_64__) && !defined(__SANITIZE_UNDEFINED__)
   atomic_store_explicit((_Atomic(u64) *)p, x, memory_order_relaxed);
 #else
-  if (!((intptr_t)p & 7)) {
+  if (!((uintptr_t)p & 7)) {
     atomic_store_explicit((_Atomic(u64) *)p, Little64(x), memory_order_release);
   } else {
     Write64(p, x);

diff --git a/blink/bus.h b/blink/bus.h
@@ -1,7 +1,9 @@
 #ifndef BLINK_MOP_H_
 #define BLINK_MOP_H_
 #include <limits.h>
+#include <stdbool.h>
 
+#include "blink/assert.h"
 #include "blink/atomic.h"
 #include "blink/builtin.h"
 #include "blink/dll.h"
@@ -49,11 +51,11 @@ void InitBus(void);
 void LockBus(const u8 *);
 void UnlockBus(const u8 *);
 
-i64 Load8(const u8[1]);
-i64 Load16(const u8[2]);
-i64 Load32(const u8[4]);
-i64 Load64(const u8[8]);
-i64 Load64Unlocked(const u8[8]);
+i64 Load8(const u8[1]) nosideeffect;
+i64 Load16(const u8[2]) nosideeffect;
+i64 Load32(const u8[4]) nosideeffect;
+i64 Load64(const u8[8]) nosideeffect;
+i64 Load64Unlocked(const u8[8]) nosideeffect;
 
 void Store8(u8[1], u64);
 void Store16(u8[2], u64);
@@ -76,13 +78,16 @@ void WriteMemoryBW(u64, u8[8], u64);
 void WriteRegisterBW(u64, u8[8], u64);
 i64 ReadRegisterOrMemoryBW(u64, u8[8]);
 void WriteRegisterOrMemoryBW(u64, u8[8], u64);
+u64 LoadPte32_(const u8 *) nosideeffect;
+bool CasPte32_(u8 *, u64, u64);
+void StorePte32_(u8 *, u64);
 
-static inline u64 ReadPte(const u8 *pte) {
+nosideeffect static inline u64 LoadPte(const u8 *pte) {
 #if CAN_64BIT
   return Little64(
       atomic_load_explicit((_Atomic(u64) *)pte, memory_order_acquire));
 #else
-  return Load64(pte);
+  return LoadPte32_(pte);
 #endif
 }
 
@@ -91,7 +96,18 @@ static inline void StorePte(u8 *pte, u64 val) {
   atomic_store_explicit((_Atomic(u64) *)pte, Little64(val),
                         memory_order_release);
 #else
-  Store64(pte, val);
+  StorePte32_(pte, val);
+#endif
+}
+
+static inline bool CasPte(u8 *pte, u64 oldval, u64 newval) {
+#if CAN_64BIT
+  oldval = Little64(oldval);
+  return atomic_compare_exchange_strong_explicit(
+      (_Atomic(u64) *)pte, &oldval, Little64(newval),  //
+      memory_order_release, memory_order_relaxed);
+#else
+  return CasPte32_(pte, oldval, newval);
 #endif
 }
 

diff --git a/blink/cmpxchg.c b/blink/cmpxchg.c
@@ -45,7 +45,7 @@ static void LockCmpxchgMem32(struct Machine *m, u64 rde, u8 *p) {
   u32 x, z;
   bool didit;
   q = RegRexrReg(m, rde);
-  if (!((intptr_t)p & 3)) {
+  if (!((uintptr_t)p & 3)) {
     x = atomic_load_explicit((_Atomic(u32) *)m->ax, memory_order_relaxed);
     didit = atomic_compare_exchange_strong_explicit(
         (_Atomic(u32) *)p, &x,
@@ -75,7 +75,7 @@ static void Cmpxchg(struct Machine *m, u64 rde, u8 *p) {
   if (Rexw(rde)) {
     u64 x;
 #if CAN_64BIT
-    if (Lock(rde) && !((intptr_t)p & 7)) {
+    if (Lock(rde) && !((uintptr_t)p & 7)) {
       x = atomic_load_explicit((_Atomic(u64) *)m->ax, memory_order_relaxed);
       atomic_compare_exchange_strong_explicit(
           (_Atomic(u64) *)p, &x,
@@ -106,7 +106,7 @@ static void Cmpxchg(struct Machine *m, u64 rde, u8 *p) {
       }
     }
   } else if (!Osz(rde)) {
-    if (Lock(rde) && !((intptr_t)p & 3)) {
+    if (Lock(rde) && !((uintptr_t)p & 3)) {
       u32 ax =
           atomic_load_explicit((_Atomic(u32) *)m->ax, memory_order_relaxed);
       didit = atomic_compare_exchange_strong_explicit(
@@ -130,7 +130,7 @@ static void Cmpxchg(struct Machine *m, u64 rde, u8 *p) {
       Put32(p + 4, 0);
     }
   } else {
-    if (Lock(rde) && !((intptr_t)p & 1)) {
+    if (Lock(rde) && !((uintptr_t)p & 1)) {
       u16 ax =
           atomic_load_explicit((_Atomic(u16) *)m->ax, memory_order_relaxed);
       atomic_compare_exchange_strong_explicit(