| Summary: | stress/array-buffer-view-watchpoint-can-be-fired-in-really-add-in-dfg.js failing on ppc64le and s390x | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> | ||||||||||||
| Component: | JavaScriptCore | Assignee: | Michael Catanzaro <mcatanzaro> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | bugs-noreply, clopez, mcatanzaro, pmatos, tpopela, webkit-bug-importer | ||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||
| Hardware: | PC | ||||||||||||||
| OS: | Linux | ||||||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=211721 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Michael Catanzaro
2020-05-11 07:11:39 PDT
This turned into quite an adventure. Eventually I tracked down the source code for the file command. I'm attaching here the magic file it uses to determine CPU architecture. It's not easy to read, but it matches our implementation of determineArchitectureFromELFBinary. Looks like the magic number for 64-bit PowerPC is 21 (regardless of endianness), and the magic number for all variants of S/390 is 22. So we could blindly add them to determineArchitectureFromELFBinary and see whether or not it works.... But tbh, since the test is failing on armv7 as well, the past of least-resistance would be to skip unconditionally until it can be fixed. Ideally someone with more knowledge of JSC would decide how to handle it... all I can do is make it skippable. I also found bug #210641. Maybe it would make sense to use fewer iterations on all architectures? If that's not enough to enter dfg, then maybe this should be an x86_64-only test...? Anyway, here's my attempt to skip the test on ppc64le and s390x. We won't know if it works until after it lands. Created attachment 399137 [details]
Patch
OK, since I see no other comments or suggestions, let's land this? Ping reviewers. Committed r261689: <https://trac.webkit.org/changeset/261689> All reviewed patches have been landed. Closing bug and clearing flags on attachment 399137 [details]. Thanks! It fixed ppc64le, but not s390x, the determineArchitectureFromELFBinary change doesn't work there. (In reply to Michael Catanzaro from comment #9) > It fixed ppc64le, but not s390x, the determineArchitectureFromELFBinary > change doesn't work there. Looking at an actual s390x binary, I got the right number (22, so 0x16) but the script fails to properly check the endianness of the binary. It's expecting 0x16 0x00 but big endian is 0x00 0x16. Created attachment 399483 [details]
Patch
Created attachment 399496 [details]
Patch
Ping reviewers ;) Comment on attachment 399496 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399496&action=review > Tools/Scripts/run-jsc-stress-tests:383 > + # MIPS and PowerPC may are bi-endian. S390 (which includes S390x) is big endian. The rest are little endian. Please fix this comment.: - # MIPS and PowerPC may are bi-endian. S390 (which includes S390x) is big endian. The rest are little endian. + # MIPS and PowerPC may be little-endian or big-endian. S390 (which includes S390x) is big-endian. The rest are little-endian. MIPS can be little-endian or can be also big-endian. The MIPS architecture variant we target our CI its MIPS32le (le stands for little-endian). I meant "bi-" as in "both," but I see I have a typo there anyway, and your wording is more clear, so sure. Created attachment 399737 [details]
Patch for landing
Comment on attachment 399737 [details]
Patch for landing
Another typo :S
Created attachment 399738 [details]
Patch for landing
Committed r261862: <https://trac.webkit.org/changeset/261862> All reviewed patches have been landed. Closing bug and clearing flags on attachment 399738 [details]. |