Bug 186283

Summary: REGRESSION: (r232114 - 232120): Multiple JSC Stress test failures on 32 bit architecture
Product: WebKit Reporter: Dawei Fenton (:realdawei) <realdawei>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: REOPENED    
Severity: Normal CC: commit-queue, lforschler, mark.lam, ryanhaddad, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Skip the tests. none

Dawei Fenton (:realdawei)
Reported 2018-06-04 13:50:36 PDT
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
Attachments
Patch (3.13 KB, patch)
2018-06-05 11:40 PDT, Dawei Fenton (:realdawei) 		no flags
DetailsFormatted DiffDiff
Patch (3.15 KB, patch)
2018-06-05 13:32 PDT, Dawei Fenton (:realdawei) 		no flags
DetailsFormatted DiffDiff
Skip the tests. (3.15 KB, patch)
2018-06-05 14:02 PDT, Dawei Fenton (:realdawei) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Dawei Fenton (:realdawei)
Comment 1 2018-06-05 11:40:44 PDT
Created attachment 341981 [details] Patch
Mark Lam
Comment 2 2018-06-05 11:52:19 PDT
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 3 2018-06-05 11:54:33 PDT
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 4 2018-06-05 11:59:06 PDT
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
Comment 5 2018-06-05 12:33:13 PDT
(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
Comment 6 2018-06-05 13:00:27 PDT
(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)
Comment 7 2018-06-05 13:06:37 PDT
(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)
Comment 8 2018-06-05 13:32:06 PDT
Created attachment 341989 [details] Patch
Mark Lam
Comment 9 2018-06-05 13:39:16 PDT
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)
Comment 10 2018-06-05 13:41:46 PDT
(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)
Comment 11 2018-06-05 14:02:41 PDT
Created attachment 341993 [details] Skip the tests.
WebKit Commit Bot
Comment 12 2018-06-05 15:57:12 PDT
Comment on attachment 341993 [details] Skip the tests. Clearing flags on attachment: 341993 Committed r232525: <https://trac.webkit.org/changeset/232525>
WebKit Commit Bot
Comment 13 2018-06-05 15:57:13 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 14 2018-06-05 15:58:20 PDT
<rdar://problem/40827539>
Dawei Fenton (:realdawei)
Comment 15 2018-06-06 17:59:39 PDT
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
Note You need to log in before you can comment on or make changes to this bug.