workflow: add test-ubsan ci#46297

RafaelGSS · 2023-01-20T20:23:13Z

Fixes#46062

small-icu isn't ubsan-clean, I have added it to the suppressions file.

nodejs-github-bot · 2023-01-20T20:23:18Z

Review requested:

@nodejs/actions
@nodejs/gyp

common.gypi

targos · 2023-01-22T16:39:52Z

Example of an error that comes up a lot:

2023-01-22T16:07:27.6738113Z === release test-crypto-dh-leak === 2023-01-22T16:07:27.6738530Z Path: parallel/test-crypto-dh-leak 2023-01-22T16:07:27.6746530Z ##[error]--- stderr --- ../deps/v8/src/api/api-arguments-inl.h:332:3: runtime error: call to function v8::internal::Accessors::ReconfigureToDataProperty(v8::Local<v8::Name>, v8::Local<v8::Value>, v8::PropertyCallbackInfo<v8::Boolean> const&) through pointer to incorrect function type 'void (*)(v8::Local<v8::Name>, v8::Local<v8::Value>, const v8::PropertyCallbackInfo<void> &)' (/home/runner/work/node/node/out/Release/node+0x1e43970): note: v8::internal::Accessors::ReconfigureToDataProperty(v8::Local<v8::Name>, v8::Local<v8::Value>, v8::PropertyCallbackInfo<v8::Boolean> const&) defined here SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../deps/v8/src/api/api-arguments-inl.h:332:3 in ../deps/v8/src/handles/global-handles.cc:853:3: runtime error: call to function node::BaseObject::MakeWeak()::$_0::__invoke(v8::WeakCallbackInfo<node::BaseObject> const&) through pointer to incorrect function type 'void (*)(const v8::WeakCallbackInfo<void> &)' (/home/runner/work/node/node/out/Release/node+0x1683d30): note: node::BaseObject::MakeWeak()::$_0::__invoke(v8::WeakCallbackInfo<node::BaseObject> const&) defined here SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../deps/v8/src/handles/global-handles.cc:853:3 in node:assert:400 throw err; ^ AssertionError [ERR_ASSERTION]: before=90697728 after=104456192 at Object.<anonymous> (/home/runner/work/node/node/test/parallel/test-crypto-dh-leak.js:29:1) at Module._compile (node:internal/modules/cjs/loader:1246:14) at Module._extensions..js (node:internal/modules/cjs/loader:1300:10) at Module.load (node:internal/modules/cjs/loader:1103:32) at Module._load (node:internal/modules/cjs/loader:942:12) at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:83:12) at node:internal/main/run_main_module:23:47{generatedMessage: false, code: 'ERR_ASSERTION', actual: false, expected: true, operator: '==' } Node.js v20.0.0-pre Command: out/Release/node --expose-gc --noconcurrent_recompilation /home/runner/work/node/node/test/parallel/test-crypto-dh-leak.js

bnoordhuis · 2023-01-23T11:01:29Z

UBSan is technically right to complain because while V8's public type is v8::WeakCallbackInfo<T>, internally it stores and calls it as a pointer to v8::WeakCallbackInfo<void>.

I'm kind of surprised V8 doesn't have a suppression file for this; it has one for TSan but not UBSan. It's not our bug in any case so I would suggest simply suppressing it.

suppresions.supp

hybrist · 2023-07-10T13:57:14Z

@bnoordhuis It looks like the ubsan suppression for WeakCallbackInfo<T>/<void> is listed here: https://github.com/v8/v8/blob/master/tools/ubsan/ignorelist.txt#L9-L11. Or did you mean that it's missing additional places where that behavior is triggered?

bnoordhuis · 2023-07-10T20:01:00Z

I must've overlooked that file because that's the kind of suppression I expected to see.

Signed-off-by: RafaelGSS <[email protected]> Co-Authored-By: Santiago Gimeno <[email protected]>

RafaelGSS · 2024-03-13T20:41:00Z

Hey @nodejs/single-executable, could you help me with the error I'm getting on test-ubsan.yml? Should I skip test-single-executable-application-empty.js when --enable-ubsan?

nodejs-github-bot · 2024-03-14T18:57:22Z

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

hybrist

Thanks for setting this up!

nodejs-github-bot · 2024-03-15T17:49:52Z

Landed in ba06c5c

Signed-off-by: RafaelGSS <[email protected]> Co-Authored-By: Santiago Gimeno <[email protected]> PR-URL: nodejs#46297 Reviewed-By: Santiago Gimeno <[email protected]> Reviewed-By: Jan Krems <[email protected]>

Signed-off-by: RafaelGSS <[email protected]> Co-Authored-By: Santiago Gimeno <[email protected]> PR-URL: #46297 Reviewed-By: Santiago Gimeno <[email protected]> Reviewed-By: Jan Krems <[email protected]>

nodejs/node#46297

* chore: bump node in DEPS to v20.13.0 * crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL nodejs/node#52217 * test: skip test for dynamically linked OpenSSL nodejs/node#52542 * lib, url: add a `windows` option to path parsing nodejs/node#52509 * src: use dedicated routine to compile function for builtin CJS loader nodejs/node#52016 * test: mark test as flaky nodejs/node#52671 * build,tools: add test-ubsan ci nodejs/node#46297 * src: preload function for Environment nodejs/node#51539 * chore: fixup patch indices * deps: update c-ares to 1.28.1 nodejs/node#52285 * chore: handle updated filenames - nodejs/node#51999 - nodejs/node#51927 * chore: bump node in DEPS to v20.13.1 * events: extract addAbortListener for safe internal use nodejs/node#52081 * module: print location of unsettled top-level await in entry points nodejs/node#51999 * fs: add stacktrace to fs/promises nodejs/node#49849 * chore: update patches --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <[email protected]> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>

nodejs/node#46297

* chore: bump node in DEPS to v20.13.1 * chore: bump node in DEPS to v20.14.0 * crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL nodejs/node#52217 * test: skip test for dynamically linked OpenSSL nodejs/node#52542 * lib, url: add a `windows` option to path parsing nodejs/node#52509 * src: use dedicated routine to compile function for builtin CJS loader nodejs/node#52016 * test: mark test as flaky nodejs/node#52671 * build,tools: add test-ubsan ci nodejs/node#46297 * src: preload function for Environment nodejs/node#51539 * chore: fixup patch indices * deps: update c-ares to 1.28.1 nodejs/node#52285 * chore: handle updated filenames * events: extract addAbortListener for safe internal use nodejs/node#52081 * module: print location of unsettled top-level await in entry points nodejs/node#51999 * fs: add stacktrace to fs/promises nodejs/node#49849 * chore: fixup patch indices --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <[email protected]>

* chore: bump node in DEPS to v20.13.1 * chore: bump node in DEPS to v20.14.0 * chore: update build_add_gn_build_files.patch * chore: update patches * chore: update patches * build: encode non-ASCII Latin1 characters as one byte in JS2C nodejs/node#51605 * crypto: use EVP_MD_fetch and cache EVP_MD for hashes nodejs/node#51034 * chore: update filenames.json * chore: update patches * src: support configurable snapshot nodejs/node#50453 * test: remove test-domain-error-types flaky designation nodejs/node#51717 * src: avoid draining platform tasks at FreeEnvironment nodejs/node#51290 * chore: fix accidentally deleted v8 dep * lib: define FormData and fetch etc. in the built-in snapshot nodejs/node#51598 * chore: remove stray log * crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL nodejs/node#52217 * test: skip test for dynamically linked OpenSSL nodejs/node#52542 * lib, url: add a `windows` option to path parsing nodejs/node#52509 * src: use dedicated routine to compile function for builtin CJS loader nodejs/node#52016 * test: mark test as flaky nodejs/node#52671 * build,tools: add test-ubsan ci nodejs/node#46297 * src: preload function for Environment nodejs/node#51539 * deps: update c-ares to 1.28.1 nodejs/node#52285 * chore: fixup * events: extract addAbortListener for safe internal use nodejs/node#52081 * module: print location of unsettled top-level await in entry points nodejs/node#51999 * fs: add stacktrace to fs/promises nodejs/node#49849 * chore: fixup indices --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Cheng <[email protected]> Co-authored-by: Shelley Vohr <[email protected]> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>

nodejs-github-bot added build Issues and PRs related to build files or the CI. meta Issues and PRs related to the general management of the project. needs-ci PRs that need a full CI run. labels Jan 20, 2023

RafaelGSS force-pushed the build/include-ubsan branch from 7ae3556 to 3e2e67bCompare January 20, 2023 20:26

bnoordhuis reviewed Jan 21, 2023
View reviewed changes

common.gypi Outdated Show resolvedHide resolved

RafaelGSS force-pushed the build/include-ubsan branch from 3e2e67b to 5710dcfCompare January 21, 2023 14:19

