| Summary: | [JSC] llintTrue / jitTrue can encounter native functions | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||||||
| Component: | New Bugs | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Yusuke Suzuki
2020-06-20 20:26:56 PDT
Created attachment 402414 [details]
Patch
Comment on attachment 402414 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402414&action=review > Source/JavaScriptCore/tools/JSDollarVM.cpp:1798 > + unsigned index = m_currentFrame++; > + // First frame (index 0) is `llintTrue` etc. function itself. > + if (index >= 1) { > + if (visitor->codeBlock()) { > + m_jitType = visitor->codeBlock()->jitType(); > + return StackVisitor::Done; > + } If the caller is not JS code, we continue traversing. I don't think this is meaningful in practice, but keep this semantics as is. Comment on attachment 402414 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402414&action=review r=me. Can you make 2 changes? 1. rename jiTrue() to baselineJITTrue() or baselineTrue() to be consistent with our now current distinction between useJIT() and useBaselineJIT()? 2. enhance your test case to actually verify that $vm.llintTrue() only returns true when the function is a LLint function, and $vm.baselineJITTrue() only returns true if the function is baseline compiled. You can do this verification by doing str = $vm.codeBlockFor(func), and parsing the str for "LLIntFunctionCall", "BaselineFunctionCall", "DFGFunctionCall", or "FTLFunctionCall". >> Source/JavaScriptCore/tools/JSDollarVM.cpp:1798 >> + } > > If the caller is not JS code, we continue traversing. I don't think this is meaningful in practice, but keep this semantics as is. I agree. It should only check the immediate caller. Let's fix it and test accordingly. Created attachment 402676 [details]
Patch
Patch for landing
Created attachment 402677 [details]
Patch
Patch for landing
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\MSBuild\Microsoft\VC\v160\Microsoft.CppCommon.targets(230,5): error MSB6006: "cmd.exe" exited with code 1. [C:\cygwin\home\buildbot\worker\Windows-EWS\build\WebKitBuild\Release\Source\WebCore\WebCoreBindings.vcxproj] Windows failure is unrelated. Debug mac bot is also unrelated remoteFailed: [Failure instance: Traceback (failure with no frames): <class 'twisted.internet.error.ConnectionLost'>: Connection to the other side was lost in a non-clean fashion. ] And it is not using these functions at all. Only thing we should care is jsc bot here. Committed r263483: <https://trac.webkit.org/changeset/263483> Committed r263486: <https://trac.webkit.org/changeset/263486> |