Skip to content
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

regexp crash on ppc64le #44055

Closed
kapouer opened this issue Jul 30, 2022 · 5 comments
Closed

regexp crash on ppc64le #44055

kapouer opened this issue Jul 30, 2022 · 5 comments
Labels
v8 engine Issues and PRs related to the V8 dependency.

Comments

@kapouer
Copy link
Contributor

kapouer commented Jul 30, 2022

Version

v18.7.0

Platform

Linux plummer 5.10.0-16-powerpc64le #1 SMP Debian 5.10.127-2 (2022-07-23) ppc64le GNU/Linux

Subsystem

v8 regexp

What steps will reproduce the bug?

Run the attached file which contains a very long regexp.

./node-v18.7.0-linux-ppc64le/bin/node crash_irregexp_ppc64el.js

crash_irregexp_ppc64el.js.txt

How often does it reproduce? Is there a required condition?

Always.

What is the expected behavior?

No crash, as on the other architectures.

What do you see instead?

#
# Fatal error in , line 0
# Check failed: scratch != no_reg.
#
#
#
#FailureMessage Object: 0x7fffc512d510
 1: 0x1083dbf4  [./node]
 2: 0x12137b9c V8_Fatal(char const*, ...) [./node]
 3: 0x11542b8c v8::internal::TurboAssembler::StoreU64(v8::internal::Register, v8::internal::MemOperand const&, v8::internal::Register) [./node]
 4: 0x11598650 v8::internal::RegExpMacroAssemblerPPC::WriteStackPointerToRegister(int) [./node]
 5: 0x1118e6bc v8::internal::ActionNode::Emit(v8::internal::RegExpCompiler*, v8::internal::Trace*) [./node]
 6: 0x111847cc v8::internal::RegExpCompiler::Assemble(v8::internal::Isolate*, v8::internal::RegExpMacroAssembler*, v8::internal::RegExpNode*, int, v8::internal::Handle<v8::internal::String>) [./node]
 7: 0x111b0db4 v8::internal::RegExpImpl::Compile(v8::internal::Isolate*, v8::internal::Zone*, v8::internal::RegExpCompileData*, v8::base::Flags<v8::internal::RegExpFlag, int>, v8::internal::Handle<v8::internal::String>, v8::internal::Handle<v8::internal::String>, bool, unsigned int&) [./node]
 8: 0x111b16c4 v8::internal::RegExpImpl::CompileIrregexp(v8::internal::Isolate*, v8::internal::Handle<v8::internal::JSRegExp>, v8::internal::Handle<v8::internal::String>, bool) [./node]
 9: 0x111b23ec v8::internal::RegExpImpl::IrregexpPrepare(v8::internal::Isolate*, v8::internal::Handle<v8::internal::JSRegExp>, v8::internal::Handle<v8::internal::String>) [./node]
10: 0x111b27a8 v8::internal::RegExpImpl::IrregexpExec(v8::internal::Isolate*, v8::internal::Handle<v8::internal::JSRegExp>, v8::internal::Handle<v8::internal::String>, int, v8::internal::Handle<v8::internal::RegExpMatchInfo>, v8::internal::RegExp::ExecQuirks) [./node]
11: 0x111e9658 v8::internal::Runtime_RegExpExec(int, unsigned long*, v8::internal::Isolate*) [./node]
12: 0x1174bec0  [./node]
Trace/breakpoint trap

Additional information

It seems upstream v8 has fixed a similar bug not so long ago (their fix is already in nodejs), maybe they overlooked something:
v8/v8@9227a8d

Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Model name: POWER8 (architected), altivec supported
Model: 2.1 (pvr 004b 0201)

@richardlau
Copy link
Member

Cc @nodejs/platform-ppc

@daeyeon daeyeon added the v8 engine Issues and PRs related to the V8 dependency. label Jul 31, 2022
@miladfarca
Copy link
Contributor

Thanks for reporting this issue, this patch should resolve it https://chromium-review.googlesource.com/c/v8/v8/+/3797832,
will back-port it once reviewed/landed upstream.

@kapouer
Copy link
Contributor Author

kapouer commented Aug 1, 2022

I recompiled node 18.6 with the patch and it doesn't crash anymore.

@kapouer
Copy link
Contributor Author

kapouer commented Aug 1, 2022

Also the test-ci-js suite pass.

@miladfarca
Copy link
Contributor

Glad to hear that, thanks for running the tests.

danielleadams pushed a commit that referenced this issue Aug 16, 2022
Original commit message:

    PPC: pass a scratch reg when using register_location

    Change-Id: I43e4a4cadc60e958d6c9d80e725a49a3e36d8ba9
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3797832
    Reviewed-by: Junliang Yan <junyan@redhat.com>
    Commit-Queue: Milad Farazmand <mfarazma@redhat.com>
    Cr-Commit-Position: refs/heads/main@{#82146}

Fixes: #44055
PR-URL: #44115
Refs: v8/v8@9861ce1
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
ruyadorno pushed a commit that referenced this issue Aug 23, 2022
Original commit message:

    PPC: pass a scratch reg when using register_location

    Change-Id: I43e4a4cadc60e958d6c9d80e725a49a3e36d8ba9
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3797832
    Reviewed-by: Junliang Yan <junyan@redhat.com>
    Commit-Queue: Milad Farazmand <mfarazma@redhat.com>
    Cr-Commit-Position: refs/heads/main@{#82146}

Fixes: #44055
PR-URL: #44115
Refs: v8/v8@9861ce1
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Fyko pushed a commit to Fyko/node that referenced this issue Sep 15, 2022
Original commit message:

    PPC: pass a scratch reg when using register_location

    Change-Id: I43e4a4cadc60e958d6c9d80e725a49a3e36d8ba9
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3797832
    Reviewed-by: Junliang Yan <junyan@redhat.com>
    Commit-Queue: Milad Farazmand <mfarazma@redhat.com>
    Cr-Commit-Position: refs/heads/main@{#82146}

Fixes: nodejs#44055
PR-URL: nodejs#44115
Refs: v8/v8@9861ce1
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants