[lld][WebAssemby] Allow import module names to be empty strings.

[sunfish]

Summary

The component-model [canonical ABI] is currently using import module names with empty strings. Remove the special cases for empty strings from WasmObjectFile.cpp so that they can pass through as-is.

[sbc100]

Did you mean to title this "Allow import names and modules to be empty strings." ?

The change to the code here seems only to relate to the module and the not the name/field of the import, is that right? Were fields already allowed to be empty? In which case maybe the title should just be "Allow empty module names in imports?"

is the canonical ABI using empty fields or empty module names? (presumably one can't use both .. otherwise everything is the same import :)

lld/test/wasm/import-module-empty.ll

14 ↗

(On Diff #457043)

We've been in the process of trying to re-write all wasm-ld tests in assembly for a while now. I think these should be easy enough to write in .s format?

[sbc100]

lgtm with tests converted to asm.

[sbc100]

Actually perhaps we can just make this part of the existing lld/test/wasm/import-module.ll, and lld/test/wasm/import-name.ll tests.. i don't see why they shouldn't test the different edge cases in the single test file (if you do it that way I guess you can avoid re-writing them tests in assembly too :)

[sunfish]

Instead of adding new test files, add tests to existing test files.

[sunfish]

In D133037#3762343, @sbc100 wrote:

The change to the code here seems only to relate to the module and the not the name/field of the import, is that right? Were fields already allowed to be empty? In which case maybe the title should just be "Allow empty module names in imports?"

The actual change is just for module names. Empty fields were already supported. I've now updated the title.

is the canonical ABI using empty fields or empty module names? (presumably one can't use both .. otherwise everything is the same import :)

The canonical ABI is using empty module names for some things.

In D133037#3762383, @sbc100 wrote:

Actually perhaps we can just make this part of the existing lld/test/wasm/import-module.ll, and lld/test/wasm/import-name.ll tests.. i don't see why they shouldn't test the different edge cases in the single test file (if you do it that way I guess you can avoid re-writing them tests in assembly too :)

I've now done this :-).

lld/test/wasm/import-module.ll

 ; RUN: llc -filetype=obj %s -o %t.o
 ; RUN: wasm-ld --allow-undefined -o %t.wasm %t.o
 ; RUN: obj2yaml %t.wasm | FileCheck %s
 
 target triple = "wasm32-unknown-unknown-wasm"
 
 define void @_start() {
   call void @foo();
+  call void @qux();
   ret void
 }
 
 declare void @foo() #0
+declare void @qux() #1
 
 attributes #0 = { "wasm-import-module"="bar" }
+attributes #1 = { "wasm-import-module"="" }
 
 ; CHECK:        - Type:            IMPORT
 ; CHECK-NEXT:     Imports:         
 ; CHECK-NEXT:       - Module:          bar
 ; CHECK-NEXT:         Field:           foo
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         SigIndex:        0
+; CHECK-NEXT:       - Module:          ''
+; CHECK-NEXT:         Field:           qux
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         SigIndex:        0

lld/test/wasm/import-name.ll

 ; RUN: llc -filetype=obj %s -o %t.o
 ; RUN: wasm-ld -o %t.wasm %t.o
 ; RUN: obj2yaml %t.wasm | FileCheck %s
 
 target triple = "wasm32-unknown-unknown"
 
 declare void @f0() #0
+declare void @f1() #1
 
 define void @_start() {
     call void @f0()
+    call void @f1()
     ret void
 }
 
 attributes #0 = { "wasm-import-module"="somewhere" "wasm-import-name"="something" }
+attributes #1 = { "wasm-import-module"="otherwhere" "wasm-import-name"="" }
 
 ; CHECK:        - Type:            IMPORT
 ; CHECK-NEXT:     Imports:
 ; CHECK-NEXT:       - Module:          somewhere
 ; CHECK-NEXT:         Field:           something
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         SigIndex:        0
+; CHECK-NEXT:       - Module:          otherwhere
+; CHECK-NEXT:         Field:           ''
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         SigIndex:        0
 
 ; CHECK:        - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            name
 ; CHECK-NEXT:     FunctionNames:
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Name:            f0
+; CHECK-NEXT:       - Index:           1
+; CHECK-NEXT:         Name:            f1

llvm/lib/Object/WasmObjectFile.cpp

case wasm::WASM_SYMBOL_TYPE_FUNCTION:
         wasm::WasmImport &Import = *ImportedFunctions[Info.ElementIndex];
         if ((Info.Flags & wasm::WASM_SYMBOL_EXPLICIT_NAME) != 0) {
           Info.Name = readString(Ctx);
           Info.ImportName = Import.Field;
         } else {
           Info.Name = Import.Field;
         }
         Signature = &Signatures[Import.SigIndex];
-        if (!Import.Module.empty()) {
-          Info.ImportModule = Import.Module;
-        }
-      }
       break;
 
     case wasm::WASM_SYMBOL_TYPE_GLOBAL:
       Info.ElementIndex = readVaruint32(Ctx);
       if (!isValidGlobalIndex(Info.ElementIndex) ||
           IsDefined != isDefinedGlobalIndex(Info.ElementIndex))
         return make_error<GenericBinaryError>("invalid global symbol index",
                                               object_error::parse_failed);
case wasm::WASM_SYMBOL_TYPE_GLOBAL:
         wasm::WasmImport &Import = *ImportedGlobals[Info.ElementIndex];
         if ((Info.Flags & wasm::WASM_SYMBOL_EXPLICIT_NAME) != 0) {
           Info.Name = readString(Ctx);
           Info.ImportName = Import.Field;
         } else {
           Info.Name = Import.Field;
         }
         GlobalType = &Import.Global;
-        if (!Import.Module.empty()) {
-          Info.ImportModule = Import.Module;
-        }
-      }
       break;
 
     case wasm::WASM_SYMBOL_TYPE_TABLE:
       Info.ElementIndex = readVaruint32(Ctx);
       if (!isValidTableNumber(Info.ElementIndex) ||
           IsDefined != isDefinedTableNumber(Info.ElementIndex))
         return make_error<GenericBinaryError>("invalid table symbol index",
                                               object_error::parse_failed);
case wasm::WASM_SYMBOL_TYPE_TABLE:
         wasm::WasmImport &Import = *ImportedTables[Info.ElementIndex];
         if ((Info.Flags & wasm::WASM_SYMBOL_EXPLICIT_NAME) != 0) {
           Info.Name = readString(Ctx);
           Info.ImportName = Import.Field;
         } else {
           Info.Name = Import.Field;
         }
         TableType = &Import.Table;
-        if (!Import.Module.empty()) {
-          Info.ImportModule = Import.Module;
-        }
-      }
       break;
 
     case wasm::WASM_SYMBOL_TYPE_DATA:
       Info.Name = readString(Ctx);
       if (IsDefined) {
         auto Index = readVaruint32(Ctx);
         if (Index >= DataSegments.size())
           return make_error<GenericBinaryError>("invalid data symbol index",
case wasm::WASM_SYMBOL_TYPE_TAG: {
         wasm::WasmImport &Import = *ImportedTags[Info.ElementIndex];
         if ((Info.Flags & wasm::WASM_SYMBOL_EXPLICIT_NAME) != 0) {
           Info.Name = readString(Ctx);
           Info.ImportName = Import.Field;
         } else {
           Info.Name = Import.Field;
         }
         Signature = &Signatures[Import.SigIndex];
-        if (!Import.Module.empty()) {
-          Info.ImportModule = Import.Module;
-        }
-      }
       break;
     }
 
     default:
       return make_error<GenericBinaryError>("invalid symbol type: " +
                                                 Twine(unsigned(Info.Kind)),
                                             object_error::parse_failed);
     }