[WebAssembly] Fix bug is function signature checking

[sbc100]

Summary

This bug effected undefined symbols that were resolved by
existing defined symbols. We were skipping the signature
check in this case.

[sbc100]

• revert

[sbc100]

• revert

[sbc100]

• typo

[ruiu]

wasm/Symbols.h

97

This class have setFunctionType and getFunctionType, but this variance doesn't seem to be guaranteed

Obj.setFunctionType(X);
assert(X == Obj.getFunctionType());

because a value given to setFunctionType is ignored if this class have a non-null Chunk. Isn't that confusing?

[sbc100]

wasm/Symbols.h

97

setFunctionType has an assert(!Chunk) which i think covers that case.

[jgravelle-google]

Function-bitcast stuff looks good to me

[ruiu]

If we can guarantee the invariance by data structure itself, that's better than guaranteeing the same thing by code (which is asserts). Does it make sense to read function type on symbol instantiation time?

[sbc100]

In D43399#1010561, @ruiu wrote:

If we can guarantee the invariance by data structure itself, that's better than guaranteeing the same thing by code (which is asserts). Does it make sense to read function type on symbol instantiation time?

Oh I see, so getFunction() can then just return the member, rather than looking at the chunk. Sure I think that makes sense. Can we do that a separate change though?

[ruiu]

LGTM. Yes you can. :)

[ncw]

That's great, thanks for splitting it out. The longer-term fix I think is to just set FunctionType always (for defined and undefined symbols), rather than have the confusion of a member that's sometimes meaningful and sometimes not; less error-prone to keep the values of all the members in sync with each other (where meaningful).

test/wasm/init-fini.ll

entry:
   ret void
 }
 
 define hidden void @func4() {
 entry:
   ret void
 }
 
-define void @__cxa_atexit() {
-  ret void
+define i32 @__cxa_atexit(i32 %func, i32 %arg, i32 %dso_handle) {
+  ret i32 0
 }
 
 define hidden void @_start() {
 entry:
   ret void
 }
 
 @llvm.global_ctors = appending global [3 x { i32, void ()*, i8* }] [

 ; RUN: lld -flavor wasm --check-signatures %t.o %t.global-ctor-dtor.o -o %t.wasm
 ; RUN: obj2yaml %t.wasm | FileCheck %s
 
 ; CHECK:        - Type:            ELEM
 ; CHECK-NEXT:     Segments:
 ; CHECK-NEXT:       - Offset:
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           1
-; CHECK-NEXT:         Functions:       [ 6, 9, 13, 15, 17 ]
+; CHECK-NEXT:         Functions:       [ 6, 8, 12, 14, 16 ]
 
-; CHECK:              Body:            100010011007100B100E100B10101000100A100B10120B
+; CHECK:              Body:            100010011007100A100D100A100F10001009100A10110B
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            linking
 ; CHECK-NEXT:     DataSize:        0
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            name
 ; CHECK-NEXT:     FunctionNames:
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Name:            func1
 ; CHECK-NEXT:       - Index:           1
 ; CHECK-NEXT:         Name:            func2
 ; CHECK-NEXT:       - Index:           2
 ; CHECK-NEXT:         Name:            func3
 ; CHECK-NEXT:       - Index:           3
 ; CHECK-NEXT:         Name:            func4
 ; CHECK-NEXT:       - Index:           4
 ; CHECK-NEXT:         Name:            __cxa_atexit
 ; CHECK-NEXT:       - Index:           5
 ; CHECK-NEXT:         Name:            _start
 ; CHECK-NEXT:       - Index:           6
 ; CHECK-NEXT:         Name:            .Lcall_dtors.101
 ; CHECK-NEXT:       - Index:           7
 ; CHECK-NEXT:         Name:            .Lregister_call_dtors.101
 ; CHECK-NEXT:       - Index:           8
-; CHECK-NEXT:         Name:            .Lbitcast
-; CHECK-NEXT:       - Index:           9
 ; CHECK-NEXT:         Name:            .Lcall_dtors.1001
-; CHECK-NEXT:       - Index:           10
+; CHECK-NEXT:       - Index:           9
 ; CHECK-NEXT:         Name:            .Lregister_call_dtors.1001
-; CHECK-NEXT:       - Index:           11
+; CHECK-NEXT:       - Index:           10
 ; CHECK-NEXT:         Name:            myctor
-; CHECK-NEXT:       - Index:           12
+; CHECK-NEXT:       - Index:           11
 ; CHECK-NEXT:         Name:            mydtor
-; CHECK-NEXT:       - Index:           13
+; CHECK-NEXT:       - Index:           12
 ; CHECK-NEXT:         Name:            .Lcall_dtors.101
-; CHECK-NEXT:       - Index:           14
+; CHECK-NEXT:       - Index:           13
 ; CHECK-NEXT:         Name:            .Lregister_call_dtors.101
-; CHECK-NEXT:       - Index:           15
+; CHECK-NEXT:       - Index:           14
 ; CHECK-NEXT:         Name:            .Lcall_dtors.202
-; CHECK-NEXT:       - Index:           16
+; CHECK-NEXT:       - Index:           15
 ; CHECK-NEXT:         Name:            .Lregister_call_dtors.202
-; CHECK-NEXT:       - Index:           17
+; CHECK-NEXT:       - Index:           16
 ; CHECK-NEXT:         Name:            .Lcall_dtors.2002
-; CHECK-NEXT:       - Index:           18
+; CHECK-NEXT:       - Index:           17
 ; CHECK-NEXT:         Name:            .Lregister_call_dtors.2002
-; CHECK-NEXT:       - Index:           19
+; CHECK-NEXT:       - Index:           18
 ; CHECK-NEXT:         Name:            __wasm_call_ctors
 ; CHECK-NEXT: ...
 
 
 ; RUN: lld -flavor wasm --check-signatures -r %t.o %t.global-ctor-dtor.o -o %t.reloc.wasm
 ; RUN: obj2yaml %t.reloc.wasm | FileCheck -check-prefix=RELOC %s
 
 ; RELOC:          Name:            linking

 ; RELOC-NEXT:       - Name:            func4
 ; RELOC-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
 ; RELOC-NEXT:       - Name:            _start
 ; RELOC-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
 ; RELOC-NEXT:       - Name:            .Lcall_dtors.101
 ; RELOC-NEXT:         Flags:           [ BINDING_LOCAL ]
 ; RELOC-NEXT:       - Name:            .Lregister_call_dtors.101
 ; RELOC-NEXT:         Flags:           [ BINDING_LOCAL ]
-; RELOC-NEXT:       - Name:            .Lbitcast
-; RELOC-NEXT:         Flags:           [ BINDING_LOCAL ]
 ; RELOC-NEXT:       - Name:            .Lcall_dtors.1001
 ; RELOC-NEXT:         Flags:           [ BINDING_LOCAL ]
 ; RELOC-NEXT:       - Name:            .Lregister_call_dtors.1001
 ; RELOC-NEXT:         Flags:           [ BINDING_LOCAL ]
 ; RELOC-NEXT:       - Name:            myctor
 ; RELOC-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
 ; RELOC-NEXT:       - Name:            mydtor
 ; RELOC-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]

 ; RELOC-NEXT:     InitFunctions:
 ; RELOC-NEXT:       - Priority:        101
 ; RELOC-NEXT:         FunctionIndex:   0
 ; RELOC-NEXT:       - Priority:        101
 ; RELOC-NEXT:         FunctionIndex:   1
 ; RELOC-NEXT:       - Priority:        101
 ; RELOC-NEXT:         FunctionIndex:   7
 ; RELOC-NEXT:       - Priority:        101
-; RELOC-NEXT:         FunctionIndex:   11
+; RELOC-NEXT:         FunctionIndex:   10
 ; RELOC-NEXT:       - Priority:        101
-; RELOC-NEXT:         FunctionIndex:   14
+; RELOC-NEXT:         FunctionIndex:   13
 ; RELOC-NEXT:       - Priority:        202
-; RELOC-NEXT:         FunctionIndex:   11
+; RELOC-NEXT:         FunctionIndex:   10
 ; RELOC-NEXT:       - Priority:        202
-; RELOC-NEXT:         FunctionIndex:   16
+; RELOC-NEXT:         FunctionIndex:   15
 ; RELOC-NEXT:       - Priority:        1001
 ; RELOC-NEXT:         FunctionIndex:   0
 ; RELOC-NEXT:       - Priority:        1001
-; RELOC-NEXT:         FunctionIndex:   10
+; RELOC-NEXT:         FunctionIndex:   9
 ; RELOC-NEXT:       - Priority:        2002
-; RELOC-NEXT:         FunctionIndex:   11
+; RELOC-NEXT:         FunctionIndex:   10
 ; RELOC-NEXT:       - Priority:        2002
-; RELOC-NEXT:         FunctionIndex:   18
+; RELOC-NEXT:         FunctionIndex:   17
 ; RELOC-NEXT:   - Type:            CUSTOM
 ; RELOC-NEXT:     Name:            name
 ; RELOC-NEXT:     FunctionNames:
 ; RELOC-NEXT:       - Index:           0
 ; RELOC-NEXT:         Name:            func1
 ; RELOC-NEXT:       - Index:           1
 ; RELOC-NEXT:         Name:            func2
 ; RELOC-NEXT:       - Index:           2
 ; RELOC-NEXT:         Name:            func3
 ; RELOC-NEXT:       - Index:           3
 ; RELOC-NEXT:         Name:            func4
 ; RELOC-NEXT:       - Index:           4
 ; RELOC-NEXT:         Name:            __cxa_atexit
 ; RELOC-NEXT:       - Index:           5
 ; RELOC-NEXT:         Name:            _start
 ; RELOC-NEXT:       - Index:           6
 ; RELOC-NEXT:         Name:            .Lcall_dtors.101
 ; RELOC-NEXT:       - Index:           7
 ; RELOC-NEXT:         Name:            .Lregister_call_dtors.101
 ; RELOC-NEXT:       - Index:           8
-; RELOC-NEXT:         Name:            .Lbitcast
-; RELOC-NEXT:       - Index:           9
 ; RELOC-NEXT:         Name:            .Lcall_dtors.1001
-; RELOC-NEXT:       - Index:           10
+; RELOC-NEXT:       - Index:           9
 ; RELOC-NEXT:         Name:            .Lregister_call_dtors.1001
-; RELOC-NEXT:       - Index:           11
+; RELOC-NEXT:       - Index:           10
 ; RELOC-NEXT:         Name:            myctor
-; RELOC-NEXT:       - Index:           12
+; RELOC-NEXT:       - Index:           11
 ; RELOC-NEXT:         Name:            mydtor
-; RELOC-NEXT:       - Index:           13
+; RELOC-NEXT:       - Index:           12
 ; RELOC-NEXT:         Name:            .Lcall_dtors.101
-; RELOC-NEXT:       - Index:           14
+; RELOC-NEXT:       - Index:           13
 ; RELOC-NEXT:         Name:            .Lregister_call_dtors.101
-; RELOC-NEXT:       - Index:           15
+; RELOC-NEXT:       - Index:           14
 ; RELOC-NEXT:         Name:            .Lcall_dtors.202
-; RELOC-NEXT:       - Index:           16
+; RELOC-NEXT:       - Index:           15
 ; RELOC-NEXT:         Name:            .Lregister_call_dtors.202
-; RELOC-NEXT:       - Index:           17
+; RELOC-NEXT:       - Index:           16
 ; RELOC-NEXT:         Name:            .Lcall_dtors.2002
-; RELOC-NEXT:       - Index:           18
+; RELOC-NEXT:       - Index:           17
 ; RELOC-NEXT:         Name:            .Lregister_call_dtors.2002
 ; RELOC-NEXT: ...

test/wasm/signature-mismatch.ll

 ; RUN: llc -filetype=obj %p/Inputs/ret32.ll -o %t.ret32.o
 ; RUN: llc -filetype=obj %s -o %t.main.o
 ; RUN: not lld -flavor wasm --check-signatures -o %t.wasm %t.main.o %t.ret32.o 2>&1 | FileCheck %s
+; Run the test again by with the object files in the other order to verify
+; the check works when the undefined symbol is resolved by an existing defined
+; one.
+; RUN: not lld -flavor wasm --check-signatures -o %t.wasm %t.ret32.o %t.main.o 2>&1 | FileCheck %s -check-prefix=REVERSE
 
 target triple = "wasm32-unknown-unknown-wasm"
 
 ; Function Attrs: nounwind
 define hidden void @_start() local_unnamed_addr #0 {
 entry:
   %call = tail call i32 @ret32(i32 1, i64 2, i32 3) #2
   ret void
 }
 
 declare i32 @ret32(i32, i64, i32) local_unnamed_addr #1
 
 ; CHECK: error: function signature mismatch: ret32
 ; CHECK-NEXT: >>> defined as (I32, I64, I32) -> I32 in {{.*}}.main.o
 ; CHECK-NEXT: >>> defined as (F32) -> I32 in {{.*}}.ret32.o
+
+; REVERSE: error: function signature mismatch: ret32
+; REVERSE-NEXT: >>> defined as (F32) -> I32 in {{.*}}.ret32.o
+; REVERSE-NEXT: >>> defined as (I32, I64, I32) -> I32 in {{.*}}.main.o

wasm/Symbols.h

 
 class FunctionSymbol : public Symbol {
 public:
   static bool classof(const Symbol *S) {
     return S->kind() == DefinedFunctionKind ||
            S->kind() == UndefinedFunctionKind;
   }
 
-  bool hasFunctionType() const { return FunctionType != nullptr; }
+  bool hasFunctionType() const { return Chunk || FunctionType; }
   const WasmSignature &getFunctionType() const;
 
   uint32_t getTableIndex() const;
 
   // Returns true if a table index has been set for this symbol
   bool hasTableIndex() const;
 
   // Set the table index of the symbol