Bug 170612

Summary: WebAssembly: Module::getOrCreateCodeBlock is wrong
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, fpizlo, ggaren, gskachkov, jfbastien, keith_miller, mark.lam, msaboff, ticaiolima, ysuzuki
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
none
patch keith_miller: review+

Saam Barati
Reported 2017-04-07 12:07:49 PDT
It needs to check something that isn't runnable()
Attachments
patch (2.69 KB, patch)
2017-04-07 12:14 PDT, Saam Barati
no flags
patch (1.74 KB, patch)
2017-04-07 12:33 PDT, Saam Barati
keith_miller: review+
Saam Barati
Comment 1 2017-04-07 12:14:09 PDT
JF Bastien
Comment 2 2017-04-07 12:20:21 PDT
Comment on attachment 306526 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=306526&action=review > Source/JavaScriptCore/wasm/WasmCodeBlock.h:65 > + return false; "don't know" shouldn't be false? Can you use an optional instead?
Keith Miller
Comment 3 2017-04-07 12:21:30 PDT
Comment on attachment 306526 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=306526&action=review >> Source/JavaScriptCore/wasm/WasmCodeBlock.h:65 >> + return false; > > "don't know" shouldn't be false? Can you use an optional instead? I think we should just use a TriState.
Saam Barati
Comment 4 2017-04-07 12:24:38 PDT
Comment on attachment 306526 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=306526&action=review >>> Source/JavaScriptCore/wasm/WasmCodeBlock.h:65 >>> + return false; >> >> "don't know" shouldn't be false? Can you use an optional instead? > > I think we should just use a TriState. Maybe I'll just make the caller do this. Since we just have one caller. if (!codeBlock || (codeBlock->compilationFinished() && !codeBlock->runnable())
Saam Barati
Comment 5 2017-04-07 12:33:19 PDT
Keith Miller
Comment 6 2017-04-07 12:36:07 PDT
Comment on attachment 306527 [details] patch r=me.
Saam Barati
Comment 7 2017-04-07 12:43:16 PDT
Note You need to log in before you can comment on or make changes to this bug.