Bug 212399 - in_structure_property needs to handle constants on the RHS of the "in"
Summary: in_structure_property needs to handle constants on the RHS of the "in"
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Mac macOS 10.15
: P2 Normal
Assignee: Keith Miller
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-05-26 20:41 PDT by Minh Tran
Modified: 2020-06-01 16:50 PDT (History)
8 users (show)

See Also:


Attachments
Patch (2.58 KB, patch)
2020-05-27 09:15 PDT, Keith Miller
saam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Minh Tran 2020-05-26 20:41:07 PDT
The bug is in object iterator, really simple.
Some type check must happen if the iterator can use `in` or not.
In this case, the index (string d) can not use `in` function. I know `1337` is not an Object, but it still crash in some way.
I am really new to JavaScriptCore and do not understand the design pattern fully, but I hope my POC can help. Thanks.

POC:
```
const v0 = {d:13.37};
for (const v1 in v0) {
	print(typeof(v1), v1);
	print(typeof(v0[v1]), v0[v1]);
	const v2 = v1 in 1337;
}
```

LOG:
```
string d
number 13.37

ASSERTION FAILED: !reg.isConstant()
../../Source/JavaScriptCore/interpreter/CallFrameInlines.h(44) : JSC::Register &JSC::CallFrame::uncheckedR(JSC::VirtualRegister)
1   0x114a6b8d9 WTFCrash
2   0x104896590 WTF::BasicRawSentinelNode<Worker, WTF::DumbPtrTraits<Worker> >::remove()
3   0x1113625a3 JSC::CallFrame::uncheckedR(JSC::VirtualRegister)
4   0x1134d367e slow_path_in_structure_property
5   0x10f6b2e02 llint_entry
6   0x10f69bf32 vmEntryToJavaScript
7   0x112ab449b JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
8   0x112ab1fa3 JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::JSGlobalObject*, JSC::JSObject*)
9   0x11351c4da JSC::evaluate(JSC::JSGlobalObject*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
10  0x104975534 runWithOptions(GlobalObject*, CommandLine&, bool&)
11  0x1048f7174 jscmain(int, char**)::$_4::operator()(JSC::VM&, GlobalObject*, bool&) const
12  0x10489b6fe int runJSC<jscmain(int, char**)::$_4>(CommandLine const&, bool, jscmain(int, char**)::$_4 const&)
13  0x1048981ae jscmain(int, char**)
14  0x104897c8e main
15  0x7fff72122cc9 start
16  0xc
```

Steps to Reproduce:

1) Build Relaese with ASAN:
./Tools/Scripts/set-webkit-configuration --asan
./Tools/Scripts/build-webkit --jsc-only --release

2) Run JSC with JS file

Actual Results: JSC crashes with log like above

Expected Results: JSC should not crash (or raise Type Error because 1337 is not an Object)

Build Date & Hardware: commit@751ec07c691376353670d0913d09a85d490395cd (Date:   Wed May 27 02:42:14 2020 +0000)
Comment 1 Radar WebKit Bug Importer 2020-05-26 23:38:50 PDT
<rdar://problem/63662913>
Comment 2 Keith Miller 2020-05-27 09:15:12 PDT
Created attachment 400340 [details]
Patch
Comment 3 Saam Barati 2020-05-27 09:17:57 PDT
Comment on attachment 400340 [details]
Patch

nice & thanks
r=me
Comment 4 Mark Lam 2020-05-27 09:18:37 PDT
Comment on attachment 400340 [details]
Patch

r=me too
Comment 5 Keith Miller 2020-05-27 09:28:56 PDT
Committed r262197: <https://trac.webkit.org/changeset/262197>
Comment 6 Ryan Haddad 2020-05-27 12:44:01 PDT
(In reply to Keith Miller from comment #5)
> Committed r262197: <https://trac.webkit.org/changeset/262197>
JSC EWS showed 18 new failures, which are now appearing on the trunk bot:
https://build.webkit.org/builders/Apple-Catalina-Release-JSC-Tests/builds/2209/steps/jscore-test/logs/stdio
Comment 7 Saam Barati 2020-05-27 13:00:44 PDT
Comment on attachment 400340 [details]
Patch

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

> JSTests/stress/for-in-in-structure-property-constant-virtual-register.js:4
> +const v0 = {d:13.37};
> +for (const v1 in v0) {
> +        const v2 = v1 in 1337;
> +}

You need to wrap this in try/catch I believe
Comment 8 Ryan Haddad 2020-06-01 16:50:41 PDT
(In reply to Ryan Haddad from comment #6)
> (In reply to Keith Miller from comment #5)
> > Committed r262197: <https://trac.webkit.org/changeset/262197>
> JSC EWS showed 18 new failures, which are now appearing on the trunk bot:
> https://build.webkit.org/builders/Apple-Catalina-Release-JSC-Tests/builds/
> 2209/steps/jscore-test/logs/stdio
Keith fixed this in https://trac.webkit.org/changeset/262210/webkit