Add custom name section to aot file #794

JavanZhu · 2021-10-20T09:11:14Z

Wamrc will append custom name section to aot file if we set WAMR_BUILD_DUMP_CALL_STACK to 1(which is disable by default),
this makes aot_dump_call_stack() could print function name but no need to use -Wl, --export-all to export all methods(I haven't found some way to export all methods of AssemblyScript module).

I am not sure whether we should extract following duplicate methods, please help me to review it, thanks.

read_leb()
check_utf8_str()
const_str_list_insert()

core/iwasm/aot/aot_runtime.h

wenyongh · 2021-10-21T05:12:40Z

core/iwasm/aot/aot_runtime.h

@@ -223,6 +224,8 @@ typedef struct AOTModule {
    /* constant string set */
    HashMap *const_str_set;

+    StringList const_str_list;


No need to define a new string list, use const_str_set should be enough, and use macro read_string to read and insert the const string to the hash set.

core/iwasm/compilation/aot_emit_aot_file.c

wenyongh · 2021-10-21T05:24:48Z

core/iwasm/compilation/aot_emit_aot_file.c

+        size = align_uint(size, 4);
+        /* section id + section size + sub section id */
+        size += (uint32)sizeof(uint32) * 3;
+        size += comp_data->name_section_buf_end - comp_data->name_section_buf;


Can we decode the function names previously here, and emit these strings with format 2-byte string len + string so that aot loader can just use read_string macro to read string? Ref to emitting of other strings.

core/iwasm/compilation/aot_emit_aot_file.c

wenyongh · 2021-10-21T05:35:00Z

core/iwasm/aot/aot_loader.c

+}
+
+static char *
+const_str_list_insert(const uint8 *str, uint32 len, AOTModule *module,


use read_string macro instead

wenyongh · 2021-10-21T05:39:30Z

core/iwasm/aot/aot_loader.c

+    p += name_len;
+
+    while (p < p_end) {
+        read_leb_uint32(p, p_end, name_type);


just emit decoded uint32 directly, so here we can read uint32 directly, don't add decoding leb codes so as to reduce footprint.

wenyongh · 2021-10-21T05:46:37Z

core/iwasm/aot/aot_loader.c

+
+                    for (name_index = 0; name_index < num_func_name;
+                         name_index++) {
+                        read_leb_uint32(p, p_end, func_index);


Seems that here the func_index is bound to the following function_name, should add an array in AOTModule to store these func indexes? And when looking up, we should traverse the array one by one to lookup the func name of related func index?

wenyongh · 2021-10-21T05:52:27Z

core/iwasm/aot/aot_runtime.c

@@ -2822,6 +2822,12 @@ get_func_name_from_index(const AOTModuleInstance *module_inst,
    const char *func_name = NULL;
    AOTModule *module = module_inst->aot_module.ptr;

+#if WASM_ENABLE_CUSTOM_NAME_SECTION != 0
+    if (module->aux_func_name && func_index < module->aux_func_name_count) {
+        return *(module->aux_func_name + func_index);


Seems invalid, seems that we should also add a func index array in AOTModule, and here lookup it (Just my understanding, to be confirmed):

AOTModule { uint32 aux_func_name_count; uint32 *aux_func_indexes; const char *aux_func_names; }; Here: for (i = 0; i < aux_func_name_count; i++) { if (aux_func_indexes[i] == func_index) return aux_func_names[i]; } Or sort the arrays previously, and here use quick search to lookup the func name.

.gitignore

wenyongh · 2021-10-25T11:35:59Z

core/iwasm/aot/aot_loader.c

+    if (name_len == 0 || p + name_len > p_end) {
+        set_error_buf(error_buf, error_buf_size, "unexpected end");
+        return false;
+    }
+
+    if (!check_utf8_str(p, name_len)) {
+        set_error_buf(error_buf, error_buf_size, "invalid UTF-8 encoding");
+        return false;
+    }
+
+    if (memcmp(p, "name", 4) != 0) {
+        set_error_buf(error_buf, error_buf_size, "invalid custom name section");
+        return false;
+    }
+    p += name_len;


Must the name here be "name" and the name_len be 4? If yes, how about changing the codes to be:

if (name_len != 4 || p + name_len > p_end) { set_error_buf(error_buf, error_buf_size, "unexpected end"); return false; } if (memcmp(p, "name", 4) != 0) { set_error_buf(error_buf, error_buf_size, "invalid custom name section"); return false; } p += name_len;

So that we don't need to call check_utf8_str

wenyongh · 2021-10-25T11:43:59Z

core/iwasm/aot/aot_loader.c

+                    for (name_index = 0; name_index < num_func_name;
+                         name_index++) {
+                        read_uint32(p, p_end, func_index);
+                        if (func_index == previous_func_index) {


Should be name_index != 0 && func_index == previous_func_index? The first func_index can be 0.

wenyongh · 2021-10-25T11:45:18Z

core/iwasm/aot/aot_loader.c

+                            return false;
+                        }
+                        if (func_index < previous_func_index
+                            && previous_func_index != ~0U) {


How about change to if (name_index != 0 && func_index < previous_func_index)?

And had better check whether func_index < module->import_func_count + module->func_count

core/iwasm/aot/aot_loader.c

core/iwasm/compilation/aot_emit_aot_file.c

wenyongh · 2021-10-26T01:04:01Z

core/iwasm/aot/aot_runtime.c

+    for (i = 0; i < module->aux_func_name_count; i++) {
+        if (module->aux_func_indexes[i] == func_index)
+            return module->aux_func_names[i];


Traversing array elements one by one is a little slow, as the elements in aux_func_indexes are sorted, can we use binary search to search the element, so as to improve the performance?

Enable emitting custom name section to aot file when adding `--enable-dump-call-stack` or `--enable-dump-call-stack` to wamrc and there is custom name section in wasm file, which can be generated by wasi-sdk/emcc "-g" option. So aot runtime can also get the function name from the custom name section instead of export section, to which developer should use `--export-all` for wasi-sdk/emcc to generate export function names.

JavanZhu added 3 commits October 20, 2021 17:00

Add custom name section to aot file, Fix some typos, Update .gitignore

0435e61

Fix some typos.

6ab6cc3

Merge branch 'main' of https://github.com/JavanZhu/wasm-micro-runtime

598e8f9

wenyongh reviewed Oct 21, 2021

View reviewed changes

JavanZhu added 8 commits October 22, 2021 10:58

Update.

0b22e34

Fix typo.

d05f09d

Fix.

d3d1480

Format code.

ac4495a

Fix build error.

89331b3

Merge branch 'bytecodealliance:main' into main

6d46299

Replace leb with uint32.

1f49189

Fix compiler warning.

e4db19d

wenyongh reviewed Oct 25, 2021

View reviewed changes

wenyongh reviewed Oct 26, 2021

View reviewed changes

JavanZhu added 2 commits October 26, 2021 10:47

Fix reviews.

a103f0d

Fix review.

1193463

wenyongh merged commit 788e14e into bytecodealliance:main Oct 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add custom name section to aot file #794

Add custom name section to aot file #794

JavanZhu commented Oct 20, 2021

wenyongh Oct 21, 2021

wenyongh Oct 21, 2021

wenyongh Oct 21, 2021

wenyongh Oct 21, 2021

wenyongh Oct 21, 2021

wenyongh Oct 21, 2021

JavanZhu Oct 22, 2021

wenyongh Oct 25, 2021

JavanZhu Oct 26, 2021

wenyongh Oct 25, 2021

wenyongh Oct 25, 2021

wenyongh Oct 25, 2021

JavanZhu Oct 26, 2021

wenyongh Oct 26, 2021

JavanZhu Oct 26, 2021

Add custom name section to aot file #794

Add custom name section to aot file #794

Conversation

JavanZhu commented Oct 20, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment