Bug 213442 - [JSC] llintTrue / jitTrue can encounter native functions
Summary: [JSC] llintTrue / jitTrue can encounter native functions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-06-20 20:26 PDT by Yusuke Suzuki
Modified: 2020-06-24 17:19 PDT (History)
7 users (show)

See Also:


Attachments
Patch (3.25 KB, patch)
2020-06-20 20:27 PDT, Yusuke Suzuki
mark.lam: review+
Details | Formatted Diff | Diff
Patch (6.34 KB, patch)
2020-06-24 13:12 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (6.41 KB, patch)
2020-06-24 13:13 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2020-06-20 20:26:56 PDT
[JSC] llintTrue / jitTrue can encounter native functions
Comment 1 Yusuke Suzuki 2020-06-20 20:27:39 PDT
Created attachment 402414 [details]
Patch
Comment 2 Yusuke Suzuki 2020-06-20 20:28:39 PDT
<rdar://problem/64257914>
Comment 3 Yusuke Suzuki 2020-06-20 20:29:32 PDT
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 4 Mark Lam 2020-06-20 20:46:34 PDT
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.
Comment 5 Yusuke Suzuki 2020-06-24 13:12:06 PDT
Created attachment 402676 [details]
Patch

Patch for landing
Comment 6 Yusuke Suzuki 2020-06-24 13:13:19 PDT
Created attachment 402677 [details]
Patch

Patch for landing
Comment 7 Yusuke Suzuki 2020-06-24 16:37:44 PDT
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.
Comment 8 Yusuke Suzuki 2020-06-24 16:38:18 PDT
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.
Comment 9 Yusuke Suzuki 2020-06-24 16:40:19 PDT
Committed r263483: <https://trac.webkit.org/changeset/263483>
Comment 10 Yusuke Suzuki 2020-06-24 17:19:52 PDT
Committed r263486: <https://trac.webkit.org/changeset/263486>