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

src: fix incorrect SIGSEGV handling in NODE_USE_V8_WASM_TRAP_HANDLER #35282

Closed

Conversation

korniltsev
Copy link
Contributor

@korniltsev korniltsev commented Sep 21, 2020

Pass SA_SIGINFO to sa_flags so TrapWebAssemblyOrContinue is treated
as sa_sigaction, not sa_handler, otherwise siginfo_t* info contains
some garbage and wasm's SIGSEGV is passed to the node and the whole process
crashes.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

reproducer.zip

Pass SA_SIGINFO to sa_flags so the TrapWebAssemblyOrContinue is treated
as sa_sigaction, not sa_handler, otherwise siginfo_t* info contains
some garbage
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Sep 21, 2020
Copy link
Member

@richardlau richardlau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I included this in #34648 but that's held up with the test I added not working on all platforms.

@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 24, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 24, 2020
@nodejs-github-bot
Copy link
Collaborator

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 24, 2020
@richardlau richardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 26, 2020
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 26, 2020
@github-actions
Copy link
Contributor

Landed in ff02801

nodejs-github-bot pushed a commit that referenced this pull request Sep 26, 2020
Pass SA_SIGINFO to sa_flags so the TrapWebAssemblyOrContinue is treated
as sa_sigaction, not sa_handler, otherwise siginfo_t* info contains
some garbage

PR-URL: #35282
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
@github-actions github-actions bot closed this Sep 26, 2020
@korniltsev korniltsev deleted the korniltsev/wasm_sigsegv_fix branch September 27, 2020 05:57
MylesBorins pushed a commit that referenced this pull request Sep 29, 2020
Pass SA_SIGINFO to sa_flags so the TrapWebAssemblyOrContinue is treated
as sa_sigaction, not sa_handler, otherwise siginfo_t* info contains
some garbage

PR-URL: #35282
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Sep 29, 2020
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
Pass SA_SIGINFO to sa_flags so the TrapWebAssemblyOrContinue is treated
as sa_sigaction, not sa_handler, otherwise siginfo_t* info contains
some garbage

PR-URL: nodejs#35282
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants