Create trap for error message when wasm_instance_new fails (#1751)

Create trap for error message when wasm_instance_new fails:
- Similar to [this PR](#1526),
   but create a wasm_trap_t to output the error msg instead of adding error_buf to the API.
- Trap will need to be deleted by the caller but is not a breaking change as it is only
   created if trap is not NULL.
- Add error messages for all failure cases here, try to make them accurate but welcome
  feedback for improvements.

Signed-off-by: Andrew Chambers <[email protected]>

Author: nchamzn

core/iwasm/common/wasm_c_api.c

@@ -4444,25 +4444,25 @@ aot_process_export(wasm_store_t *store, const AOTModuleInstance *inst_aot,
 
 wasm_instance_t *
 wasm_instance_new(wasm_store_t *store, const wasm_module_t *module,
-                  const wasm_extern_vec_t *imports, own wasm_trap_t **traps)
+                  const wasm_extern_vec_t *imports, own wasm_trap_t **trap)
 {
-    return wasm_instance_new_with_args(store, module, imports, traps,
+    return wasm_instance_new_with_args(store, module, imports, trap,
                                        KILOBYTE(32), KILOBYTE(32));
 }
 
 wasm_instance_t *
 wasm_instance_new_with_args(wasm_store_t *store, const wasm_module_t *module,
                             const wasm_extern_vec_t *imports,
-                            own wasm_trap_t **traps, const uint32 stack_size,
+                            own wasm_trap_t **trap, const uint32 stack_size,
                             const uint32 heap_size)
 {
-    char error_buf[128] = { 0 };
+    char sub_error_buf[128] = { 0 };
+    char error_buf[256] = { 0 };
     uint32 import_count = 0;
     bool import_count_verified = false;
     wasm_instance_t *instance = NULL;
     uint32 i = 0;
     bool processed = false;
-    (void)traps;
 
     bh_assert(singleton_engine);
 
@@ -4474,6 +4474,8 @@ wasm_instance_new_with_args(wasm_store_t *store, const wasm_module_t *module,
 
     instance = malloc_internal(sizeof(wasm_instance_t));
     if (!instance) {
+        snprintf(sub_error_buf, sizeof(sub_error_buf),
+                 "Failed to malloc instance");
         goto failed;
     }
 
@@ -4490,6 +4492,8 @@ wasm_instance_new_with_args(wasm_store_t *store, const wasm_module_t *module,
                 /* make sure a complete import list */
                 if ((int32)import_count < 0
                     || import_count != actual_link_import_count) {
+                    snprintf(sub_error_buf, sizeof(sub_error_buf),
+                             "Failed to validate imports");
                     goto failed;
                 }
             }
@@ -4508,6 +4512,8 @@ wasm_instance_new_with_args(wasm_store_t *store, const wasm_module_t *module,
                 import_count = aot_link(instance, MODULE_AOT(module),
                                         (wasm_extern_t **)imports->data);
                 if ((int32)import_count < 0) {
+                    snprintf(sub_error_buf, sizeof(sub_error_buf),
+                             "Failed to validate imports");
                     goto failed;
                 }
             }
@@ -4520,18 +4526,21 @@ wasm_instance_new_with_args(wasm_store_t *store, const wasm_module_t *module,
          * also leads to below branch
          */
         if (!import_count_verified) {
+            snprintf(sub_error_buf, sizeof(sub_error_buf),
+                     "Failed to verify import count");
             goto failed;
         }
     }
 
     instance->inst_comm_rt = wasm_runtime_instantiate(
-        *module, stack_size, heap_size, error_buf, sizeof(error_buf));
+        *module, stack_size, heap_size, sub_error_buf, sizeof(sub_error_buf));
     if (!instance->inst_comm_rt) {
-        LOG_ERROR(error_buf);
         goto failed;
     }
 
     if (!wasm_runtime_create_exec_env_singleton(instance->inst_comm_rt)) {
+        snprintf(sub_error_buf, sizeof(sub_error_buf),
+                 "Failed to create exec env singleton");
         goto failed;
     }
 
@@ -4556,6 +4565,8 @@ wasm_instance_new_with_args(wasm_store_t *store, const wasm_module_t *module,
                     instance->inst_comm_rt;
                 break;
             default:
+                snprintf(sub_error_buf, sizeof(sub_error_buf),
+                         "Unknown import kind");
                 goto failed;
         }
     }
@@ -4572,6 +4583,8 @@ wasm_instance_new_with_args(wasm_store_t *store, const wasm_module_t *module,
         if (!interp_process_export(store,
                                    (WASMModuleInstance *)instance->inst_comm_rt,
                                    instance->exports)) {
+            snprintf(sub_error_buf, sizeof(sub_error_buf),
+                     "Interpreter failed to process exports");
             goto failed;
         }
 
@@ -4594,6 +4607,8 @@ wasm_instance_new_with_args(wasm_store_t *store, const wasm_module_t *module,
         if (!aot_process_export(store,
                                 (AOTModuleInstance *)instance->inst_comm_rt,
                                 instance->exports)) {
+            snprintf(sub_error_buf, sizeof(sub_error_buf),
+                     "AOT failed to process exports");
             goto failed;
         }
 
@@ -4606,11 +4621,15 @@ wasm_instance_new_with_args(wasm_store_t *store, const wasm_module_t *module,
      * leads to below branch
      */
     if (!processed) {
+        snprintf(sub_error_buf, sizeof(sub_error_buf),
+                 "Incorrect filetype and compilation flags");
         goto failed;
     }
 
     /* add it to a watching list in store */
     if (!bh_vector_append((Vector *)store->instances, &instance)) {
+        snprintf(sub_error_buf, sizeof(sub_error_buf),
+                 "Failed to add to store instances");
         goto failed;
     }
 
@@ -4619,7 +4638,15 @@ wasm_instance_new_with_args(wasm_store_t *store, const wasm_module_t *module,
     return instance;
 
 failed:
-    LOG_DEBUG("%s failed", __FUNCTION__);
+    snprintf(error_buf, sizeof(error_buf), "%s failed: %s", __FUNCTION__,
+             sub_error_buf);
+    if (trap != NULL) {
+        wasm_message_t message = { 0 };
+        wasm_name_new_from_string(&message, error_buf);
+        *trap = wasm_trap_new(store, &message);
+        wasm_byte_vec_delete(&message);
+    }
+    LOG_DEBUG(error_buf);
     wasm_instance_delete_internal(instance);
     return NULL;
 }

core/iwasm/include/wasm_c_api.h

@@ -592,13 +592,13 @@ WASM_DECLARE_REF(instance)
 
 WASM_API_EXTERN own wasm_instance_t* wasm_instance_new(
   wasm_store_t*, const wasm_module_t*, const wasm_extern_vec_t *imports,
-  own wasm_trap_t**
+  own wasm_trap_t** trap
 );
 
 // please refer to wasm_runtime_instantiate(...) in core/iwasm/include/wasm_export.h
 WASM_API_EXTERN own wasm_instance_t* wasm_instance_new_with_args(
   wasm_store_t*, const wasm_module_t*, const wasm_extern_vec_t *imports,
-  own wasm_trap_t**, const uint32_t stack_size, const uint32_t heap_size
+  own wasm_trap_t** trap, const uint32_t stack_size, const uint32_t heap_size
 );
 
 WASM_API_EXTERN void wasm_instance_exports(const wasm_instance_t*, own wasm_extern_vec_t* out);