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

aot_loader.c: Fix issues in "Refine interp/aot string storage" (#1102)

Fix issues in PR "Refine interp/aot string storage and emitting (#820)",
which had a few issues:
- It looks a wrong byte to mark the flag
- It doesn't work for long strings (>= 0x80 in case of little endian)

This commit fixes them by maintaining a list of loaded symbols while loading
relocation section to avoid reading a string repeatedly, and no need to mark
the flag again.
YAMAMOTO Takashi 3 лет назад
Родитель
Сommit
b36931a589
1 измененных файлов с 46 добавлено и 44 удалено
  1. 46 44
      core/iwasm/aot/aot_loader.c

+ 46 - 44
core/iwasm/aot/aot_loader.c

@@ -261,48 +261,34 @@ load_string(uint8 **p_buf, const uint8 *buf_end, AOTModule *module,
     char *str;
     uint16 str_len;
 
-    CHECK_BUF(p, p_end, 1);
-    if (*p & 0x80) {
-        /* The string has been adjusted */
-        str = (char *)++p;
-        /* Ensure the whole string is in range */
-        do {
-            CHECK_BUF(p, p_end, 1);
-        } while (*p++ != '\0');
+    read_uint16(p, p_end, str_len);
+    CHECK_BUF(p, p_end, str_len);
+
+    if (str_len == 0) {
+        str = "";
+    }
+    else if (p[str_len - 1] == '\0') {
+        /* The string is terminated with '\0', use it directly */
+        str = (char *)p;
+    }
+    else if (is_load_from_file_buf) {
+        /* As the file buffer can be referred to after loading,
+           we use the 2 bytes of size to adjust the string:
+           move string 2 byte backward and then append '\0' */
+        str = (char *)(p - 2);
+        bh_memmove_s(str, (uint32)(str_len + 1), p, (uint32)str_len);
+        str[str_len] = '\0';
     }
     else {
-        /* The string hasn't been adjusted */
-        read_uint16(p, p_end, str_len);
-        CHECK_BUF(p, p_end, str_len);
-
-        if (str_len == 0) {
-            str = "";
-        }
-        else if (p[str_len - 1] == '\0') {
-            /* The string is terminated with '\0', use it directly */
-            str = (char *)p;
-        }
-        else if (is_load_from_file_buf) {
-            /* As the file buffer can be referred to after loading,
-               we use the 2 bytes of size to adjust the string:
-               mark the flag with the highest bit of size[0],
-               move string 1 byte backward and then append '\0' */
-            *(p - 2) |= 0x80;
-            bh_memmove_s(p - 1, (uint32)(str_len + 1), p, (uint32)str_len);
-            p[str_len - 1] = '\0';
-            str = (char *)(p - 1);
-        }
-        else {
-            /* Load from sections, the file buffer cannot be reffered to
-               after loading, we must create another string and insert it
-               into const string set */
-            if (!(str = const_str_set_insert((uint8 *)p, str_len, module,
-                                             error_buf, error_buf_size))) {
-                goto fail;
-            }
+        /* Load from sections, the file buffer cannot be reffered to
+           after loading, we must create another string and insert it
+           into const string set */
+        if (!(str = const_str_set_insert((uint8 *)p, str_len, module, error_buf,
+                                         error_buf_size))) {
+            goto fail;
         }
-        p += str_len;
     }
+    p += str_len;
 
     *p_buf = p;
     return str;
@@ -2056,6 +2042,7 @@ load_relocation_section(const uint8 *buf, const uint8 *buf_end,
     uint8 *symbol_buf, *symbol_buf_end;
     int map_prot, map_flags;
     bool ret = false;
+    char **symbols = NULL;
 
     read_uint32(buf, buf_end, symbol_count);
 
@@ -2076,6 +2063,12 @@ load_relocation_section(const uint8 *buf, const uint8 *buf_end,
         goto fail;
     }
 
+    symbols = loader_malloc((uint64)sizeof(*symbols) * symbol_count, error_buf,
+                            error_buf_size);
+    if (symbols == NULL) {
+        goto fail;
+    }
+
 #if defined(BH_PLATFORM_WINDOWS)
     buf = symbol_buf_end;
     read_uint32(buf, buf_end, group_count);
@@ -2210,7 +2203,6 @@ load_relocation_section(const uint8 *buf, const uint8 *buf_end,
     for (i = 0, group = groups; i < group_count; i++, group++) {
         AOTRelocation *relocation;
         uint32 name_index;
-        uint8 *name_addr;
 
         /* section name address is 4 bytes aligned. */
         buf = (uint8 *)align_ptr(buf, sizeof(uint32));
@@ -2222,8 +2214,12 @@ load_relocation_section(const uint8 *buf, const uint8 *buf_end,
             goto fail;
         }
 
-        name_addr = symbol_buf + symbol_offsets[name_index];
-        read_string(name_addr, buf_end, group->section_name);
+        if (symbols[name_index] == NULL) {
+            uint8 *name_addr = symbol_buf + symbol_offsets[name_index];
+
+            read_string(name_addr, buf_end, symbols[name_index]);
+        }
+        group->section_name = symbols[name_index];
 
         read_uint32(buf, buf_end, group->relocation_count);
 
@@ -2238,7 +2234,6 @@ load_relocation_section(const uint8 *buf, const uint8 *buf_end,
         /* Load each relocation */
         for (j = 0; j < group->relocation_count; j++, relocation++) {
             uint32 symbol_index;
-            uint8 *symbol_addr;
 
             if (sizeof(void *) == 8) {
                 read_uint64(buf, buf_end, relocation->relocation_offset);
@@ -2260,8 +2255,12 @@ load_relocation_section(const uint8 *buf, const uint8 *buf_end,
                 goto fail;
             }
 
-            symbol_addr = symbol_buf + symbol_offsets[symbol_index];
-            read_string(symbol_addr, buf_end, relocation->symbol_name);
+            if (symbols[symbol_index] == NULL) {
+                uint8 *symbol_addr = symbol_buf + symbol_offsets[symbol_index];
+
+                read_string(symbol_addr, buf_end, symbols[symbol_index]);
+            }
+            relocation->symbol_name = symbols[symbol_index];
         }
 
         if (!strcmp(group->section_name, ".rel.text")
@@ -2327,6 +2326,9 @@ load_relocation_section(const uint8 *buf, const uint8 *buf_end,
     ret = true;
 
 fail:
+    if (symbols) {
+        wasm_runtime_free(symbols);
+    }
     if (groups) {
         for (i = 0, group = groups; i < group_count; i++, group++)
             if (group->relocations)