Bug 166416 – WebAssembly: Make the spec-tests/start.wast.js test pass

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Saam Barati]

Two things are needed:
1. Our start functions are borked if the thing is an import (the current code assumes all imports are exports, which is wrong)
2. Unreachable should throw an exception, not crash.

[Radar WebKit Bug Importer]

<rdar://problem/29784532>

[Saam Barati]

Created attachment 297648 [details]
patch

[Yusuke Suzuki]

Comment on attachment 297648 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=297648&action=review

r=me

> Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:185
>          if (!m_startFunction.get()) {

I think this is always true, right? If so, let's drop this brace.

[JF Bastien]

Comment on attachment 297648 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=297648&action=review

>> Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:185
>>          if (!m_startFunction.get()) {
> 
> I think this is always true, right? If so, let's drop this brace.

Indeed, the erroneous code I had above was the only code which touched m_startFunction. This should now never be set when we get here.

I think you should also move this code down here:
97    bool hasStart = !!moduleInformation.startFunctionIndexSpace;
98    auto startFunctionIndexSpace = moduleInformation.startFunctionIndexSpace.value_or(0);

[Saam Barati]

Comment on attachment 297648 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=297648&action=review

>>> Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:185
>>>          if (!m_startFunction.get()) {
>> 
>> I think this is always true, right? If so, let's drop this brace.
> 
> Indeed, the erroneous code I had above was the only code which touched m_startFunction. This should now never be set when we get here.
> 
> I think you should also move this code down here:
> 97    bool hasStart = !!moduleInformation.startFunctionIndexSpace;
> 98    auto startFunctionIndexSpace = moduleInformation.startFunctionIndexSpace.value_or(0);

Sounds good, I'll make these changes.

[Saam Barati]

landed in:
https://trac.webkit.org/changeset/210102