Просмотр исходного кода

Fix data races in atomic wait/notify and interp goto table (#1971)

Add shared memory lock when accessing the address to atomic wait/notify
inside linear memory to resolve its data race issue.

And statically initialize the goto table of interpreter labels to resolve the
data race issue of accessing the table.
Enrico Loparco 3 лет назад
Родитель
Сommit
bc60064de5
2 измененных файлов с 37 добавлено и 19 удалено
  1. 28 8
      core/iwasm/common/wasm_shared_memory.c
  2. 9 11
      core/iwasm/interpreter/wasm_opcode.h

+ 28 - 8
core/iwasm/common/wasm_shared_memory.c

@@ -157,13 +157,13 @@ int32
 shared_memory_inc_reference(WASMModuleCommon *module)
 shared_memory_inc_reference(WASMModuleCommon *module)
 {
 {
     WASMSharedMemNode *node = search_module(module);
     WASMSharedMemNode *node = search_module(module);
+    uint32 ref_count = -1;
     if (node) {
     if (node) {
         os_mutex_lock(&node->lock);
         os_mutex_lock(&node->lock);
-        node->ref_count++;
+        ref_count = ++node->ref_count;
         os_mutex_unlock(&node->lock);
         os_mutex_unlock(&node->lock);
-        return node->ref_count;
     }
     }
-    return -1;
+    return ref_count;
 }
 }
 
 
 int32
 int32
@@ -385,7 +385,8 @@ wasm_runtime_atomic_wait(WASMModuleInstanceCommon *module, void *address,
     WASMModuleInstance *module_inst = (WASMModuleInstance *)module;
     WASMModuleInstance *module_inst = (WASMModuleInstance *)module;
     AtomicWaitInfo *wait_info;
     AtomicWaitInfo *wait_info;
     AtomicWaitNode *wait_node;
     AtomicWaitNode *wait_node;
-    bool check_ret, is_timeout;
+    WASMSharedMemNode *node;
+    bool check_ret, is_timeout, no_wait;
 
 
     bh_assert(module->module_type == Wasm_Module_Bytecode
     bh_assert(module->module_type == Wasm_Module_Bytecode
               || module->module_type == Wasm_Module_AoT);
               || module->module_type == Wasm_Module_AoT);
@@ -417,8 +418,13 @@ wasm_runtime_atomic_wait(WASMModuleInstanceCommon *module, void *address,
 
 
     os_mutex_lock(&wait_info->wait_list_lock);
     os_mutex_lock(&wait_info->wait_list_lock);
 
 
-    if ((!wait64 && *(uint32 *)address != (uint32)expect)
-        || (wait64 && *(uint64 *)address != expect)) {
+    node = search_module((WASMModuleCommon *)module_inst->module);
+    os_mutex_lock(&node->shared_mem_lock);
+    no_wait = (!wait64 && *(uint32 *)address != (uint32)expect)
+              || (wait64 && *(uint64 *)address != expect);
+    os_mutex_unlock(&node->shared_mem_lock);
+
+    if (no_wait) {
         os_mutex_unlock(&wait_info->wait_list_lock);
         os_mutex_unlock(&wait_info->wait_list_lock);
         return 1;
         return 1;
     }
     }
@@ -476,7 +482,9 @@ wasm_runtime_atomic_wait(WASMModuleInstanceCommon *module, void *address,
     wasm_runtime_free(wait_node);
     wasm_runtime_free(wait_node);
     os_mutex_unlock(&wait_info->wait_list_lock);
     os_mutex_unlock(&wait_info->wait_list_lock);
 
 
+    os_mutex_lock(&node->shared_mem_lock);
     release_wait_info(wait_map, wait_info, address);
     release_wait_info(wait_map, wait_info, address);
+    os_mutex_unlock(&node->shared_mem_lock);
 
 
     (void)check_ret;
     (void)check_ret;
     return is_timeout ? 2 : 0;
     return is_timeout ? 2 : 0;
@@ -489,17 +497,29 @@ wasm_runtime_atomic_notify(WASMModuleInstanceCommon *module, void *address,
     WASMModuleInstance *module_inst = (WASMModuleInstance *)module;
     WASMModuleInstance *module_inst = (WASMModuleInstance *)module;
     uint32 notify_result;
     uint32 notify_result;
     AtomicWaitInfo *wait_info;
     AtomicWaitInfo *wait_info;
+    WASMSharedMemNode *node;
+    bool out_of_bounds;
 
 
     bh_assert(module->module_type == Wasm_Module_Bytecode
     bh_assert(module->module_type == Wasm_Module_Bytecode
               || module->module_type == Wasm_Module_AoT);
               || module->module_type == Wasm_Module_AoT);
 
 
-    if ((uint8 *)address < module_inst->memories[0]->memory_data
-        || (uint8 *)address + 4 > module_inst->memories[0]->memory_data_end) {
+    node = search_module((WASMModuleCommon *)module_inst->module);
+    if (node)
+        os_mutex_lock(&node->shared_mem_lock);
+    out_of_bounds =
+        ((uint8 *)address < module_inst->memories[0]->memory_data
+         || (uint8 *)address + 4 > module_inst->memories[0]->memory_data_end);
+
+    if (out_of_bounds) {
+        if (node)
+            os_mutex_unlock(&node->shared_mem_lock);
         wasm_runtime_set_exception(module, "out of bounds memory access");
         wasm_runtime_set_exception(module, "out of bounds memory access");
         return -1;
         return -1;
     }
     }
 
 
     wait_info = acquire_wait_info(address, false);
     wait_info = acquire_wait_info(address, false);
+    if (node)
+        os_mutex_unlock(&node->shared_mem_lock);
 
 
     /* Nobody wait on this address */
     /* Nobody wait on this address */
     if (!wait_info)
     if (!wait_info)

+ 9 - 11
core/iwasm/interpreter/wasm_opcode.h

@@ -675,12 +675,14 @@ typedef enum WASMAtomicEXTOpcode {
 } WASMAtomicEXTOpcode;
 } WASMAtomicEXTOpcode;
 
 
 #if WASM_ENABLE_DEBUG_INTERP != 0
 #if WASM_ENABLE_DEBUG_INTERP != 0
-#define DEF_DEBUG_BREAK_HANDLE(_name) \
-    _name[DEBUG_OP_BREAK] = HANDLE_OPCODE(DEBUG_OP_BREAK); /* 0xd7 */
+#define DEF_DEBUG_BREAK_HANDLE() \
+    [DEBUG_OP_BREAK] = HANDLE_OPCODE(DEBUG_OP_BREAK), /* 0xd7 */
 #else
 #else
-#define DEF_DEBUG_BREAK_HANDLE(_name)
+#define DEF_DEBUG_BREAK_HANDLE()
 #endif
 #endif
 
 
+#define SET_GOTO_TABLE_ELEM(opcode) [opcode] = HANDLE_OPCODE(opcode)
+
 /*
 /*
  * Macro used to generate computed goto tables for the C interpreter.
  * Macro used to generate computed goto tables for the C interpreter.
  */
  */
@@ -903,14 +905,10 @@ typedef enum WASMAtomicEXTOpcode {
         HANDLE_OPCODE(EXT_OP_LOOP),                  /* 0xd4 */ \
         HANDLE_OPCODE(EXT_OP_LOOP),                  /* 0xd4 */ \
         HANDLE_OPCODE(EXT_OP_IF),                    /* 0xd5 */ \
         HANDLE_OPCODE(EXT_OP_IF),                    /* 0xd5 */ \
         HANDLE_OPCODE(EXT_OP_BR_TABLE_CACHE),        /* 0xd6 */ \
         HANDLE_OPCODE(EXT_OP_BR_TABLE_CACHE),        /* 0xd6 */ \
-    };                                                          \
-    do {                                                        \
-        _name[WASM_OP_MISC_PREFIX] =                            \
-            HANDLE_OPCODE(WASM_OP_MISC_PREFIX); /* 0xfc */      \
-        _name[WASM_OP_ATOMIC_PREFIX] =                          \
-            HANDLE_OPCODE(WASM_OP_ATOMIC_PREFIX); /* 0xfe */    \
-        DEF_DEBUG_BREAK_HANDLE(_name)                           \
-    } while (0)
+        SET_GOTO_TABLE_ELEM(WASM_OP_MISC_PREFIX),    /* 0xfc */ \
+        SET_GOTO_TABLE_ELEM(WASM_OP_ATOMIC_PREFIX),  /* 0xfe */ \
+        DEF_DEBUG_BREAK_HANDLE()                                \
+    };
 
 
 #ifdef __cplusplus
 #ifdef __cplusplus
 }
 }