-
Notifications
You must be signed in to change notification settings - Fork 30.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
deps: cherry-pick a51f429 from V8 upstream #7834
Conversation
Original commit message: [regexp] Fix case-insensitive matching for one-byte subjects. The bug occurs because we do not canonicalize character class ranges before adding case equivalents. While adding case equivalents, we abort early for one-byte subject strings, assuming that the ranges are sorted. Which they are not. R=marja@chromium.org BUG=v8:5199 Review-Url: https://codereview.chromium.org/2159683002 Cr-Commit-Position: refs/heads/master@{nodejs#37833} Fixes: nodejs#7708
LGTM |
|
LGTM with green ci |
new CI: https://ci.nodejs.org/job/node-test-pull-request/3458/. The Power big endian machines continue to have recurring problems in the CI. /cc @mhdawson: Do these machines have enough disk space? |
Another attempt at the CI: https://ci.nodejs.org/job/node-test-pull-request/3484/ |
PPC failed (10 days ago PPC passed but FreeBSD and Windows had problems) :
|
LGTM. I think this is ready to land. @nodejs/build, @mhdawson: Can anything be done about the Power machines in the CI, or should we be ignoring them? They have been causing a lot of churn and wasted time. |
@ofrobots: there's a lot of free space on those machines. That error seems relatively new (at least looking week to week) -- we just need to establish if its really a regression or build-related. |
@ofrobots I've cleaned cache in case it was corrupt for some reason. |
I have seen this very error as a flake on a few PRs in the past week or so (maybe more):
If this is not file-system related, then it is puzzling why it is failing this way. I am assuming that the compiler and linker on this system are sane. At any rate this error doesn't seem related to the PRs I have seen it on. EDIT: could you also verify that |
All green: https://ci.nodejs.org/job/node-test-pull-request/3519/. For posterity, the |
Original commit message: [regexp] Fix case-insensitive matching for one-byte subjects. The bug occurs because we do not canonicalize character class ranges before adding case equivalents. While adding case equivalents, we abort early for one-byte subject strings, assuming that the ranges are sorted. Which they are not. R=marja@chromium.org BUG=v8:5199 Review-Url: https://codereview.chromium.org/2159683002 Cr-Commit-Position: refs/heads/master@{#37833} Fixes: #7708 PR-URL: #7834 Ref: #7833 Reviewed-By: targos - Michaël Zasso <mic.besace@gmail.com> Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: jasnell - James M Snell <jasnell@gmail.com> Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com>
Landed on |
Original commit message: [regexp] Fix case-insensitive matching for one-byte subjects. The bug occurs because we do not canonicalize character class ranges before adding case equivalents. While adding case equivalents, we abort early for one-byte subject strings, assuming that the ranges are sorted. Which they are not. R=marja@chromium.org BUG=v8:5199 Review-Url: https://codereview.chromium.org/2159683002 Cr-Commit-Position: refs/heads/master@{#37833} Fixes: nodejs/node#7708 PR-URL: nodejs/node#7834 Ref: nodejs/node#7833 Reviewed-By: targos - Michaël Zasso <mic.besace@gmail.com> Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: jasnell - James M Snell <jasnell@gmail.com> Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com>
Checklist
make -j4 test
(UNIX)Affected core subsystem(s)
deps V8
Description of change
Cherry-pick a51f429 from V8 upstream. Fixes case-insensitive regex problem in #7708.
/cc @nodejs/v8