Page MenuHomePhabricator

[WebAssembly] Error on mismatched function signature in final output
ClosedPublic

Authored by sbc100 on Jun 20 2018, 2:13 PM.

Details

Summary

During symbol resolution, emit warnings for function signature
mismatches. During GC, if any mismatched symbol is marked as live
then generate an error.

This means that we only error out if the mismatch is written to the
final output. i.e. if we would generate an invalid wasm file.

Diff Detail

Repository
rL LLVM

Event Timeline

sbc100 created this revision.Jun 20 2018, 2:13 PM
sbc100 retitled this revision from [WebAssembly] Error if mismatched function signture is includes in final output to [WebAssembly] Error if mismatched function signture is included in final output.Jun 20 2018, 2:14 PM
sbc100 added a reviewer: jgravelle-google.
sbc100 updated this revision to Diff 152187.Jun 20 2018, 3:51 PM
  • rebase
sbc100 updated this revision to Diff 152192.Jun 20 2018, 3:57 PM
  • rebase
sbc100 updated this revision to Diff 152193.Jun 20 2018, 4:00 PM
  • cleanup
sbc100 retitled this revision from [WebAssembly] Error if mismatched function signture is included in final output to [WebAssembly] Error on mismatched function signature in final output .Jun 20 2018, 4:03 PM
sbc100 edited the summary of this revision. (Show Details)
sbc100 edited the summary of this revision. (Show Details)
jgravelle-google accepted this revision.Jun 20 2018, 4:05 PM
jgravelle-google added inline comments.
wasm/SymbolTable.cpp
134 ↗(On Diff #152192)

It feels odd to me that we warn about a mismatch, and then later on error with the same message. Would feel more natural to only print the error, though it's probably useful enough to have the ">>> defind as" context that it's fine as-is.

This revision is now accepted and ready to land.Jun 20 2018, 4:05 PM

Exactly, we don't have the detailed information later on, unless we were to cache it somewhere I guess. Its certainly essential to know where to mismatches are coming from.

This revision was automatically updated to reflect the committed changes.
ruiu added a subscriber: ruiu.Jun 26 2018, 11:55 PM

I'm not sure if this is a good thing to do, because my understanding is that -gc-sections shouldn't generally change the linking semantics except it will produce a smaller binary. With this change, you are now easy to write code that links only when you pass -gc-sections. This is perhaps not problematic for wasm because -gc-section is on by default, but still, I feel like -gc-sections shouldn't do anything more than reducing the output size. What do you think?

lld/trunk/wasm/SymbolTable.cpp
125

msg -> Msg

I'm not sure if this is a good thing to do, because my understanding is that -gc-sections shouldn't generally change the linking semantics except it will produce a smaller binary. With this change, you are now easy to write code that links only when you pass -gc-sections. This is perhaps not problematic for wasm because -gc-section is on by default, but still, I feel like -gc-sections shouldn't do anything more than reducing the output size. What do you think?

Hmm.. I tend to agree. I'm working on some changes that should hopefully make these error less common, and then we should be able to turn it for all functions (even those that get GC'd)

This change was reverted in rL335355