targos marked this pull request as ready for review January 22, 2023 12:28

targos marked this pull request as draft January 22, 2023 12:28

RafaelGSS marked this pull request as ready for review February 4, 2023 15:39

RafaelGSS force-pushed the build/include-ubsan branch from 5710dcf to 25b9f77Compare February 4, 2023 15:40

targos reviewed Feb 5, 2023
View reviewed changes

suppresions.supp Outdated Show resolvedHide resolved

RafaelGSS mentioned this pull request Jul 2, 2023
Signed integer overflow in latest node 18 LTS release #48621
Closed

RafaelGSS force-pushed the build/include-ubsan branch from aab758c to 1d88cb1Compare August 12, 2023 02:48

RafaelGSS force-pushed the build/include-ubsan branch 2 times, most recently from 1a9d4c8 to baa0456Compare February 26, 2024 17:29

build,tools: add test-ubsan ci
c9f3f96
Signed-off-by: RafaelGSS <[email protected]> Co-Authored-By: Santiago Gimeno <[email protected]>

RafaelGSS force-pushed the build/include-ubsan branch from baa0456 to c9f3f96Compare February 26, 2024 17:54

RafaelGSS added 6 commits February 28, 2024 11:09

fixup! attempt to use absolute suppressions path
682e426

fixup! fixup! attempt to use absolute suppressions path
fc20e84

fixup! attempt to fix suppressions
cf76754

debug
71804c3

fixup! debug
c803294

workflow: adjust github env for ubsan options
f989175

RafaelGSS force-pushed the build/include-ubsan branch from 2f5808c to f989175Compare March 2, 2024 03:04

fixup! workflow: adjust github env for ubsan options
f4bbe7b

fixup! fixup! workflow: adjust github env for ubsan options
6cb6909

test: skip sea tests when ubsan=1
03a8b7f

RafaelGSS added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 14, 2024

github-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 14, 2024

RafaelGSS added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Mar 14, 2024

santigimeno approved these changes Mar 14, 2024
View reviewed changes

RafaelGSS added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 14, 2024

hybrist approved these changes Mar 15, 2024
View reviewed changes

RafaelGSS added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 15, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 15, 2024

nodejs-github-bot merged commit ba06c5c into nodejs:mainMar 15, 2024

RafaelGSS mentioned this pull request Apr 15, 2024
2024-04-24, Version 22.0.0 (Current) #52505
Merged

marco-ippolito mentioned this pull request May 2, 2024
v20.13.0 proposal #52793
Merged

codebytere added a commit to electron/electron that referenced this pull request May 8, 2024
build,tools: add test-ubsan ci
2531159
nodejs/node#46297

codebytere added a commit to electron/electron that referenced this pull request May 31, 2024
build,tools: add test-ubsan ci
157dc11
nodejs/node#46297

codebytere added a commit to electron/electron that referenced this pull request May 31, 2024
build,tools: add test-ubsan ci
a58eef6
nodejs/node#46297

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

workflow: add test-ubsan ci#46297

workflow: add test-ubsan ci #46297

RafaelGSS commented Jan 20, 2023•
edited
Loading

Uh oh!

nodejs-github-bot commented Jan 20, 2023

Uh oh!

Uh oh!

targos commented Jan 22, 2023

Uh oh!

bnoordhuis commented Jan 23, 2023

Uh oh!

Uh oh!

hybrist commented Jul 10, 2023

Uh oh!

bnoordhuis commented Jul 10, 2023

Uh oh!

RafaelGSS commented Mar 13, 2024

Uh oh!

nodejs-github-bot commented Mar 14, 2024

Uh oh!

hybrist left a comment

Uh oh!

nodejs-github-bot commented Mar 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

workflow: add test-ubsan ci#46297

workflow: add test-ubsan ci #46297

Conversation

RafaelGSS commented Jan 20, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Jan 20, 2023

Uh oh!

Uh oh!

targos commented Jan 22, 2023

Uh oh!

bnoordhuis commented Jan 23, 2023

Uh oh!

Uh oh!

hybrist commented Jul 10, 2023

Uh oh!

bnoordhuis commented Jul 10, 2023

Uh oh!

RafaelGSS commented Mar 13, 2024

Uh oh!

nodejs-github-bot commented Mar 14, 2024

Uh oh!

hybrist left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Mar 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

RafaelGSS commented Jan 20, 2023•
edited
Loading