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.

Sign up for GitHub

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

Jump to bottom

src: handle wasm out of bound in osx will raise SIGBUS correctly #46561

Merged
merged 4 commits into from
Jun 12, 2023
Merged

src: handle wasm out of bound in osx will raise SIGBUS correctly #46561

merged 4 commits into from
Jun 12, 2023

Conversation

HerrCai0907
Copy link
Contributor

fix: #46559
OSX will raise both SIGBUS and SIGSEGV when out of bound memory visit, This commit set sigaction in OSX for two signals to handle this.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Feb 8, 2023
addaleax
addaleax approved these changes Feb 8, 2023
View reviewed changes
src/node.cc Outdated Show resolved Hide resolved
@addaleax addaleax added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 8, 2023
@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Feb 8, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Feb 8, 2023

Failed to start CI
- Validating Jenkins credentials
✖  Jenkins credentials invalid
https://github.com/nodejs/node/actions/runs/4125736345

@HerrCai0907
Copy link
Contributor Author

Is it possible to re-run the CI?

@legendecas
Copy link
Member

legendecas commented Feb 13, 2023 •
edited
Loading

CI has been locked down for Feb 14 security releases.

@mhdawson mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 2, 2023
@mhdawson
Copy link
Member

mhdawson commented Mar 2, 2023

Requested new CI

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 2, 2023
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/50172/

This was referenced Mar 3, 2023
Open
Open
Open
Open
Open
Open
@aduh95 aduh95 added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Mar 9, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 9, 2023
This was referenced Mar 10, 2023
Open
Open
Open
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52193/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52201/

This was referenced Jun 11, 2023
Open
Open
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52216/

@HerrCai0907
Copy link
Contributor Author

Could it land now?

@RaisinTen RaisinTen added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jun 12, 2023
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jun 12, 2023
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/46561
✔  Done loading data for nodejs/node/pull/46561
----------------------------------- PR info ------------------------------------
Title      src: handle wasm out of bound in osx will raise SIGBUS correctly (#46561)
Author     Congcong Cai  (@HerrCai0907, first-time contributor)
Branch     HerrCai0907:fix/sigbus-in-mac-wasm -> nodejs:main
Labels     c++, author ready, commit-queue-squash
Commits    4
 - src: handle wasm out of bound in osx will raise SIGBUS correctly
 - format
 - add TODO mark
 - Update test/parallel/test-wasm-memory-out-of-bound.js
Committers 2
 - Congcong Cai 
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/46561
Fixes: https://github.com/nodejs/node/issues/46559
Reviewed-By: Anna Henningsen 
Reviewed-By: James M Snell 
Reviewed-By: Darshan Sen 
Reviewed-By: Minwoo Jung 
Reviewed-By: Michael Dawson 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/46561
Fixes: https://github.com/nodejs/node/issues/46559
Reviewed-By: Anna Henningsen 
Reviewed-By: James M Snell 
Reviewed-By: Darshan Sen 
Reviewed-By: Minwoo Jung 
Reviewed-By: Michael Dawson 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - Update test/parallel/test-wasm-memory-out-of-bound.js
   ℹ  This PR was created on Wed, 08 Feb 2023 09:50:45 GMT
   ✔  Approvals: 5
   ✔  - Anna Henningsen (@addaleax): https://github.com/nodejs/node/pull/46561#pullrequestreview-1289424314
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/46561#pullrequestreview-1290258481
   ✔  - Darshan Sen (@RaisinTen) (TSC): https://github.com/nodejs/node/pull/46561#pullrequestreview-1290656679
   ✔  - Minwoo Jung (@JungMinu): https://github.com/nodejs/node/pull/46561#pullrequestreview-1291233080
   ✔  - Michael Dawson (@mhdawson) (TSC): https://github.com/nodejs/node/pull/46561#pullrequestreview-1457624085
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2023-06-12T02:27:52Z: https://ci.nodejs.org/job/node-test-pull-request/52216/
- Querying data for job/node-test-pull-request/52216/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/5244996345

@RaisinTen RaisinTen added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jun 12, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 12, 2023
@nodejs-github-bot nodejs-github-bot merged commit ed92b1f into nodejs:main Jun 12, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in ed92b1f

@github-actions github-actions bot mentioned this pull request Jun 13, 2023
Open
22 tasks
@HerrCai0907 HerrCai0907 deleted the fix/sigbus-in-mac-wasm branch June 13, 2023 07:10
This was referenced Jun 14, 2023
Open
Open
@github-actions github-actions bot mentioned this pull request Jun 21, 2023
Open
29 tasks
RafaelGSS pushed a commit that referenced this pull request Jul 3, 2023
@HerrCai0907 @RafaelGSS
src: handle wasm out of bound in osx will raise SIGBUS correctly
6591826 
fix: #46559
OSX will raise both SIGBUS and SIGSEGV when out of bound memory visit,
This commit set sigaction in OSX for two signals to handle this.

PR-URL: #46561
Fixes: #46559
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Jul 3, 2023
Merged
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@HerrCai0907 @Ceres6
src: handle wasm out of bound in osx will raise SIGBUS correctly
140f17b 
fix: nodejs#46559
OSX will raise both SIGBUS and SIGSEGV when out of bound memory visit,
This commit set sigaction in OSX for two signals to handle this.

PR-URL: nodejs#46561
Fixes: nodejs#46559
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@HerrCai0907 @Ceres6
src: handle wasm out of bound in osx will raise SIGBUS correctly
ad17252 
fix: nodejs#46559
OSX will raise both SIGBUS and SIGSEGV when out of bound memory visit,
This commit set sigaction in OSX for two signals to handle this.

PR-URL: nodejs#46561
Fixes: nodejs#46559
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
ruyadorno pushed a commit that referenced this pull request Sep 7, 2023
@HerrCai0907 @ruyadorno
src: handle wasm out of bound in osx will raise SIGBUS correctly
cb3abcb 
fix: #46559
OSX will raise both SIGBUS and SIGSEGV when out of bound memory visit,
This commit set sigaction in OSX for two signals to handle this.

PR-URL: #46561
Fixes: #46559
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@ruyadorno ruyadorno mentioned this pull request Sep 8, 2023
Merged
ruyadorno pushed a commit that referenced this pull request Sep 13, 2023
@HerrCai0907 @ruyadorno
src: handle wasm out of bound in osx will raise SIGBUS correctly
6c72622 
fix: #46559
OSX will raise both SIGBUS and SIGSEGV when out of bound memory visit,
This commit set sigaction in OSX for two signals to handle this.

PR-URL: #46561
Fixes: #46559
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@RaisinTen RaisinTen RaisinTen approved these changes

@mhdawson mhdawson mhdawson approved these changes

@JungMinu JungMinu JungMinu approved these changes

@addaleax addaleax addaleax approved these changes

Assignees
No one assigned
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++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

run wasm out of memory in mac causes node crash due to SIGBUS