8000 src: fix incorrect SIGSEGV handling in NODE_USE_V8_WASM_TRAP_HANDLER by korniltsev · Pull Request #35282 · nodejs/node · GitHub
[go: up one dir, main page]

Skip to content

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

Reviewers

@Trott Trott Trott approved these changes

@JungMinu JungMinu JungMinu approved these changes

@richardlau richardlau richardlau approved these changes

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

0