Bug 214491 – REGRESSION(r264330): infinite loop in JSC stress tests using cloop

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Catanzaro]

r264330 added an infinite loop that was supposed to be broken by the --returnEarlyFromInfiniteLoopsForFuzzing=true flag. However, it doesn't work when the tests are run with cloop. This has caused JSC stress tests to run forever only on x86_64 (with cloop enabled). Instead of skipping the tests on particular architectures, let's run the loops only if $vm.useJIT(), suggested by Yusuke.

I think there is a problem with architecture detection for arm64, because the tests are still running fine on this architecture.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #0)
> I think there is a problem with architecture detection for arm64, because
> the tests are still running fine on this architecture.

Seems I touched it last, in r261862. My changes look correct, though....

[Michael Catanzaro]

Created attachment 404582 [details]
Patch

[Michael Catanzaro]

Looks like I broke the test on jsc-armv7. No clue why, unless maybe this feature depends on a higher JIT tier than the baseline JIT...? I'm not sure what else could be going wrong.

The solution would be to not remove the skip line, so if nobody else has a guess, that's what I'll do.

[Yusuke Suzuki]

Comment on attachment 404582 [details]
Patch

r=me

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #3)
> Looks like I broke the test on jsc-armv7. No clue why, unless maybe this
> feature depends on a higher JIT tier than the baseline JIT...? I'm not sure
> what else could be going wrong.
> 
> The solution would be to not remove the skip line, so if nobody else has a
> guess, that's what I'll do.

Yusuke, are you OK with that "solution?" (The current version of the patch breaks jsc-armv7, and that would avoid doing so... but it doesn't seem great.)

[Saam Barati]

Comment on attachment 404582 [details]
Patch

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

> JSTests/stress/validate-does-gc-with-return-early-from-infinite-loop.js:3
> +if ($vm.useJIT()) {

this looks wrong in combination of removing the above skip. I didn't implement it on 32-bit.

[Michael Catanzaro]

So to confirm, this test should run only (a) on 64-bit platforms, and (b) only if JIT is enabled. Yes? In that case, my patch would be fine if I leave the Skip line unchanged, right?

[Saam Barati]

(In reply to Michael Catanzaro from comment #7)
> So to confirm, this test should run only (a) on 64-bit platforms, and (b)
> only if JIT is enabled. Yes? In that case, my patch would be fine if I leave
> the Skip line unchanged, right?

Yes, I believe that is the way to go.

[Michael Catanzaro]

Created attachment 404814 [details]
Patch

[EWS]

Committed r264668: <https://trac.webkit.org/changeset/264668>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 404814 [details].

[Radar WebKit Bug Importer]

<rdar://problem/65895487>