build,tools: add test-ubsan ci

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 · May 3, 2024 · 93bddb5 · 93bddb5
1 parent 6930205
commit 93bddb5
Show file tree

Hide file tree

Showing 7 changed files with 110 additions and 2 deletions.
diff --git a/.github/workflows/test-ubsan.yml 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 } },
+      );
     });
   }
 });