[LibOS] Force variable update on tcb.test_range.has_fault in test_user_memory()

In test_user_memory(), a memory range is tested via probing of each page
in the range. If the memory page was not allocated, it leads to a
segfault which is captured by the LibOS memfault_upcall() and reported
in the variable tcb.test_range.has_fault. However, the compiler may
optimize away accesses to this variable in test_user_memory(), believing
it is never updated anywhere else. This commit introduces a memory
barrier to prevent this compiler optimization (same for test_user_string()).

LibOS/shim/include/shim_tls.h

@@ -64,6 +64,7 @@ struct shim_tcb {
     struct {
         void * start, * end;
         void * cont_addr;
+        bool has_fault;
     } test_range;
 };
 

LibOS/shim/src/bookkeep/shim_signal.c

@@ -254,6 +254,7 @@ static void memfault_upcall (PAL_PTR event, PAL_NUM arg, PAL_CONTEXT * context)
         && (void *) arg >= tcb->test_range.start
         && (void *) arg <= tcb->test_range.end) {
         assert(context);
+        tcb->test_range.has_fault = true;
         context->rip = (PAL_NUM) tcb->test_range.cont_addr;
         goto ret_exception;
     }
@@ -365,14 +366,15 @@ bool test_user_memory (void * addr, size_t size, bool write)
     assert(tcb && tcb->tp);
     __disable_preempt(tcb);
 
-    bool  has_fault = true;
-
     /* Add the memory region to the watch list. This is not racy because
      * each thread has its own record. */
     assert(!tcb->test_range.cont_addr);
+    tcb->test_range.has_fault = false;
     tcb->test_range.cont_addr = &&ret_fault;
     tcb->test_range.start = addr;
     tcb->test_range.end   = addr + size - 1;
+    /* enforce compiler to store tcb->test_range into memory */
+    __asm__ volatile(""::: "memory");
 
     /* Try to read or write into one byte inside each page */
     void * tmp = addr;
@@ -385,11 +387,14 @@ bool test_user_memory (void * addr, size_t size, bool write)
         tmp = ALIGN_UP(tmp + 1);
     }
 
-    has_fault = false; /* All accesses have passed. Nothing wrong. */
-
 ret_fault:
+    /* enforce compiler to load tcb->test_range.has_fault below */
+    __asm__ volatile("": "=m"(tcb->test_range.has_fault));
+
     /* If any read or write into the target region causes an exception,
      * the control flow will immediately jump to here. */
+    bool has_fault = tcb->test_range.has_fault;
+    tcb->test_range.has_fault = false;
     tcb->test_range.cont_addr = NULL;
     tcb->test_range.start = tcb->test_range.end = NULL;
     __enable_preempt(tcb);
@@ -432,10 +437,11 @@ bool test_user_string (const char * addr)
     assert(tcb && tcb->tp);
     __disable_preempt(tcb);
 
-    bool has_fault = true;
-
     assert(!tcb->test_range.cont_addr);
+    tcb->test_range.has_fault = false;
     tcb->test_range.cont_addr = &&ret_fault;
+    /* enforce compiler to store tcb->test_range into memory */
+    __asm__ volatile(""::: "memory");
 
     do {
         /* Add the memory region to the watch list. This is not racy because
@@ -454,11 +460,14 @@ bool test_user_string (const char * addr)
         next = ALIGN_UP(addr + 1);
     } while (size == maxlen);
 
-    has_fault = false; /* All accesses have passed. Nothing wrong. */
-
 ret_fault:
+    /* enforce compiler to load tcb->test_range.has_fault below */
+    __asm__ volatile("": "=m"(tcb->test_range.has_fault));
+
     /* If any read or write into the target region causes an exception,
      * the control flow will immediately jump to here. */
+    bool has_fault = tcb->test_range.has_fault;
+    tcb->test_range.has_fault = false;
     tcb->test_range.cont_addr = NULL;
     tcb->test_range.start = tcb->test_range.end = NULL;
     __enable_preempt(tcb);