212569 2020-05-30 09:50:43 -0700 JSTests/exceptionFuzz/earley-boyer.js fails with early exception thrown. 2020-06-01 10:59:16 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=212598 InRadar P2 Normal --- 1 mark.lam ticaiolima fpizlo keith_miller msaboff rmorisset saam ticaiolima tzagallo webkit-bug-importer ysuzuki oldest_to_newest 1657629 0 mark.lam 2020-05-30 09:50:43 -0700 Here's how I run the test: $ VM=WebKitBuild/Release && DYLD_FRAMEWORK_PATH=$VM lldb $VM/jsc -- --dumpGeneratedBytecodes=1 --useBaselineJIT=0 --fireExceptionFuzzAt=782 --useDollarVM\=true --useExceptionFuzz\=true JSTests/exceptionFuzz/earley-boyer.js We get an uncaught exception (which should not happen): JSC EXCEPTION FUZZ: Throwing fuzz exception with call frame 0x1079fab68, seen in LLIntSlowPaths and return address 0x1078abc71. Exception: Error: Exception Fuzz global code@JSTests/exceptionFuzz/earley-boyer.js:46:20 JSC EXCEPTION FUZZ: encountered 782 checks. This uncaught exception should not happen because the entire source of JSTests/exceptionFuzz/earley-boyer.js is wrapped in a try catch statement. From the bytecode dump, Exception Handlers: 1: { start: [4031] end: [109677] target: [109745] } catch The bytecode around 4031 are: [3953] *new_func loc6, loc4, 292 [3961] *put_to_scope loc8, 292, loc6, 2048<ThrowIfNotFound|GlobalProperty|NotInitialization|NotStrictMode>, 0, 0 [3977] *new_func loc6, loc4, 293 [3985] *put_to_scope loc8, 293, loc6, 2048<ThrowIfNotFound|GlobalProperty|NotInitialization|NotStrictMode>, 0, 0 [4001] *new_func loc6, loc4, 294 [4009] *put_to_scope loc8, 294, loc6, 2048<ThrowIfNotFound|GlobalProperty|NotInitialization|NotStrictMode>, 0, 0 [4025] mov loc6, Undefined(const0) [4028] mov loc6, Undefined(const0) [4031] *resolve_scope loc7, loc4, 295, GlobalProperty, 0 [4045] *put_to_scope loc7, 295, this, 1048576<DoNotThrowIfNotFound|GlobalProperty|Initialization|NotStrictMode>, 0, 0 [4061] *resolve_scope loc7, loc4, 296, GlobalProperty, 0 [4075] *put_to_scope loc7, 296, Int32: -1(const1), 1048576<DoNotThrowIfNotFound|GlobalProperty|Initialization|NotStrictMode>, 0, 0 Note all the new_func opcodes that came before 4031. There's definitely instructions that can throw generated before beginning of the try block. Need to investigate further which bytecode --fireExceptionFuzzAt=782 corresponds to, but it looks like there maybe a bug here. 1657916 1 ticaiolima 2020-06-01 08:20:39 -0700 I gave a shot investigating this issue. Right now we are hoisting function declarations out of exception handler. It is not clear to me yet if this is the behavior we want to have, but this explains why we are failing with early exception. I created another test case that we can observe the same behavior: ``` try { function foo(){} function bar(){} function baz(){} foo(); } catch(e) { print(e); } ``` This fails early if we run with `--fireExceptionFuzzAt=10` or any number below 10. The bytecode generated for this is: ``` <global>#D6X2C4:[0x10b9bc000->0x10b6d6768, NoneGlobal, 153]: 34 instructions (0 16-bit instructions, 0 32-bit instructions, 14 instructions with metadata); 269 bytes (116 metadata bytes); 1 parameter(s); 18 callee register(s); 6 variable(s); scope at loc4 [ 0] enter [ 1] get_scope loc4 [ 3] mov loc5, loc4 [ 6] check_traps [ 7] new_func loc6, loc4, 0 [ 11] resolve_scope loc7, loc4, 0, GlobalProperty, 0 [ 18] mov loc8, loc7 [ 21] put_to_scope loc8, 0, loc6, 2048<ThrowIfNotFound|GlobalProperty|NotInitialization|NotStrictMode>, 0, 0 [ 29] new_func loc6, loc4, 1 [ 33] put_to_scope loc8, 1, loc6, 2048<ThrowIfNotFound|GlobalProperty|NotInitialization|NotStrictMode>, 0, 0 [ 41] new_func loc6, loc4, 2 [ 45] put_to_scope loc8, 2, loc6, 2048<ThrowIfNotFound|GlobalProperty|NotInitialization|NotStrictMode>, 0, 0 [ 53] mov loc6, Undefined(const0) [ 56] mov loc6, Undefined(const0) [ 59] resolve_scope loc8, loc4, 0, GlobalProperty, 0 [ 66] get_from_scope loc7, loc8, 0, 2048<ThrowIfNotFound|GlobalProperty|NotInitialization|NotStrictMode>, 0, 0 [ 74] call loc6, loc7, 1, 14 [ 80] jmp 41(->121) [ 82] mov loc4, loc5 [ 85] mov loc8, <JSValue()>(const1) [ 88] mov loc8, loc7 [ 91] mov loc6, Undefined(const0) [ 94] resolve_scope loc12, loc4, 3, GlobalProperty, 0 [ 101] get_from_scope loc9, loc12, 3, 2048<ThrowIfNotFound|GlobalProperty|NotInitialization|NotStrictMode>, 0, 0 [ 109] mov loc11, loc8 [ 112] call loc6, loc9, 2, 18 [ 118] mov loc7, Undefined(const0) [ 121] mov loc6, Undefined(const0) [ 124] resolve_scope loc8, loc4, 0, GlobalProperty, 0 [ 131] get_from_scope loc7, loc8, 0, 2048<ThrowIfNotFound|GlobalProperty|NotInitialization|NotStrictMode>, 0, 0 [ 139] call loc6, loc7, 1, 14 [ 145] end loc6 [ 147] catch loc8, loc7 [ 151] jmp -69(->82) Identifiers: id0 = foo id1 = bar id2 = baz id3 = print Constants: k0 = Undefined k1 = <JSValue()> Exception Handlers: 1: { start: [ 56] end: [ 82] target: [ 147] } catch ``` Also, FYI this is causing some flaky failures on EWS and also buildbots (e.g https://build.webkit.org/builders/Apple-Catalina-Release-JSC-Tests) 1657930 2 ticaiolima 2020-06-01 08:52:55 -0700 I noticed that he following test don't fail: ``` try { (function () { function foo(){} function bar(){} function baz(){} foo(); })(); } catch(e) { print(e); } ``` This avoids that function declarations are hoisted before handler start point. It only fails early with `--fireExceptionFuzzAt=1` because there is an exception check on `op_enter`. I'm not convinced that this is the right solution, but I think it would be good to use it temporarily on `exceptionFuzz` to make tree green while we investigate what's the correct fix. 1657938 3 400737 ticaiolima 2020-06-01 09:16:42 -0700 Created attachment 400737 Patch 1657943 4 400737 mark.lam 2020-06-01 09:32:51 -0700 Comment on attachment 400737 Patch r=me. Please file a new bug for try-catch range not including hoisted function declarations so that we don't just drop it. Please also note that one should undo this workaround in order to reproduce the issue, and that the issue is intermittent. Thanks. 1657952 5 ticaiolima 2020-06-01 10:02:34 -0700 (In reply to Mark Lam from comment #4) > Comment on attachment 400737 [details] > Patch > > r=me. Please file a new bug for try-catch range not including hoisted > function declarations so that we don't just drop it. Please also note that > one should undo this workaround in order to reproduce the issue, and that > the issue is intermittent. Thanks. Thank you very much for the review. I opened https://bugs.webkit.org/show_bug.cgi?id=212598 and related it with this issue. 1657963 6 400737 ticaiolima 2020-06-01 10:44:54 -0700 Comment on attachment 400737 Patch I was waiting until EWS process it, but it is taking too long due to current flakiness of exceptionFuzz/earley-boyer.js. I'm setting cq+ and watching bots to verify if this cause any regression. 1657970 7 ews-feeder 2020-06-01 10:58:08 -0700 Committed r262383: <https://trac.webkit.org/changeset/262383> All reviewed patches have been landed. Closing bug and clearing flags on attachment 400737. 1657972 8 webkit-bug-importer 2020-06-01 10:59:16 -0700 <rdar://problem/63834898> 400737 2020-06-01 09:16:42 -0700 2020-06-01 10:58:09 -0700 Patch bug-212569-20200601131640.patch text/plain 2949 ticaiolima U3VidmVyc2lvbiBSZXZpc2lvbjogMjYyMzY5CmRpZmYgLS1naXQgYS9KU1Rlc3RzL0NoYW5nZUxv ZyBiL0pTVGVzdHMvQ2hhbmdlTG9nCmluZGV4IDNhNTMzNjAwMjI0YjQ4MGZlYzQ1YjAxYjVhOWFj NTIyMTJhMjFmNTguLmEzMDBkMTM4YWUzZTYxZWZmNzA3Mzk5NjdhYTI3ODhiZTk3YzEyZWEgMTAw NjQ0Ci0tLSBhL0pTVGVzdHMvQ2hhbmdlTG9nCisrKyBiL0pTVGVzdHMvQ2hhbmdlTG9nCkBAIC0x LDMgKzEsMjEgQEAKKzIwMjAtMDYtMDEgIENhaW8gTGltYSAgPHRpY2Fpb2xpbWFAZ21haWwuY29t PgorCisgICAgICAgIEpTVGVzdHMvZXhjZXB0aW9uRnV6ei9lYXJsZXktYm95ZXIuanMgZmFpbHMg d2l0aCBlYXJseSBleGNlcHRpb24gdGhyb3duLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjEyNTY5CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgVGhpcyBpcyBhIHRlbXBvcmFyeSBmaXggdG8gYXZvaWQgZmxh d2t5IHJlc3VsdHMgd2hlbiBydW5uaW5nCisgICAgICAgIGBleGNlcHRpb25GdXp6YCB0ZXN0cy4g UmlnaHQgbm93LCBleGNlcHRpb24gaGFuZGxlciBzdGFydHMgYWZ0ZXIKKyAgICAgICAgZnVuY3Rp b24gZGVjbGFyYXRpb25zLCBzaW5jZSB0aGV5IGFyZSBob2lzdGVkLiBTaW5jZSBmdW5jdGlvbgor ICAgICAgICBkZWNsYXJhdGlvbnMgZW1pdCBgbmV3X2Z1bmNgIGJ5dGVjb2RlLCB0aG9zZSBieXRl Y29kZXMgY2FuCisgICAgICAgIHRocm93IGFuIGV4Y2VwdGlvbiBvdXRzaWRlIGV4cGVjdGVkIGV4 Y2VwdGlvbiBoYW5kbGVyLgorICAgICAgICBFbWJlZGRpbmcgdGhlbSBpbiBhIGZ1bmN0aW9uIGF2 b2lkIHN1Y2ggaG9pc3RpbmcgdG8gaGFwcGVuLgorCisgICAgICAgICogZXhjZXB0aW9uRnV6ei8z ZC1jdWJlLmpzOgorICAgICAgICAqIGV4Y2VwdGlvbkZ1enovZGF0ZS1mb3JtYXQteHBhcmIuanM6 CisgICAgICAgICogZXhjZXB0aW9uRnV6ei9lYXJsZXktYm95ZXIuanM6CisKIDIwMjAtMDUtMzAg IFl1c3VrZSBTdXp1a2kgIDx5c3V6dWtpQGFwcGxlLmNvbT4KIAogICAgICAgICBbSlNDXSBmb3It aW4gc2hvdWxkIGFsbG9jYXRlIG5ldyB0ZW1wb3JhcnkgcmVnaXN0ZXIgZm9yIGJhc2UKZGlmZiAt LWdpdCBhL0pTVGVzdHMvZXhjZXB0aW9uRnV6ei8zZC1jdWJlLmpzIGIvSlNUZXN0cy9leGNlcHRp b25GdXp6LzNkLWN1YmUuanMKaW5kZXggN2I5NmI2ZGVkOTM5N2UwMzE1OTRmNGU3ZmRjYmJmZTFl MzZmMGU1Zi4uZTRhMmEyODNiYWIwOGFhODZkZTU1YjU3MmQ1MWRkNWQ4OWI0ZGVmOCAxMDA2NDQK LS0tIGEvSlNUZXN0cy9leGNlcHRpb25GdXp6LzNkLWN1YmUuanMKKysrIGIvSlNUZXN0cy9leGNl cHRpb25GdXp6LzNkLWN1YmUuanMKQEAgLTEsNCArMSw1IEBACiB0cnkgeworKGZ1bmN0aW9uKCkg ewogCiAvLyAzRCBDdWJlIFJvdGF0aW9uCiAvLyBodHRwOi8vd3d3LnNwZWljaC5uZXQvY29tcHV0 ZXIvbW96dGVzdGluZy8zZC5odG0KQEAgLTM1NSw2ICszNTYsNyBAQCBUZXN0aW5nID0gbnVsbDsK IExvb3BUaW1lID0gbnVsbDsKIERpc3BsQXJlYSA9IG51bGw7CiAKK30pKCk7CiB9IGNhdGNoIChl KSB7CiAgICAgcHJpbnQoIkpTQyBFWENFUFRJT04gRlVaWjogQ2F1Z2h0IGV4Y2VwdGlvbjogIiAr IGUpOwogfQpkaWZmIC0tZ2l0IGEvSlNUZXN0cy9leGNlcHRpb25GdXp6L2RhdGUtZm9ybWF0LXhw YXJiLmpzIGIvSlNUZXN0cy9leGNlcHRpb25GdXp6L2RhdGUtZm9ybWF0LXhwYXJiLmpzCmluZGV4 IDg5ZTk4N2IwZWNjYjE2MDBmYWM5Mzk1ZmMxZTZjYWFiOGUxYWMyMDQuLmY2MTNiMjM3MWU1ODQ3 MzBmNWM4ODdiYTU1MjQ2MmNlNmU2ZDZmY2UgMTAwNjQ0Ci0tLSBhL0pTVGVzdHMvZXhjZXB0aW9u RnV6ei9kYXRlLWZvcm1hdC14cGFyYi5qcworKysgYi9KU1Rlc3RzL2V4Y2VwdGlvbkZ1enovZGF0 ZS1mb3JtYXQteHBhcmIuanMKQEAgLTEsNCArMSw1IEBACiB0cnkgeworKGZ1bmN0aW9uKCkgewog CiAvKgogICogQ29weXJpZ2h0IChDKSAyMDA0IEJhcm9uIFNjaHdhcnR6IDxiYXJvbiBhdCBzZXF1 ZW50IGRvdCBvcmc+CkBAIC00MjEsNiArNDIyLDcgQEAgZm9yIChpID0gMDsgaSA8IDQwMDA7ICsr aSkgewogLy8gRklYTUU6IEZpbmQgYSB3YXkgdG8gdmFsaWRhdGUgdGhpcyB0ZXN0LgogLy8gaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTExNDg0OQogCit9KSgpOwogfSBj YXRjaCAoZSkgewogICAgIHByaW50KCJKU0MgRVhDRVBUSU9OIEZVWlo6IENhdWdodCBleGNlcHRp b246ICIgKyBlKTsKIH0KZGlmZiAtLWdpdCBhL0pTVGVzdHMvZXhjZXB0aW9uRnV6ei9lYXJsZXkt Ym95ZXIuanMgYi9KU1Rlc3RzL2V4Y2VwdGlvbkZ1enovZWFybGV5LWJveWVyLmpzCmluZGV4IGQ3 OTkyYWY3YjE1M2UxYzYzOWU4ZWM2OTAwYTA1MWI2NDJmYjcyZTUuLjVkNzlkMmQ0YzQxNWJiOTVm ODFkNjQ2Yjg4OTExMTljNmY1ODk2ZTYgMTAwNjQ0Ci0tLSBhL0pTVGVzdHMvZXhjZXB0aW9uRnV6 ei9lYXJsZXktYm95ZXIuanMKKysrIGIvSlNUZXN0cy9leGNlcHRpb25GdXp6L2VhcmxleS1ib3ll ci5qcwpAQCAtMSw0ICsxLDUgQEAKIHRyeSB7CisoZnVuY3Rpb24oKSB7CiAvLyBUaGlzIGZpbGUg aXMgYXV0b21hdGljYWxseSBnZW5lcmF0ZWQgYnkgc2NoZW1lMmpzLCBleGNlcHQgZm9yIHRoZQog Ly8gYmVuY2htYXJrIGhhcm5lc3MgY29kZSBhdCB0aGUgYmVnaW5uaW5nIGFuZCBlbmQgb2YgdGhl IGZpbGUuCiAKQEAgLTQ2ODEsNyArNDY4Miw3IEBAIGZvciAodmFyIGkgPSAwOyBpIDwgNDsgKytp KSB7CiAgIEJnTF9lYXJsZXl6ZDJiZW5jaG1hcmt6ZDIoKTsKICAgQmdMX25ib3llcnpkMmJlbmNo bWFya3pkMigpOwogfQotCit9KSgpOwogfSBjYXRjaCAoZSkgewogICAgIHByaW50KCJKU0MgRVhD RVBUSU9OIEZVWlo6IENhdWdodCBleGNlcHRpb246ICIgKyBlKTsKIH0K