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

test: allow SIGBUS in signal-handler abort test #47851

Merged
merged 3 commits into from
May 5, 2023

Conversation

targos
Copy link
Member

@targos targos commented May 4, 2023

FreeBSD uses SIGBUS after update to v12.4.

Refs: nodejs/build#3134

FreeBSD uses SIGBUS after update to v12.4.

Refs: nodejs/build#3134
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels May 4, 2023
@nodejs-github-bot
Copy link
Collaborator

@targos
Copy link
Member Author

targos commented May 4, 2023

I forced the above CI run to be executed on https://ci.nodejs.org/manage/computer/test-digitalocean-freebsd12-x64-1/ (FreeBSD 12.4)

test/abort/test-signal-handler.js Outdated Show resolved Hide resolved
Co-authored-by: Santiago Gimeno <santiago.gimeno@gmail.com>
@targos targos added the freebsd Issues and PRs related to the FreeBSD platform. label May 4, 2023
@targos targos added request-ci Add this label to start a Jenkins CI on a PR. fast-track PRs that do not need to wait for 48 hours to land. labels May 4, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 4, 2023

Fast-track has been requested by @targos. Please 👍 to approve.

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

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@nodejs-github-bot
Copy link
Collaborator

@targos targos added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label May 5, 2023
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

Code change LGTM, no complaints, but I suggest replacing causeSegfault() with abort() because the former contains UB (literal nullptr dereference - compilers are allowed to optimize that away, volatile or not.)

@nodejs-github-bot
Copy link
Collaborator

@targos targos added the commit-queue Add this label to land a pull request using GitHub Actions. label May 5, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 5, 2023
@nodejs-github-bot nodejs-github-bot merged commit 3c82d48 into nodejs:main May 5, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 3c82d48

@targos targos deleted the test-freebsd-12.4 branch May 5, 2023 11:04
targos added a commit that referenced this pull request May 5, 2023
FreeBSD uses SIGBUS after update to v12.4.

Refs: nodejs/build#3134
PR-URL: #47851
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
targos added a commit that referenced this pull request May 5, 2023
FreeBSD uses SIGBUS after update to v12.4.

Refs: nodejs/build#3134
PR-URL: #47851
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
targos added a commit that referenced this pull request May 5, 2023
FreeBSD uses SIGBUS after update to v12.4.

Refs: nodejs/build#3134
PR-URL: #47851
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
targos added a commit that referenced this pull request May 5, 2023
FreeBSD uses SIGBUS after update to v12.4.

Refs: nodejs/build#3134
PR-URL: #47851
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@targos
Copy link
Member Author

targos commented May 5, 2023

@nodejs/collaborators if this test fails in your CI runs, you'll have to make sure they include this commit (either by rebasing or starting a fresh node-test-pull-request (don't "resume build"))

@richardlau richardlau added lts-watch-v16.x lts-watch-v18.x PRs that may need to be released in v18.x. and removed lts-watch-v18.x PRs that may need to be released in v18.x. lts-watch-v16.x labels May 5, 2023
denihs pushed a commit to meteor/node-v14-esm that referenced this pull request Jun 22, 2023
FreeBSD uses SIGBUS after update to v12.4.

Refs: nodejs/build#3134
PR-URL: nodejs#47851
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
denihs pushed a commit to meteor/node-v14-esm that referenced this pull request Jun 22, 2023
FreeBSD uses SIGBUS after update to v12.4.

Refs: nodejs/build#3134
PR-URL: nodejs#47851
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
denihs pushed a commit to meteor/node-v14-esm that referenced this pull request Jun 22, 2023
FreeBSD uses SIGBUS after update to v12.4.

Refs: nodejs/build#3134
PR-URL: nodejs#47851
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
erikolofsson pushed a commit to Malterlib/node that referenced this pull request Jun 26, 2023
FreeBSD uses SIGBUS after update to v12.4.

Refs: nodejs/build#3134
PR-URL: nodejs#47851
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
erikolofsson pushed a commit to Malterlib/node that referenced this pull request Jun 26, 2023
FreeBSD uses SIGBUS after update to v12.4.

Refs: nodejs/build#3134
PR-URL: nodejs#47851
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
denihs pushed a commit to meteor/node-v14-esm that referenced this pull request Jun 26, 2023
FreeBSD uses SIGBUS after update to v12.4.

Refs: nodejs/build#3134
PR-URL: nodejs#47851
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. fast-track PRs that do not need to wait for 48 hours to land. freebsd Issues and PRs related to the FreeBSD platform. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants