RESOLVED FIXED 230915
DoesGCCheck does not use enough bits for nodeIndex 
https://bugs.webkit.org/show_bug.cgi?id=230915
Summary DoesGCCheck does not use enough bits for nodeIndex
Saam Barati
Reported 2021-09-28 14:36:36 PDT
...
Attachments
Patch. (12.13 KB, patch)
2021-09-28 15:34 PDT, Saam Barati 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (12.19 KB, patch)
2021-09-28 15:38 PDT, Saam Barati 		mark.lam: review+
DetailsFormatted DiffDiff
patch for landing (12.19 KB, patch)
2021-09-28 17:39 PDT, Saam Barati 		no flags
DetailsFormatted DiffDiff
[fast-cq] fix test (937 bytes, patch)
2021-09-29 09:54 PDT, Saam Barati 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Saam Barati
Comment 1 2021-09-28 14:37:06 PDT
<rdar://83297515>
Saam Barati
Comment 2 2021-09-28 15:34:06 PDT
Created attachment 439533 [details] Patch.
Saam Barati
Comment 3 2021-09-28 15:38:03 PDT
Created attachment 439534 [details] Patch
Mark Lam
Comment 4 2021-09-28 16:09:59 PDT
Comment on attachment 439534 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=439534&action=review r=me with fix. > Source/JavaScriptCore/dfg/DFGDoesGCCheck.h:79 > + unsigned nodeOp() { return u.other & nodeOpMask; } This should be: unsigned nodeOp() { return (u.other >> nodeOpShift) & nodeOpMask; }
Saam Barati
Comment 5 2021-09-28 17:18:48 PDT
Comment on attachment 439534 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=439534&action=review >> Source/JavaScriptCore/dfg/DFGDoesGCCheck.h:79 >> + unsigned nodeOp() { return u.other & nodeOpMask; } > > This should be: > unsigned nodeOp() { return (u.other >> nodeOpShift) & nodeOpMask; } Will fix. Thanks
Saam Barati
Comment 6 2021-09-28 17:39:36 PDT
Created attachment 439549 [details] patch for landing
EWS
Comment 7 2021-09-28 18:33:09 PDT
Committed r283207 (242249@main): <https://commits.webkit.org/242249@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 439549 [details].
Michael Catanzaro
Comment 8 2021-09-29 06:23:35 PDT
Hi, this new test fails on every architecture (x86_64, aarch64, ppc64le, s390x) with the error: Running stress/verify-can-gc-node-index.js.default stress/verify-can-gc-node-index.js.default: Exception: RangeError: Maximum call stack size exceeded. stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:7:11 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: f@verify-can-gc-node-index.js:8:6 stress/verify-can-gc-node-index.js.default: ERROR: Unexpected exit code: 3 FAIL: stress/verify-can-gc-node-index.js.default So it's doing too much recursion. This is using the JSCOnly port and Red Hat's internal CI, which has cloop enabled, but I see the JSC EWS failed in the same way, and I doubt that EWS is using cloop.
Saam Barati
Comment 9 2021-09-29 09:54:25 PDT
Created attachment 439617 [details] [fast-cq] fix test
EWS
Comment 10 2021-09-29 09:56:13 PDT
Committed r283231 (242273@main): <https://commits.webkit.org/242273@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 439617 [details].
Note You need to log in before you can comment on or make changes to this bug.