Bug 186283 – REGRESSION: (r232114 - 232120): Multiple JSC Stress test failures on 32 bit architecture

• Status: REOPENED
• Component: Tools / Tests

[Dawei Fenton (:realdawei)]

A set of JSC stress test failures have been occurring since May 23.

https://build.webkit.org/builders/Apple%20High%20Sierra%2032-bit%20JSC%20%28BuildAndTest%29/builds/1978/steps/webkit-32bit-jsc-test/logs/stdio

** The following JSC stress test failures have been introduced:
	executableAllocationFuzz.yaml/executableAllocationFuzz/v8-raytrace.js.executable-allocation-fuzz-default
	executableAllocationFuzz.yaml/executableAllocationFuzz/v8-raytrace.js.executable-allocation-fuzz-no-cjit
	slowMicrobenchmarks.yaml/slowMicrobenchmarks/default-derived-constructor.js.default
	slowMicrobenchmarks.yaml/slowMicrobenchmarks/default-derived-constructor.js.no-cjit
	stress/put-direct-index-broken-2.js.dfg-maximal-flush-validate-no-cjit
	stress/splay-flash-access.js.default
	stress/splay-flash-access.js.ftl-no-cjit
	stress/spread-forward-call-varargs-stack-overflow.js.dfg-maximal-flush-validate-no-cjit

[Dawei Fenton (:realdawei)]

Created attachment 341981 [details]
Patch

[Mark Lam]

Comment on attachment 341981 [details]
Patch

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

> JSTests/stress/splay-flash-access.js:1
> +//@skip if $architecture == "x86"

I don't think you should drop the old condition.  This should be //@skip if $memoryLimited or $architecture == "x86"

[Ryan Haddad]

Comment on attachment 341981 [details]
Patch

My understanding was that the goal was to skip these on the 32-bit JSC bot. This will skip the tests on 64 bit.

[Ryan Haddad]

Comment on attachment 341981 [details]
Patch

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

> JSTests/executableAllocationFuzz.yaml:26
> +      if ($hostOS == "windows" || $architecture == "x86")

I believe this should be "i386", not "x86".

> JSTests/slowMicrobenchmarks/default-derived-constructor.js:1
> +//@skip if $architecture == "x86"

Ditto.

> JSTests/stress/put-direct-index-broken-2.js:1
> +//@skip if $architecture == "x86"

Ditto.

>> JSTests/stress/splay-flash-access.js:1
>> +//@skip if $architecture == "x86"
> 
> I don't think you should drop the old condition.  This should be //@skip if $memoryLimited or $architecture == "x86"

Ditto.

> JSTests/stress/spread-forward-call-varargs-stack-overflow.js:1
> +//@skip if $architecture == "x86"

Ditto.

[Mark Lam]

(In reply to Ryan Haddad from comment #4)
> Comment on attachment 341981 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=341981&action=review
> 
> > JSTests/executableAllocationFuzz.yaml:26
> > +      if ($hostOS == "windows" || $architecture == "x86")
> 
> I believe this should be "i386", not "x86".

Not true.  The $architecture value is determined by Tools/Scripts/run-jsc-stress-tests.  According to run-jsc-stress-tests, "x86" is distinct from "x86_64", and is used to represent 32-bit only.  For example, see the line:

$isFTLPlatform = !($architecture == "x86" || $architecture == "arm" || $architecture == "mips" || $hostOS == "windows")

... where the platform is considered to be not an FTL platform if $architecture is "x86" i.e. "x86" is 32-bit only.  There's also no code in run-jsc-stress-tests that sets $architecture to "i386".

"x86" is the right value to test against for 32-bit x86 architecture.

[Ryan Haddad]

(In reply to Mark Lam from comment #5)
> (In reply to Ryan Haddad from comment #4)
> > Comment on attachment 341981 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=341981&action=review
> > 
> > > JSTests/executableAllocationFuzz.yaml:26
> > > +      if ($hostOS == "windows" || $architecture == "x86")
> > 
> > I believe this should be "i386", not "x86".
> 
> Not true.  The $architecture value is determined by
> Tools/Scripts/run-jsc-stress-tests.  According to run-jsc-stress-tests,
> "x86" is distinct from "x86_64", and is used to represent 32-bit only.  For
> example, see the line:
> 
> $isFTLPlatform = !($architecture == "x86" || $architecture == "arm" ||
> $architecture == "mips" || $hostOS == "windows")
> 
> ... where the platform is considered to be not an FTL platform if
> $architecture is "x86" i.e. "x86" is 32-bit only.  There's also no code in
> run-jsc-stress-tests that sets $architecture to "i386".
> 
> "x86" is the right value to test against for 32-bit x86 architecture.
I stand corrected! Thanks, Mark.

[Dawei Fenton (:realdawei)]

(In reply to Mark Lam from comment #2)
> Comment on attachment 341981 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=341981&action=review
> 
> > JSTests/stress/splay-flash-access.js:1
> > +//@skip if $architecture == "x86"
> 
> I don't think you should drop the old condition.  This should be //@skip if
> $memoryLimited or $architecture == "x86"

ok got it, will make this change to keep both conditions

[Dawei Fenton (:realdawei)]

Created attachment 341989 [details]
Patch

[Mark Lam]

Comment on attachment 341989 [details]
Patch

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

> JSTests/slowMicrobenchmarks/default-derived-constructor.js:1
> +//@skip if $architecture == "x86"

Sorry I didn't catch this earlier.  Stylistically, I think we should have a space between '//@' and 'skip'.  I know there's a few violations of this, but in general, that's how we do it.  Can you please fix?

[Dawei Fenton (:realdawei)]

(In reply to Mark Lam from comment #9)
> Comment on attachment 341989 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=341989&action=review
> 
> > JSTests/slowMicrobenchmarks/default-derived-constructor.js:1
> > +//@skip if $architecture == "x86"
> 
> Sorry I didn't catch this earlier.  Stylistically, I think we should have a
> space between '//@' and 'skip'.  I know there's a few violations of this,
> but in general, that's how we do it.  Can you please fix?

yep, you got it.

[Dawei Fenton (:realdawei)]

Created attachment 341993 [details]
Skip the tests.

[WebKit Commit Bot]

Comment on attachment 341993 [details]
Skip the tests.

Clearing flags on attachment: 341993

Committed r232525: <https://trac.webkit.org/changeset/232525>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/40827539>

[Dawei Fenton (:realdawei)]

Re-opening this bug to reflect the fact that these tests were skipped.  The cause of the failures needs further investigation.   The regression occurred somewhere in the range of r232114 - 232120