-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
build: add asan check in Github action #31902
Conversation
Seems like a good idea but how long does |
https://github.com/nodejs/node/pull/31902/checks?check_run_id=460428978 took 1 hr 40mins. |
ASAN checks failed on first error (https://github.com/nodejs/node/runs/460428978?check_suite_focus=true), so I add |
@bnoordhuis @addaleax @richardlau Can you review this ? thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and thanks, this is useful!
To save everyone some clicking: it takes 205 minutes or about 3.5 hours for a full run. 1.5h is just the build itself though so maybe that can be sped up somehow.
PR-URL: #31902 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Landed in 3ec4b21 |
PR-URL: #31902 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: #31902 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: #31902 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This related to #30257
This currently only runs on linux only. Acutally windows has made some progress on
this too, see https://devblogs.microsoft.com/cppblog/addresssanitizer-asan-for-windows-with-msvc/, but it require cmake as build toolchain IIUC. Also current code asan checks broken on macOS, so I didn't add them.
currrent Docker Image source: https://github.com/gengjiawen/node-build/blob/master/Dockerfile.
todo
Hopefully we can remove
continue-on-error
in the future.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes