feat: refactor dl_sym

1. refactor dl_sym
2. update test

tests/dl.cpp

@@ -5,8 +5,6 @@ extern "C" {
 #include "utils/defs.h"
 #include "wrt/lib/dl.h"
 #include "wasm_exec_env.h"
-
-extern bool signature_check(const char *signature, int *arg_count, bool *has_ret);
 }
 
 DLContext   dl_context;
@@ -15,47 +13,10 @@ WASMExecEnv exec_env;
 void init() {
     dl_context.mem.malloc = malloc;
     dl_context.mem.free   = free;
-    dl_context.symTable   = nullptr;
+    dl_init(&dl_context);
-    memset(dl_context.hnd, 0, sizeof(dl_context.hnd));
-    memset(dl_context.sym, 0, sizeof(dl_context.sym));
     exec_env.attachment = &dl_context;
 }
 
-#define check_symbol_consistency(sym, postfix)                       \
-    do {                                                             \
-        ASSERT_GT((sym), 0);                                         \
-        ASSERT_LT((sym), DL_MAX_SYMBOLS);                            \
-        ASSERT_NE(dl_context.sym[(sym)-1].symbol, nullptr);          \
-        ASSERT_NE(dl_context.sym[(sym)-1].cif, nullptr);             \
-        ASSERT_GT(dl_context.sym[(sym)-1].backref, 0);               \
-        unsigned ref##postfix = dl_context.sym[(sym)-1].backref;     \
-        ASSERT_GT(ref##postfix, 0);                                  \
-        ASSERT_LE(ref##postfix, DL_MAX_HANDLES);                     \
-        ASSERT_NE(dl_context.hnd[ref##postfix - 1].handle, nullptr); \
-    } while (0)
-
-#define check_hnd_empty(postfix)                     \
-    do {                                             \
-        for (auto &hnd##postfix : dl_context.hnd) {  \
-            ASSERT_EQ(hnd##postfix.handle, nullptr); \
-        }                                            \
-    } while (0)
-
-#define check_sym_empty(postfix)                     \
-    do {                                             \
-        for (auto &sym##postfix : dl_context.sym) {  \
-            ASSERT_EQ(sym##postfix.symbol, nullptr); \
-            ASSERT_EQ(sym##postfix.cif, nullptr);    \
-            ASSERT_EQ(sym##postfix.backref, 0);      \
-        }                                            \
-    } while (0)
-
-#define check_empty(postfix)      \
-    do {                          \
-        check_sym_empty(postfix); \
-        check_hnd_empty(postfix); \
-    } while (0)
-
 TEST(DLTest, InitAndFree) {
     // use valgrind !
     auto *context = (DLContext *)malloc(sizeof(DLContext));
@@ -84,20 +45,18 @@ TEST(DLTest, OpenAndClose) {
     int handle = dl_open(&exec_env, "./SimpleLib.so", RTLD_LAZY);
     ASSERT_GT(handle, 0);
     int result = dl_close(&exec_env, handle);
-    ASSERT_EQ(result, WRT_OK);
+    EXPECT_EQ(result, WRT_OK);
-    check_empty(0);
 
     handle = dl_open(&exec_env, "not_exists", RTLD_LAZY);
-    ASSERT_EQ(handle, WRT_ERROR);
+    EXPECT_EQ(handle, WRT_ERROR);
-    check_empty(1);
 
     handle = dl_open(&exec_env, "", RTLD_LAZY);
-    ASSERT_EQ(handle, WRT_ERROR);
+    EXPECT_EQ(handle, WRT_ERROR);
-    check_empty(2);
 
     handle = dl_open(&exec_env, nullptr, RTLD_LAZY);
-    ASSERT_EQ(handle, WRT_ERROR);
+    EXPECT_EQ(handle, WRT_ERROR);
-    check_empty(3);
+
+    dl_free(&dl_context);
 }
 
 TEST(DLTest, ManyOpen) {
@@ -111,15 +70,47 @@ TEST(DLTest, ManyOpen) {
 
     {
         int handle = dl_open(&exec_env, "./SimpleLib.so", RTLD_LAZY);
-        ASSERT_EQ(handle, WRT_ERROR);
+        EXPECT_EQ(handle, WRT_ERROR);
     }
 
     for (int handle : handles) {
         int result = dl_close(&exec_env, handle);
-        ASSERT_EQ(result, WRT_OK);
+        EXPECT_EQ(result, WRT_OK);
     }
 
-    check_empty(0);
+    for (int i = 0; i < DL_MAX_HANDLES; i++) {
+        EXPECT_EQ(dl_context.hnd->handle, nullptr);
+    }
+
+    dl_free(&dl_context);
+}
+
+TEST(DLTest, GetCloseSym) {
+    init();
+
+    int hnd = dl_open(&exec_env, "./SimpleLib.so", RTLD_LAZY);
+    ASSERT_EQ(hnd, 1);
+
+    int sym;
+
+    sym = dl_sym(&exec_env, hnd, "fib_fast", "(i)i");
+    EXPECT_EQ(sym, 1);
+    EXPECT_NE(dl_context.sym[0].symbol, nullptr);
+    EXPECT_NE(dl_context.sym[0].cif, nullptr);
+    EXPECT_EQ(dl_context.sym[0].cif->nargs, 1);
+    EXPECT_EQ(dl_context.sym[0].cif->rtype, &ffi_type_sint);
+    EXPECT_EQ(dl_context.sym[0].cif->arg_types[0], &ffi_type_sint);
+    EXPECT_EQ(dl_context.sym[0].backref, 1);
+
+    sym = dl_sym(&exec_env, hnd, "fib_fast", "(v)i");
+    EXPECT_EQ(sym, WRT_ERROR);
+
+    dl_close_sym(&exec_env, 1);
+    EXPECT_EQ(dl_context.sym[0].symbol, nullptr);
+    EXPECT_EQ(dl_context.sym[0].cif, nullptr);
+    EXPECT_EQ(dl_context.sym[0].backref, 0);
+
+    dl_free(&dl_context);
 }
 
 TEST(DLTest, SignatureCheck) {
@@ -140,38 +131,33 @@ TEST(DLTest, SignatureCheck) {
         {"(i(i))i", false, 0, false},    {"i)i", false, 0, false},  {"((i)i", false, 0, false},
         {"(i))i", false, 0, false},      {"())", false, 0, false}};
 
-    for (const auto &cur_case : cases) {
-        int  arg_count = 0;
-        bool has_ret   = false;
-        bool ok        = signature_check(cur_case.test, &arg_count, &has_ret);
-
-        EXPECT_EQ(ok, cur_case.result);
-        if (ok) {
-            EXPECT_EQ(arg_count, cur_case.arg_count);
-            EXPECT_EQ(has_ret, cur_case.has_ret);
-        }
-    }
-}
-
-TEST(DLTest, GetCloseSym) {
     init();
-
     int hnd = dl_open(&exec_env, "./SimpleLib.so", RTLD_LAZY);
-    {
+    ASSERT_EQ(hnd, 1);
-        int sym = dl_sym(&exec_env, hnd, "fib_fast", "(i)i");
+
-        check_symbol_consistency(sym, 0);
+    for (const auto &cur_case : cases) {
+        int sym = dl_sym(&exec_env, hnd, "fib_fast", cur_case.test);
+        if (cur_case.result) {
+            EXPECT_EQ(sym, 1);
+
+            EXPECT_NE(dl_context.sym[0].symbol, nullptr);
+            EXPECT_NE(dl_context.sym[0].cif, nullptr);
+            EXPECT_EQ(dl_context.sym[0].cif->nargs, cur_case.arg_count);
+            if (cur_case.has_ret) {
+                EXPECT_NE(dl_context.sym[0].cif->rtype, &ffi_type_void);
+                EXPECT_NE(dl_context.sym[0].cif->rtype, nullptr);
+            } else {
+                EXPECT_EQ(dl_context.sym[0].cif->rtype, &ffi_type_void);
             }
 
-    {
+            dl_close_sym(&exec_env, 1);
-        int sym = dl_sym(&exec_env, hnd, "fib_fast", "(v)i");
+            EXPECT_EQ(dl_context.sym[0].symbol, nullptr);
-        ASSERT_EQ(sym, WRT_ERROR);
+            EXPECT_EQ(dl_context.sym[0].cif, nullptr);
+            EXPECT_EQ(dl_context.sym[0].backref, 0);
+        } else {
+            EXPECT_EQ(sym, WRT_ERROR);
+        }
     }
 
-    {
+    dl_free(&dl_context);
-        dl_close_sym(&exec_env, hnd, 1);
-        check_sym_empty(0);
-    }
-
-    dl_close(&exec_env, hnd);
-    check_empty(0);
 }

wrt/lib/dl.c

@@ -14,6 +14,12 @@
         return ret;                                   \
     } while (0)
 
+#define LOG_AND_RETURN(msg, ret) \
+    do {                         \
+        LOG_ERR(msg);            \
+        return ret;              \
+    } while (0)
+
 #define GET_CONTEXT(ret)                                             \
     wasm_module_inst_t module_inst = get_module_inst(exec_env);      \
     (void)module_inst;                                               \
@@ -90,123 +96,78 @@ int dl_open(wasm_exec_env_t exec_env, const char *filename, int flags) {
     return WRT_ERROR;
 }
 
-// TODO: integrate it into create_ffi_cif
+ffi_cif *create_ffi_cif(DLContext *ctx, const char *signature) {
-bool signature_check(const char *signature, int *arg_count, bool *has_ret) {
+#define RET_ERR() LOG_AND_RETURN("Invalid signature", NULL)
-    const char allowed_chars[]   = {'i', 'l', 'f', 'd', 'p'};
-    const int  allowed_chars_len = sizeof(allowed_chars) / sizeof(char);
 
-    // tmpl: '(' ch* ')' ch?
+    if (signature == NULL) RET_ERR();
+    size_t sig_len = strlen(signature);
+    if (sig_len < 2) RET_ERR();
 
+    // Parse signature
+    // signature ::= '(' type_arg* ')' type_ret?
+
+    // alloca temporary memory
+    ffi_type **arg_types_tmp = alloca(sizeof(ffi_type *) * sig_len);
+    ffi_type  *ret_type      = NULL;
+
+    int         arg_count = 0;
     const char *ch        = signature;
 
     // 1. first char must be '('
-    if (*ch != '(') return false;
+    if (*ch++ != '(') RET_ERR();
 
-    // 2. check inside paren
+    // 2. check type_arg*
-    *arg_count = 0;
+    bool right_paren = false;
-    ch++;
-    bool right_paren = false, illegal = false;
     for (; *ch != '\0'; ch++) {
-        if (*ch == ')') {
+        if (*ch == ')') { // no args
             right_paren = true;
             ch++;
             break;
         }
 
-        bool found = false;
-        for (int i = 0; i < allowed_chars_len; i++) {
-            if (*ch == allowed_chars[i]) {
-                found = true;
-                break;
-            }
-        }
-        if (!found) {
-            illegal = true;
-            break;
-        }
-
-        (*arg_count)++;
-    }
-    if (!right_paren || illegal) return false;
-
-    // 3. check after paren
-    *has_ret = false;
-    if (*ch == '\0') return true;
-
-    *has_ret   = true;
-    bool found = false;
-    for (int i = 0; i < allowed_chars_len; i++) {
-        if (*ch == allowed_chars[i]) {
-            found = true;
-            break;
-        }
-    }
-    if (!found) return false;
-    if (*(ch + 1) != '\0') return false;
-
-    return true;
-}
-
-ffi_cif *create_ffi_cif(DLContext *ctx, const char *signature) {
-    int  arg_count;
-    bool has_ret;
-
-    if (!signature_check(signature, &arg_count, &has_ret)) return NULL;
-
-    ffi_cif   *cif       = ctx->mem.malloc(sizeof(ffi_cif));
-    ffi_type **arg_types = ctx->mem.malloc(sizeof(ffi_type *) * arg_count);
-    ffi_type  *ret_type;
-
-    const char *ch = signature + 1;
-    { // extract args
-        int i = 0;
-        while (*ch != ')') {
         switch (*ch) {
-                case 'i': arg_types[i++] = &ffi_type_sint; break;
+            case 'i': arg_types_tmp[arg_count++] = &ffi_type_sint; break;
-                case 'l': arg_types[i++] = &ffi_type_slong; break;
+            case 'l': arg_types_tmp[arg_count++] = &ffi_type_slong; break;
-                case 'f': arg_types[i++] = &ffi_type_float; break;
+            case 'f': arg_types_tmp[arg_count++] = &ffi_type_float; break;
-                case 'd': arg_types[i++] = &ffi_type_double; break;
+            case 'd': arg_types_tmp[arg_count++] = &ffi_type_double; break;
-                case 'p': arg_types[i++] = &ffi_type_pointer; break;
+            case 'p': arg_types_tmp[arg_count++] = &ffi_type_pointer; break;
-                default: {
+            default: RET_ERR();
-                    LOG_ERR("Invalid signature: %s", signature);
-                    goto fail;
-                }
-            }
-            ch++;
         }
     }
+    if (!right_paren) RET_ERR();
 
-    // ch = signature + 2 + arg_count;
+    // 3. check type_ret?
-    ch++;
+    if (*ch == '\0') { // no ret
-    { // extract ret
+        ret_type = &ffi_type_void;
+    } else {
         switch (*ch) {
             case 'i': ret_type = &ffi_type_sint32; break;
-            case 'l': ret_type = &ffi_type_sint64; break;
+            case 'l': ret_type = &ffi_type_slong; break;
             case 'f': ret_type = &ffi_type_float; break;
             case 'd': ret_type = &ffi_type_double; break;
-            case 'p': ret_type = &ffi_type_pointer; break;
+            default: RET_ERR();
-            case '\0': ret_type = &ffi_type_void; break;
-            default: {
-                LOG_ERR("Invalid signature: %s", signature);
-                goto fail;
-            }
         }
+        if (*(ch + 1) != '\0') RET_ERR();
     }
 
+    // Create ffi_cif and copy arg_types_tmp and ret_type
+    ffi_cif   *cif       = ctx->mem.malloc(sizeof(ffi_cif));
+    ffi_type **arg_types = ctx->mem.malloc(sizeof(ffi_type *) * arg_count);
+
+    for (int i = 0; i < arg_count; i++) arg_types[i] = arg_types_tmp[i];
+
     ffi_status status = ffi_prep_cif(cif, FFI_DEFAULT_ABI, arg_count, ret_type, arg_types);
     if (status != FFI_OK) {
         LOG_ERR("ffi_prep_cif failed: %d", status);
-        goto fail;
-    }
-
-    return cif;
-
-fail:
         ctx->mem.free(arg_types);
         ctx->mem.free(cif);
         return NULL;
     }
 
+    return cif;
+#undef RET_ERR
+}
+
 int dl_sym(wasm_exec_env_t exec_env, int handle, const char *symbol, const char *signature) {
     if (handle < 1 || handle > DL_MAX_HANDLES) return WRT_ERROR;
     if (symbol == NULL || signature == NULL) return WRT_ERROR;
@@ -311,7 +272,7 @@ int64_t dl_call(wasm_exec_env_t exec_env, int symbol, _va_list va_args) {
                 }
                 *ext_ptrs = wasm_runtime_addr_app_to_native(module_inst, ptr);
                 args[i]   = ext_ptrs;
-                LOG_WARN("dl_call: ptr arg[%d] = 0x%lx, buf = %p", i, (uintptr_t)*ext_ptrs,
+                LOG_DBG("dl_call: ptr arg[%d] = 0x%lx, buf = %p", i, (uintptr_t)*ext_ptrs,
                         ext_ptrs);
                 ext_ptrs++;