workflow: add test-ubsan ci by RafaelGSS · Pull Request #46297 · nodejs/node · GitHub

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

Review requested:

@nodejs/actions
@nodejs/gyp

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

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.

@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?

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

Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> Co-Authored-By: Santiago Gimeno <santiago.gimeno@gmail.com>

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?

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

Thanks for setting this up!

Landed in ba06c5c

Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> Co-Authored-By: Santiago Gimeno <santiago.gimeno@gmail.com> PR-URL: #46297 Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Jan Krems <jan.krems@gmail.com>

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 <shelley.vohr@gmail.com> 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 <shelley.vohr@gmail.com>

* 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 <zcbenz@gmail.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> 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 3e2e67b Compare January 20, 2023 20:26

bnoordhuis reviewed Jan 21, 2023

View reviewed changes

Comment thread common.gypi Outdated

RafaelGSS force-pushed the build/include-ubsan branch from 3e2e67b to 5710dcf Compare 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 25b9f77 Compare February 4, 2023 15:40

targos reviewed Feb 5, 2023

View reviewed changes

Comment thread suppresions.supp Outdated

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 1d88cb1 Compare August 12, 2023 02:48

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

build,tools: add test-ubsan ci

c9f3f96

Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> Co-Authored-By: Santiago Gimeno <santiago.gimeno@gmail.com>

RafaelGSS force-pushed the build/include-ubsan branch from baa0456 to c9f3f96 Compare 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 f989175 Compare 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-actions Bot 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 outstanding review comments, 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:main Mar 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

workflow: add test-ubsan ci by RafaelGSS · Pull Request #46297 · nodejs/node · GitHub

Uh oh!

Uh oh!

workflow: add test-ubsan ci - #46297

workflow: add test-ubsan ci#46297
nodejs-github-bot merged 10 commits into
nodejs:mainfrom
RafaelGSS:build/include-ubsan

RafaelGSS commented Jan 20, 2023 •
edited
Loading

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

Uh oh!

nodejs-github-bot commented Mar 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

workflow: add test-ubsan ci by RafaelGSS · Pull Request #46297 · nodejs/node · GitHub

Uh oh!

Uh oh!

workflow: add test-ubsan ci - #46297

workflow: add test-ubsan ci#46297nodejs-github-bot merged 10 commits intonodejs:mainnodejs/node:mainfrom RafaelGSS:build/include-ubsanRafaelGSS/node:build/include-ubsanCopy head branch name to clipboard

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

workflow: add test-ubsan ci#46297
nodejs-github-bot merged 10 commits into
nodejs:mainfrom
RafaelGSS:build/include-ubsan

RafaelGSS commented Jan 20, 2023 •
edited
Loading