name function imports using name section (#1290)

Author: kripken

src/wasm-binary.h

@@ -826,8 +826,6 @@ class WasmBinaryBuilder {
   void readMemory();
   void readSignatures();
 
-  std::vector<Name> functionImportIndexes; // index in function index space => name of function import
-
   // gets a name in the combined function import+defined function space
   Name getFunctionIndexName(Index i);
   void getResizableLimits(Address& initial, Address& max, bool& shared, Address defaultIfNoMax);
@@ -844,7 +842,9 @@ class WasmBinaryBuilder {
 
   // We read functions before we know their names, so we need to backpatch the names later
   std::vector<Function*> functions; // we store functions here before wasm.addFunction after we know their names
+  std::vector<Import*> functionImports; // we store function imports here before wasm.addFunctionImport after we know their names
   std::map<Index, std::vector<Call*>> functionCalls; // at index i we have all calls to the defined function i
+  std::map<Index, std::vector<CallImport*>> functionImportCalls; // at index i we have all callImports to the imported function i
   Function* currFunction = nullptr;
   Index endOfFunction = -1; // before we see a function (like global init expressions), there is no end of function to check
 

src/wasm/wasm-binary.cpp

@@ -1486,12 +1486,12 @@ void WasmBinaryBuilder::readSignatures() {
 }
 
 Name WasmBinaryBuilder::getFunctionIndexName(Index i) {
-  if (i < functionImportIndexes.size()) {
-    auto* import = wasm.getImport(functionImportIndexes[i]);
+  if (i < functionImports.size()) {
+    auto* import = functionImports[i];
     assert(import->kind == ExternalKind::Function);
     return import->name;
   } else {
-    i -= functionImportIndexes.size();
+    i -= functionImports.size();
     if (i >= wasm.functions.size()) {
       throw ParseException("bad function index");
     }
@@ -1529,7 +1529,8 @@ void WasmBinaryBuilder::readImports() {
         }
         curr->functionType = wasm.functionTypes[index]->name;
         assert(curr->functionType.is());
-        functionImportIndexes.push_back(curr->name);
+        functionImports.push_back(curr);
+        continue; // don't add the import yet, we add them later after we know their names
         break;
       }
       case ExternalKind::Table: {
@@ -1962,10 +1963,14 @@ Name WasmBinaryBuilder::getGlobalName(Index index) {
 }
 
 void WasmBinaryBuilder::processFunctions() {
-  for (auto& func : functions) {
+  for (auto* func : functions) {
     wasm.addFunction(func);
   }
 
+  for (auto* import : functionImports) {
+    wasm.addImport(import);
+  }
+
   // we should have seen all the functions
   // we assume this later down in fact, when we read wasm.functions[index],
   // as index was validated vs functionTypes.size()
@@ -2002,6 +2007,14 @@ void WasmBinaryBuilder::processFunctions() {
     }
   }
 
+  for (auto& iter : functionImportCalls) {
+    size_t index = iter.first;
+    auto& calls = iter.second;
+    for (auto* call : calls) {
+      call->target = functionImports[index]->name;
+    }
+  }
+
   for (auto& pair : functionTable) {
     auto i = pair.first;
     auto& indexes = pair.second;
@@ -2076,18 +2089,16 @@ void WasmBinaryBuilder::readNames(size_t payloadLen) {
       continue;
     }
     auto num = getU32LEB();
-    uint32_t importedFunctions = 0;
-    for (auto& import : wasm.imports) {
-      if (import->kind != ExternalKind::Function) continue;
-      importedFunctions++;
-    }
     for (size_t i = 0; i < num; i++) {
       auto index = getU32LEB();
-      if (index < importedFunctions) {
-        getInlineString(); // TODO: use this
-      } else if (index - importedFunctions < functions.size()) {
-        auto name = getInlineString();
-        functions[index - importedFunctions]->name = name;
+      auto name = getInlineString();
+      // note: we silently ignore errors here, as name section errors
+      //       are not fatal. should we warn?
+      auto numFunctionImports = functionImports.size();
+      if (index < numFunctionImports) {
+        functionImports[index]->name = name;
+      } else if (index - numFunctionImports < functions.size()) {
+        functions[index - numFunctionImports]->name = name;
       }
     }
     // disallow duplicate names
@@ -2359,19 +2370,20 @@ Expression* WasmBinaryBuilder::visitCall() {
   auto index = getU32LEB();
   FunctionType* type;
   Expression* ret;
-  if (index < functionImportIndexes.size()) {
+  if (index < functionImports.size()) {
     // this is a call of an imported function
     auto* call = allocator.alloc<CallImport>();
-    auto* import = wasm.getImport(functionImportIndexes[index]);
-    call->target = import->name;
+    auto* import = functionImports[index];
     type = wasm.getFunctionType(import->functionType);
+    functionImportCalls[index].push_back(call);
+    call->target = import->name; // name section may modify it
     fillCall(call, type);
     call->finalize();
     ret = call;
   } else {
     // this is a call of a defined function
     auto* call = allocator.alloc<Call>();
-    auto adjustedIndex = index - functionImportIndexes.size();
+    auto adjustedIndex = index - functionImports.size();
     if (adjustedIndex >= functionTypes.size()) {
       throw ParseException("bad call index");
     }

test/dylib.wasm.fromBinary

@@ -3,10 +3,10 @@
  (type $1 (func (result i32)))
  (type $2 (func))
  (import "env" "memoryBase" (global $import$0 i32))
- (import "env" "_puts" (func $import$1 (param i32) (result i32)))
  (import "env" "memory" (memory $0 256))
  (import "env" "table" (table 0 anyfunc))
  (import "env" "tableBase" (global $import$4 i32))
+ (import "env" "_puts" (func $import$1 (param i32) (result i32)))
  (global $global$0 (mut i32) (i32.const 0))
  (global $global$1 (mut i32) (i32.const 0))
  (global $global$2 i32 (i32.const 0))

test/merge/dylib.wasm.combined

@@ -7,10 +7,10 @@
  (type $1$0 (func (result i32)))
  (type $2$0 (func))
  (import "env" "memoryBase" (global $import$0 i32))
- (import "env" "_puts" (func $import$1 (param i32) (result i32)))
  (import "env" "memory" (memory $0 256))
  (import "env" "table" (table 0 anyfunc))
  (import "env" "tableBase" (global $import$4 i32))
+ (import "env" "_puts" (func $import$1 (param i32) (result i32)))
  (import "env" "memoryBase" (global $import$0$0 i32))
  (import "env" "tableBase" (global $import$4$0 i32))
  (global $global$0 (mut i32) (i32.const 0))

test/merge/dylib.wasm.combined.finalized

@@ -7,10 +7,10 @@
  (type $1$0 (func (result i32)))
  (type $2$0 (func))
  (import "env" "memoryBase" (global $import$0 i32))
- (import "env" "_puts" (func $import$1 (param i32) (result i32)))
  (import "env" "memory" (memory $0 256))
  (import "env" "table" (table 8 anyfunc))
  (import "env" "tableBase" (global $import$4 i32))
+ (import "env" "_puts" (func $import$1 (param i32) (result i32)))
  (import "env" "memoryBase" (global $import$0$0 i32))
  (import "env" "tableBase" (global $import$4$0 i32))
  (global $global$0 (mut i32) (i32.const 0))

test/merge/dylib.wasm.combined.finalized.opt

@@ -2,9 +2,9 @@
  (type $1 (func (param i32) (result i32)))
  (type $2 (func (result i32)))
  (type $3 (func))
- (import "env" "_puts" (func $import$1 (param i32) (result i32)))
  (import "env" "memory" (memory $0 256))
  (import "env" "table" (table 8 anyfunc))
+ (import "env" "_puts" (func $import$1 (param i32) (result i32)))
  (global $global$0 (mut i32) (i32.const 0))
  (global $global$1 (mut i32) (i32.const 0))
  (global $global$2 i32 (i32.const 0))

test/merge/dylib.wasm.combined.opt

@@ -3,10 +3,10 @@
  (type $2 (func (result i32)))
  (type $3 (func))
  (import "env" "memoryBase" (global $import$0 i32))
- (import "env" "_puts" (func $import$1 (param i32) (result i32)))
  (import "env" "memory" (memory $0 256))
  (import "env" "table" (table 0 anyfunc))
  (import "env" "tableBase" (global $import$4 i32))
+ (import "env" "_puts" (func $import$1 (param i32) (result i32)))
  (import "env" "memoryBase" (global $import$0$0 i32))
  (global $global$0 (mut i32) (i32.const 0))
  (global $global$1 (mut i32) (i32.const 0))

test/unit.wast.fromBinary

@@ -9,9 +9,9 @@
  (type $7 (func (param f64) (result f64)))
  (type $8 (func (result i64)))
  (type $9 (func (param i32 i64)))
- (import "env" "_emscripten_asm_const_vi" (func $import$0))
- (import "asm2wasm" "f64-to-int" (func $import$1 (param f64) (result i32)))
- (import "asm2wasm" "f64-rem" (func $import$2 (param f64 f64) (result f64)))
+ (import "env" "_emscripten_asm_const_vi" (func $_emscripten_asm_const_vi))
+ (import "asm2wasm" "f64-to-int" (func $f64-to-int (param f64) (result i32)))
+ (import "asm2wasm" "f64-rem" (func $f64-rem (param f64 f64) (result f64)))
  (table 10 anyfunc)
  (elem (i32.const 0) $z $big_negative $z $z $w $w $importedDoubles $w $z $cneg)
  (memory $0 4096 4096)
@@ -153,7 +153,7 @@
   (local $var$0 i32)
   (local $var$1 f64)
   (set_local $var$0
-   (call $import$1
+   (call $f64-to-int
     (get_local $var$1)
    )
   )
@@ -274,7 +274,7 @@
   )
  )
  (func $frem (; 12 ;) (type $4) (result f64)
-  (call $import$2
+  (call $f64-rem
    (f64.const 5.5)
    (f64.const 1.2)
   )