From 93bddb598f5ec269cd53953ff48e817400285f58 Mon Sep 17 00:00:00 2001 From: Rafael Gonzaga Date: Fri, 15 Mar 2024 14:49:51 -0300 Subject: [PATCH] build,tools: add test-ubsan ci Signed-off-by: RafaelGSS Co-Authored-By: Santiago Gimeno PR-URL: https://github.com/nodejs/node/pull/46297 Reviewed-By: Santiago Gimeno Reviewed-By: Jan Krems --- .github/workflows/test-ubsan.yml | 63 ++++++++++++++++++++++ common.gypi | 24 +++++++++ configure.py | 7 +++ src/spawn_sync.cc | 2 +- suppressions.supp | 6 +++ test/common/sea.js | 4 ++ test/parallel/test-node-output-console.mjs | 6 ++- 7 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/test-ubsan.yml create mode 100644 suppressions.supp diff --git a/.github/workflows/test-ubsan.yml b/.github/workflows/test-ubsan.yml new file mode 100644 index 00000000000000..25f2481720021c --- /dev/null +++ b/.github/workflows/test-ubsan.yml @@ -0,0 +1,63 @@ +name: Test UBSan + +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + paths-ignore: + - .mailmap + - '**.md' + - AUTHORS + - doc/** + - .github/** + - '!.github/workflows/ubsan-asan.yml' + push: + branches: + - main + - canary + - v[0-9]+.x-staging + - v[0-9]+.x + paths-ignore: + - .mailmap + - '**.md' + - AUTHORS + - doc/** + - .github/** + - '!.github/workflows/ubsan-asan.yml' + +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} + cancel-in-progress: true + +env: + PYTHON_VERSION: '3.11' + FLAKY_TESTS: keep_retrying + +permissions: + contents: read + +jobs: + test-ubsan: + if: github.event.pull_request.draft == false + runs-on: ubuntu-20.04 + env: + CC: gcc + CXX: g++ + LINK: g++ + CONFIG_FLAGS: --enable-ubsan + steps: + - uses: actions/checkout@v3 + with: + persist-credentials: false + - name: Store suppressions path + run: | + echo "UBSAN_OPTIONS=suppressions=$GITHUB_WORKSPACE/suppressions.supp" >> $GITHUB_ENV + - name: Set up Python ${{ env.PYTHON_VERSION }} + uses: actions/setup-python@v4 + with: + python-version: ${{ env.PYTHON_VERSION }} + - name: Environment Information + run: npx envinfo + - name: Build + run: make build-ci -j2 V=1 + - name: Test + run: make run-ci -j2 V=1 TEST_CI_ARGS="-p actions -t 300 --measure-flakiness 9" diff --git a/common.gypi b/common.gypi index d05119d248223e..c49e2494659bf2 100644 --- a/common.gypi +++ b/common.gypi @@ -2,6 +2,7 @@ 'variables': { 'configuring_node%': 0, 'asan%': 0, + 'ubsan%': 0, 'werror': '', # Turn off -Werror in V8 build. 'visibility%': 'hidden', # V8's visibility setting 'target_arch%': 'ia32', # set v8's target architecture @@ -374,6 +375,29 @@ }], ], }], + ['ubsan == 1 and OS != "mac" and OS != "zos"', { + 'cflags+': [ + '-fno-omit-frame-pointer', + '-fsanitize=undefined', + ], + 'defines': [ 'UNDEFINED_SANITIZER'], + 'cflags!': [ '-fno-omit-frame-pointer' ], + 'ldflags': [ '-fsanitize=undefined' ], + }], + ['ubsan == 1 and OS == "mac"', { + 'xcode_settings': { + 'OTHER_CFLAGS+': [ + '-fno-omit-frame-pointer', + '-fsanitize=undefined', + '-DUNDEFINED_SANITIZER' + ], + }, + 'target_conditions': [ + ['_type!="static_library"', { + 'xcode_settings': {'OTHER_LDFLAGS': ['-fsanitize=undefined']}, + }], + ], + }], # The defines bellow must include all things from the external_v8_defines # list in v8/BUILD.gn. ['v8_enable_v8_checks == 1', { diff --git a/configure.py b/configure.py index ec4f31f69a46d3..11c0df455451d4 100755 --- a/configure.py +++ b/configure.py @@ -724,6 +724,12 @@ default=None, help='compile for Address Sanitizer to find memory bugs') +parser.add_argument('--enable-ubsan', + action='store_true', + dest='enable_ubsan', + default=None, + help='compile for Undefined Behavior Sanitizer') + parser.add_argument('--enable-static', action='store_true', dest='enable_static', @@ -1437,6 +1443,7 @@ def configure_node(o): o['variables']['linked_module_files'] = options.linked_module o['variables']['asan'] = int(options.enable_asan or 0) + o['variables']['ubsan'] = int(options.enable_ubsan or 0) if options.coverage: o['variables']['coverage'] = 'true' diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc index 472889e5e650db..d03803fe3abc5b 100644 --- a/src/spawn_sync.cc +++ b/src/spawn_sync.cc @@ -68,7 +68,7 @@ void SyncProcessOutputBuffer::OnRead(const uv_buf_t* buf, size_t nread) { size_t SyncProcessOutputBuffer::Copy(char* dest) const { - memcpy(dest, data_, used()); + if (dest != nullptr) memcpy(dest, data_, used()); return used(); } diff --git a/suppressions.supp b/suppressions.supp new file mode 100644 index 00000000000000..9dc89fb83df5b4 --- /dev/null +++ b/suppressions.supp @@ -0,0 +1,6 @@ +vptr:deps/icu-small/source/common/uloc_tag.cpp +vptr:deps/icu-small/source/common/unistr.cpp +shift-base:deps/v8/src/wasm/decoder.h +vptr:deps/icu-small/source/common/sharedobject.cpp +vptr:deps/icu-small/source/i18n/coll.cpp +nonnull-attribute:deps/v8/src/snapshot/snapshot-source-sink.h diff --git a/test/common/sea.js b/test/common/sea.js index c726829cd2b6de..f220fbe723332d 100644 --- a/test/common/sea.js +++ b/test/common/sea.js @@ -46,6 +46,10 @@ function skipIfSingleExecutableIsNotSupported() { } } + if (process.config.variables.ubsan) { + common.skip('UndefinedBehavior Sanitizer is not supported'); + } + tmpdir.refresh(); // The SEA tests involve making a copy of the executable and writing some fixtures diff --git a/test/parallel/test-node-output-console.mjs b/test/parallel/test-node-output-console.mjs index 5a1b9feb6c8bed..f995c170540ffa 100644 --- a/test/parallel/test-node-output-console.mjs +++ b/test/parallel/test-node-output-console.mjs @@ -31,7 +31,11 @@ describe('console output', { concurrency: true }, () => { .transform(snapshot.replaceWindowsLineEndings, snapshot.replaceWindowsPaths, replaceStackTrace); for (const { name, transform, env } of tests) { it(name, async () => { - await snapshot.spawnAndAssert(fixtures.path(name), transform ?? defaultTransform, { env }); + await snapshot.spawnAndAssert( + fixtures.path(name), + transform ?? defaultTransform, + { env: { ...env, ...process.env } }, + ); }); } });