shared heap: Fix some issues and add basic unit test case (#3801)

Fix some issues and add basic unit test case for shared heap feature.

Signed-off-by: wenlingyun1 <wenlingyun1@xiaomi.com>

core/iwasm/common/wasm_memory.c

@@ -148,22 +148,13 @@ static void
 wasm_munmap_linear_memory(void *mapped_mem, uint64 commit_size,
                           uint64 map_size);
 
-static void
-set_error_buf(char *error_buf, uint32 error_buf_size, const char *string)
-{
-    if (error_buf != NULL) {
-        snprintf(error_buf, error_buf_size,
-                 "Operation of shared heap failed: %s", string);
-    }
-}
-
 static void *
-runtime_malloc(uint64 size, char *error_buf, uint32 error_buf_size)
+runtime_malloc(uint64 size)
 {
     void *mem;
 
     if (size >= UINT32_MAX || !(mem = wasm_runtime_malloc((uint32)size))) {
-        set_error_buf(error_buf, error_buf_size, "allocate memory failed");
+        LOG_WARNING("Allocate memory failed");
         return NULL;
     }
 
@@ -172,27 +163,32 @@ runtime_malloc(uint64 size, char *error_buf, uint32 error_buf_size)
 }
 
 WASMSharedHeap *
-wasm_runtime_create_shared_heap(SharedHeapInitArgs *init_args, char *error_buf,
+wasm_runtime_create_shared_heap(SharedHeapInitArgs *init_args)
-                                uint32 error_buf_size)
 {
     uint64 heap_struct_size = sizeof(WASMSharedHeap);
     uint32 size = init_args->size;
     WASMSharedHeap *heap;
 
-    if (!(heap = runtime_malloc(heap_struct_size, error_buf, error_buf_size))) {
+    if (size == 0) {
         goto fail1;
     }
+
+    if (!(heap = runtime_malloc(heap_struct_size))) {
+        goto fail1;
+    }
+
     if (!(heap->heap_handle =
-              runtime_malloc(mem_allocator_get_heap_struct_size(), error_buf,
+              runtime_malloc(mem_allocator_get_heap_struct_size()))) {
-                             error_buf_size))) {
         goto fail2;
     }
+
+    size = align_uint(size, os_getpagesize());
+    heap->size = size;
     heap->start_off_mem64 = UINT64_MAX - heap->size + 1;
     heap->start_off_mem32 = UINT32_MAX - heap->size + 1;
 
-    size = align_uint(size, os_getpagesize());
     if (size > APP_HEAP_SIZE_MAX || size < APP_HEAP_SIZE_MIN) {
-        set_error_buf(error_buf, error_buf_size, "invalid size of shared heap");
+        LOG_WARNING("Invalid size of shared heap");
         goto fail3;
     }
 
@@ -201,7 +197,7 @@ wasm_runtime_create_shared_heap(SharedHeapInitArgs *init_args, char *error_buf,
     }
     if (!mem_allocator_create_with_struct_and_pool(
             heap->heap_handle, heap_struct_size, heap->base_addr, size)) {
-        set_error_buf(error_buf, error_buf_size, "init share heap failed");
+        LOG_WARNING("init share heap failed");
         goto fail4;
     }
 
@@ -365,20 +361,25 @@ wasm_runtime_shared_heap_malloc(WASMModuleInstanceCommon *module_inst,
     WASMMemoryInstance *memory =
         wasm_get_default_memory((WASMModuleInstance *)module_inst);
     WASMSharedHeap *shared_heap = get_shared_heap(module_inst);
+    void *native_addr = NULL;
 
     if (!memory || !shared_heap)
         return 0;
 
-    *p_native_addr = mem_allocator_malloc(shared_heap->heap_handle, size);
+    native_addr = mem_allocator_malloc(shared_heap->heap_handle, size);
-    if (!*p_native_addr)
+    if (!native_addr)
         return 0;
 
+    if (p_native_addr) {
+        *p_native_addr = native_addr;
+    }
+
     if (memory->is_memory64)
         return shared_heap->start_off_mem64
-               + ((uint8 *)*p_native_addr - shared_heap->base_addr);
+               + ((uint8 *)native_addr - shared_heap->base_addr);
     else
         return shared_heap->start_off_mem32
-               + ((uint8 *)*p_native_addr - shared_heap->base_addr);
+               + ((uint8 *)native_addr - shared_heap->base_addr);
 }
 
 void

core/iwasm/common/wasm_memory.h

@@ -43,8 +43,7 @@ SET_LINEAR_MEMORY_SIZE(WASMMemoryInstance *memory, uint64 size)
 
 #if WASM_ENABLE_SHARED_HEAP != 0
 WASMSharedHeap *
-wasm_runtime_create_shared_heap(SharedHeapInitArgs *init_args, char *error_buf,
+wasm_runtime_create_shared_heap(SharedHeapInitArgs *init_args);
-                                uint32 error_buf_size);
 
 bool
 wasm_runtime_attach_shared_heap(WASMModuleInstanceCommon *module_inst,

core/iwasm/common/wasm_runtime_common.c

@@ -4498,6 +4498,11 @@ wasm_runtime_invoke_native_raw(WASMExecEnv *exec_env, void *func_ptr,
                                uint32 *argv, uint32 argc, uint32 *argv_ret)
 {
     WASMModuleInstanceCommon *module = wasm_runtime_get_module_inst(exec_env);
+#if WASM_ENABLE_MEMORY64 != 0
+    WASMMemoryInstance *memory =
+        wasm_get_default_memory((WASMModuleInstance *)module);
+    bool is_memory64 = memory ? memory->is_memory64 : false;
+#endif
     typedef void (*NativeRawFuncPtr)(WASMExecEnv *, uint64 *);
     NativeRawFuncPtr invoke_native_raw = (NativeRawFuncPtr)func_ptr;
     uint64 argv_buf[16] = { 0 }, *argv1 = argv_buf, *argv_dst, size, arg_i64;
@@ -4525,11 +4530,11 @@ wasm_runtime_invoke_native_raw(WASMExecEnv *exec_env, void *func_ptr,
 #endif
             {
                 *(uint32 *)argv_dst = arg_i32 = *argv_src++;
-                /* TODO: memory64 if future there is a way for supporting
+                if (signature
-                 * wasm64 and wasm32 in libc at the same time, remove the
+#if WASM_ENABLE_MEMORY64 != 0
-                 * macro control */
+                    && !is_memory64
-#if WASM_ENABLE_MEMORY64 == 0
+#endif
-                if (signature) {
+                ) {
                     if (signature[i + 1] == '*') {
                         /* param is a pointer */
                         if (signature[i + 2] == '~')
@@ -4558,7 +4563,6 @@ wasm_runtime_invoke_native_raw(WASMExecEnv *exec_env, void *func_ptr,
                                 module, (uint64)arg_i32);
                     }
                 }
-#endif
                 break;
             }
             case VALUE_TYPE_I64:
@@ -4568,7 +4572,7 @@ wasm_runtime_invoke_native_raw(WASMExecEnv *exec_env, void *func_ptr,
                                 GET_I64_FROM_ADDR(argv_src));
                 argv_src += 2;
                 arg_i64 = *argv_dst;
-                if (signature) {
+                if (signature && is_memory64) {
                     /* TODO: memory64 pointer with length need a new symbol
                      * to represent type i64, with '~' still represent i32
                      * length */
@@ -4729,9 +4733,6 @@ wasm_runtime_invoke_native_raw(WASMExecEnv *exec_env, void *func_ptr,
 fail:
     if (argv1 != argv_buf)
         wasm_runtime_free(argv1);
-#if WASM_ENABLE_MEMORY64 == 0
-    (void)arg_i64;
-#endif
     return ret;
 }
 
@@ -5655,6 +5656,11 @@ wasm_runtime_invoke_native(WASMExecEnv *exec_env, void *func_ptr,
                            uint32 *argv_ret)
 {
     WASMModuleInstanceCommon *module = wasm_runtime_get_module_inst(exec_env);
+#if WASM_ENABLE_MEMORY64 != 0
+    WASMMemoryInstance *memory =
+        wasm_get_default_memory((WASMModuleInstance *)module);
+    bool is_memory64 = memory ? memory->is_memory64 : false;
+#endif
     uint64 argv_buf[32] = { 0 }, *argv1 = argv_buf, *ints, *stacks, size,
            arg_i64;
     uint32 *argv_src = argv, i, argc1, n_ints = 0, n_stacks = 0;
@@ -5720,11 +5726,11 @@ wasm_runtime_invoke_native(WASMExecEnv *exec_env, void *func_ptr,
             {
                 arg_i32 = *argv_src++;
                 arg_i64 = arg_i32;
-                /* TODO: memory64 if future there is a way for supporting
+                if (signature
-                 * wasm64 and wasm32 in libc at the same time, remove the
+#if WASM_ENABLE_MEMORY64 != 0
-                 * macro control */
+                    && !is_memory64
-#if WASM_ENABLE_MEMORY64 == 0
+#endif
-                if (signature) {
+                ) {
                     if (signature[i + 1] == '*') {
                         /* param is a pointer */
                         if (signature[i + 2] == '~')
@@ -5751,7 +5757,6 @@ wasm_runtime_invoke_native(WASMExecEnv *exec_env, void *func_ptr,
                             module, (uint64)arg_i32);
                     }
                 }
-#endif
                 if (n_ints < MAX_REG_INTS)
                     ints[n_ints++] = arg_i64;
                 else
@@ -5763,7 +5768,7 @@ wasm_runtime_invoke_native(WASMExecEnv *exec_env, void *func_ptr,
             {
                 arg_i64 = GET_I64_FROM_ADDR(argv_src);
                 argv_src += 2;
-                if (signature) {
+                if (signature && is_memory64) {
                     /* TODO: memory64 pointer with length need a new symbol
                      * to represent type i64, with '~' still represent i32
                      * length */

core/iwasm/include/wasm_export.h

@@ -323,11 +323,9 @@ typedef enum {
     WASM_LOG_LEVEL_VERBOSE = 4
 } log_level_t;
 
-#if WASM_ENABLE_SHARED_HEAP != 0
 typedef struct SharedHeapInitArgs {
-    uint32 size;
+    uint32_t size;
 } SharedHeapInitArgs;
-#endif
 
 /**
  * Initialize the WASM runtime environment, and also initialize
@@ -2119,21 +2117,21 @@ wasm_runtime_detect_native_stack_overflow_size(wasm_exec_env_t exec_env,
 WASM_RUNTIME_API_EXTERN bool
 wasm_runtime_is_underlying_binary_freeable(const wasm_module_t module);
 
-#if WASM_ENABLE_SHARED_HEAP != 0
 /**
  * Create a shared heap
+ *
  * @param init_args the initialization arguments
- * @param error_buf buffer to output the error info if failed
+ * @return the shared heap created
- * @param error_buf_size the size of the error buffer
  */
 WASM_RUNTIME_API_EXTERN wasm_shared_heap_t
-wasm_runtime_create_shared_heap(SharedHeapInitArgs *init_args, char *error_buf,
+wasm_runtime_create_shared_heap(SharedHeapInitArgs *init_args);
-                                uint32 error_buf_size);
 
 /**
  * Attach a shared heap to a module instance
+ *
  * @param module_inst the module instance
  * @param shared_heap the shared heap
+ * @return true if success, false if failed
  */
 WASM_RUNTIME_API_EXTERN bool
 wasm_runtime_attach_shared_heap(wasm_module_inst_t module_inst,
@@ -2141,6 +2139,7 @@ wasm_runtime_attach_shared_heap(wasm_module_inst_t module_inst,
 
 /**
  * Detach a shared heap from a module instance
+ *
  * @param module_inst the module instance
  */
 WASM_RUNTIME_API_EXTERN void
@@ -2148,22 +2147,29 @@ wasm_runtime_detach_shared_heap(wasm_module_inst_t module_inst);
 
 /**
  * Allocate memory from a shared heap
+ *
  * @param module_inst the module instance
  * @param size required memory size
  * @param p_native_addr native address of allocated memory
+ *
+ * @return return the allocated memory address, which re-uses part of the wasm
+ * address space and is in the range of [UINT32 - shared_heap_size + 1, UINT32]
+ * (when the wasm memory is 32-bit) or [UINT64 - shared_heap_size + 1, UINT64]
+ * (when the wasm memory is 64-bit). Note that it is not an absolute address.
+ *         Return non-zero if success, zero if failed.
  */
-WASM_RUNTIME_API_EXTERN uint64
+WASM_RUNTIME_API_EXTERN uint64_t
-wasm_runtime_shared_heap_malloc(wasm_module_inst_t module_inst, uint64 size,
+wasm_runtime_shared_heap_malloc(wasm_module_inst_t module_inst, uint64_t size,
                                 void **p_native_addr);
 
 /**
  * Free the memory allocated from shared heap
+ *
  * @param module_inst the module instance
  * @param ptr the offset in wasm app
  */
 WASM_RUNTIME_API_EXTERN void
-wasm_runtime_shared_heap_free(wasm_module_inst_t module_inst, uint64 ptr);
+wasm_runtime_shared_heap_free(wasm_module_inst_t module_inst, uint64_t ptr);
-#endif
 
 #ifdef __cplusplus
 }

core/iwasm/interpreter/wasm_runtime.h

@@ -97,7 +97,7 @@ typedef struct WASMSharedHeap {
     struct WASMSharedHeap *next;
     void *heap_handle;
     uint8 *base_addr;
-    uint32 size;
+    uint64 size;
     uint64 start_off_mem64;
     uint64 start_off_mem32;
 } WASMSharedHeap;

core/iwasm/libraries/shared-heap/shared_heap_wrapper.c

@@ -31,8 +31,10 @@ shared_free_wrapper(wasm_exec_env_t exec_env, void *ptr)
 {
     wasm_module_inst_t module_inst = get_module_inst(exec_env);
 
-    if (!validate_native_addr(ptr, (uint64)sizeof(uint32)))
+    if (!validate_native_addr(ptr, (uint64)sizeof(uintptr_t))) {
+        LOG_WARNING("Invalid app address");
         return;
+    }
 
     module_shared_free(addr_native_to_app(ptr));
 }

tests/unit/CMakeLists.txt

@@ -50,3 +50,4 @@ add_subdirectory(linux-perf)
 add_subdirectory(gc)
 add_subdirectory(memory64)
 add_subdirectory(tid-allocator)
+add_subdirectory(shared-heap)

tests/unit/shared-heap/CMakeLists.txt

@@ -0,0 +1,59 @@
+# Copyright (C) 2024 Xiaomi Corporation.  All rights reserved.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+cmake_minimum_required(VERSION 3.14)
+
+project(test-shared-heap)
+
+add_definitions(-DRUN_ON_LINUX)
+
+set(WAMR_BUILD_APP_FRAMEWORK 0)
+set(WAMR_BUILD_AOT 0)
+set(WAMR_BUILD_INTERP 1)
+set(WAMR_BUILD_FAST_INTERP 1)
+set(WAMR_BUILD_JIT 0)
+set(WAMR_BUILD_MEMORY64 1)
+set(WAMR_BUILD_SHARED_HEAP 1)
+
+# Compile wasm modules
+add_subdirectory(wasm-apps)
+
+# if only load this CMake other than load it as subdirectory
+include(../unit_common.cmake)
+
+set(LLVM_SRC_ROOT "${WAMR_ROOT_DIR}/core/deps/llvm")
+
+if (NOT EXISTS "${LLVM_SRC_ROOT}/build")
+    message(FATAL_ERROR "Cannot find LLVM dir: ${LLVM_SRC_ROOT}/build")
+endif ()
+
+set(CMAKE_PREFIX_PATH "${LLVM_SRC_ROOT}/build;${CMAKE_PREFIX_PATH}")
+find_package(LLVM REQUIRED CONFIG)
+include_directories(${LLVM_INCLUDE_DIRS})
+add_definitions(${LLVM_DEFINITIONS})
+message(STATUS "Found LLVM ${LLVM_PACKAGE_VERSION}")
+message(STATUS "Using LLVMConfig.cmake in: ${LLVM_DIR}")
+
+include(${IWASM_DIR}/compilation/iwasm_compl.cmake)
+
+include_directories(${CMAKE_CURRENT_SOURCE_DIR})
+
+file(GLOB_RECURSE source_all ${CMAKE_CURRENT_SOURCE_DIR}/*.cc)
+
+set(UNIT_SOURCE ${source_all})
+
+aux_source_directory(. SRC_LIST)
+
+set(unit_test_sources
+        ${UNIT_SOURCE}
+        ${WAMR_RUNTIME_LIB_SOURCE}
+        ${UNCOMMON_SHARED_SOURCE}
+        ${SRC_LIST}
+        )
+
+# Now simply link against gtest or gtest_main as needed. Eg
+add_executable(shared_heap_test ${unit_test_sources})
+
+target_link_libraries(shared_heap_test ${LLVM_AVAILABLE_LIBS} gtest_main)
+
+gtest_discover_tests(shared_heap_test)

tests/unit/shared-heap/shared_heap_test.cc

@@ -0,0 +1,142 @@
+/*
+ * Copyright (C) 2024 Xiaomi Corporation.  All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ */
+
+#include "test_helper.h"
+#include "gtest/gtest.h"
+
+#include "bh_read_file.h"
+#include "wasm_runtime_common.h"
+
+class shared_heap_test : public testing::Test
+{
+  protected:
+    virtual void SetUp() {}
+    static void SetUpTestCase() {}
+    virtual void TearDown() {}
+    WAMRRuntimeRAII<512 * 1024> runtime;
+};
+
+struct ret_env {
+    wasm_exec_env_t exec_env;
+    wasm_module_t wasm_module;
+    wasm_module_inst_t wasm_module_inst;
+    unsigned char *wasm_file_buf;
+    char error_buf[128];
+};
+
+struct ret_env
+load_wasm(char *wasm_file_tested, unsigned int app_heap_size)
+{
+    std::string wasm_mem_page = wasm_file_tested;
+    const char *wasm_file = strdup(wasm_mem_page.c_str());
+    wasm_module_inst_t wasm_module_inst = nullptr;
+    wasm_module_t wasm_module = nullptr;
+    wasm_exec_env_t exec_env = nullptr;
+    unsigned char *wasm_file_buf = nullptr;
+    unsigned int wasm_file_size = 0;
+    unsigned int stack_size = 16 * 1024, heap_size = app_heap_size;
+    char error_buf[128] = { 0 };
+    struct ret_env ret_module_env;
+
+    memset(ret_module_env.error_buf, 0, 128);
+    wasm_file_buf =
+        (unsigned char *)bh_read_file_to_buffer(wasm_file, &wasm_file_size);
+    if (!wasm_file_buf) {
+        goto fail;
+    }
+
+    wasm_module = wasm_runtime_load(wasm_file_buf, wasm_file_size, error_buf,
+                                    sizeof(error_buf));
+    if (!wasm_module) {
+        memcpy(ret_module_env.error_buf, error_buf, 128);
+        goto fail;
+    }
+
+    wasm_module_inst = wasm_runtime_instantiate(
+        wasm_module, stack_size, heap_size, error_buf, sizeof(error_buf));
+    if (!wasm_module_inst) {
+        memcpy(ret_module_env.error_buf, error_buf, 128);
+        goto fail;
+    }
+
+    exec_env = wasm_runtime_create_exec_env(wasm_module_inst, stack_size);
+
+fail:
+    ret_module_env.exec_env = exec_env;
+    ret_module_env.wasm_module = wasm_module;
+    ret_module_env.wasm_module_inst = wasm_module_inst;
+    ret_module_env.wasm_file_buf = wasm_file_buf;
+
+    return ret_module_env;
+}
+
+void
+destroy_module_env(struct ret_env module_env)
+{
+    if (module_env.exec_env) {
+        wasm_runtime_destroy_exec_env(module_env.exec_env);
+    }
+
+    if (module_env.wasm_module_inst) {
+        wasm_runtime_deinstantiate(module_env.wasm_module_inst);
+    }
+
+    if (module_env.wasm_module) {
+        wasm_runtime_unload(module_env.wasm_module);
+    }
+
+    if (module_env.wasm_file_buf) {
+        wasm_runtime_free(module_env.wasm_file_buf);
+    }
+}
+
+TEST_F(shared_heap_test, test_shared_heap)
+{
+    struct ret_env tmp_module_env;
+    WASMFunctionInstanceCommon *func_test = nullptr;
+    bool ret = false;
+    uint32 argv[1] = { 65535 };
+    const char *exception = nullptr;
+    SharedHeapInitArgs args;
+    WASMSharedHeap *shared_heap = nullptr;
+
+    args.size = 1024;
+    shared_heap = wasm_runtime_create_shared_heap(&args);
+    tmp_module_env = load_wasm((char *)"test.wasm", 0);
+
+    if (!shared_heap) {
+        printf("Failed to create shared heap\n");
+        goto test_failed;
+    }
+    if (!wasm_runtime_attach_shared_heap(tmp_module_env.wasm_module_inst, shared_heap)) {
+        printf("Failed to attach shared heap\n");
+        goto test_failed;
+    }
+    func_test = wasm_runtime_lookup_function(
+        tmp_module_env.wasm_module_inst, "test");
+    if (!func_test) {
+        printf("\nFailed to wasm_runtime_lookup_function!\n");
+        goto test_failed;
+    }
+
+    ret =
+        wasm_runtime_call_wasm(tmp_module_env.exec_env, func_test, 1, argv);
+    if (!ret) {
+        printf("\nFailed to wasm_runtime_call_wasm!\n");
+        const char *s = wasm_runtime_get_exception(tmp_module_env.wasm_module_inst);
+        printf("exception: %s\n", s);
+        goto test_failed;
+    }
+
+    wasm_runtime_detach_shared_heap(tmp_module_env.wasm_module_inst);
+
+    EXPECT_EQ(10, argv[0]);
+
+    destroy_module_env(tmp_module_env);
+    return;
+test_failed:
+    destroy_module_env(tmp_module_env);
+    EXPECT_EQ(1, 0);
+}

tests/unit/shared-heap/wasm-apps/CMakeLists.txt

@@ -0,0 +1,39 @@
+# Copyright (C) 2024 Xiaomi Corporation.  All rights reserved.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+cmake_minimum_required(VERSION 3.14)
+project(wasm-apps)
+
+set(WAMR_ROOT_DIR ${CMAKE_CURRENT_SOURCE_DIR}/../../../..)
+
+set(CMAKE_SYSTEM_PROCESSOR wasm32)
+set(CMAKE_SYSROOT ${WAMR_ROOT_DIR}/wamr-sdk/app/libc-builtin-sysroot)
+
+if (NOT DEFINED WASI_SDK_DIR)
+    set(WASI_SDK_DIR "/opt/wasi-sdk")
+endif ()
+
+set(CMAKE_C_FLAGS "-nostdlib -pthread -Qunused-arguments")
+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -z stack-size=8192 -nostdlib -O0")
+set(CMAKE_C_COMPILER_TARGET "wasm32")
+set(CMAKE_C_COMPILER "${WASI_SDK_DIR}/bin/clang")
+
+set(DEFINED_SYMBOLS
+        "${WAMR_ROOT_DIR}/wamr-sdk/app/libc-builtin-sysroot/share/defined-symbols.txt")
+
+set(CMAKE_EXE_LINKER_FLAGS
+        "-Wl,--no-entry           \
+      -Wl,--initial-memory=65536  \
+      -Wl,--export-all            \
+      -Wl,--allow-undefined"
+        )
+
+add_executable(test.wasm test.c)
+target_link_libraries(test.wasm)
+
+add_custom_command(TARGET test.wasm POST_BUILD
+        COMMAND ${CMAKE_COMMAND} -E copy
+        ${CMAKE_CURRENT_BINARY_DIR}/test.wasm
+        ${CMAKE_CURRENT_BINARY_DIR}/../
+        COMMENT "Copy test.wasm to the same directory of google test"
+        )

tests/unit/shared-heap/wasm-apps/test.c

@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2024 Xiaomi Corporation.  All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ */
+
+#include <stdio.h>
+
+extern void *
+shared_malloc(int size);
+extern void
+shared_free(void *offset);
+
+int
+test()
+{
+    int *ptr = (int *)shared_malloc(10);
+
+    *ptr = 10;
+    int a = *ptr;
+    shared_free(ptr);
+    return a;
+}