Skip to content

Commit

Permalink
test: simplify ASan build checks
Browse files Browse the repository at this point in the history
Always use `process.config.variables.asan`.
This removes the need for a special ASAN env var.

PR-URL: #52430
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
  • Loading branch information
targos authored and marco-ippolito committed May 3, 2024
1 parent 17d5ba9 commit 5b758b9
Show file tree
Hide file tree
Showing 8 changed files with 15 additions and 15 deletions.
1 change: 0 additions & 1 deletion .github/workflows/test-asan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ jobs:
CXX: clang++
LINK: clang++
CONFIG_FLAGS: --enable-asan
ASAN: true
steps:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
with:
Expand Down
4 changes: 2 additions & 2 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ const isFreeBSD = process.platform === 'freebsd';
const isOpenBSD = process.platform === 'openbsd';
const isLinux = process.platform === 'linux';
const isOSX = process.platform === 'darwin';
const isAsan = process.env.ASAN !== undefined;
const isASan = process.config.variables.asan === 1;
const isPi = (() => {
try {
// Normal Raspberry Pi detection is to find the `Raspberry Pi` string in
Expand Down Expand Up @@ -960,7 +960,7 @@ const common = {
hasMultiLocalhost,
invalidArgTypeHelper,
isAlive,
isAsan,
isASan,
isDumbTerminal,
isFreeBSD,
isLinux,
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-crypto-dh-leak.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
if (process.config.variables.asan)
common.skip('ASAN messes with memory measurements');
if (common.isASan)
common.skip('ASan messes with memory measurements');

const assert = require('assert');
const crypto = require('crypto');
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-crypto-secure-heap.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ if (!common.hasCrypto)
if (common.isWindows)
common.skip('Not supported on Windows');

if (process.config.variables.asan)
common.skip('ASAN does not play well with secure heap allocations');
if (common.isASan)
common.skip('ASan does not play well with secure heap allocations');

const assert = require('assert');
const { fork } = require('child_process');
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-strace-openat-openssl.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ if (!common.hasCrypto)
common.skip('missing crypto');
if (!common.isLinux)
common.skip('linux only');
if (common.isAsan)
if (common.isASan)
common.skip('strace does not work well with address sanitizer builds');
if (spawnSync('strace').error !== undefined) {
common.skip('missing strace');
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-v8-serialize-leak.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ for (let i = 0; i < 1000000; i++) {
async function main() {
await common.gcUntil('RSS should go down', () => {
const after = process.memoryUsage.rss();
if (process.config.variables.asan) {
console.log(`asan: before=${before} after=${after}`);
if (common.isASan) {
console.log(`ASan: before=${before} after=${after}`);
return after < before * 10;
} else if (process.config.variables.node_builtin_modules_path) {
console.log(`node_builtin_modules_path: before=${before} after=${after}`);
Expand Down
4 changes: 2 additions & 2 deletions test/pummel/test-vm-memleak.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@

const common = require('../common');

if (process.config.variables.asan) {
common.skip('ASAN messes with memory measurements');
if (common.isASan) {
common.skip('ASan messes with memory measurements');
}

const assert = require('assert');
Expand Down
7 changes: 4 additions & 3 deletions tools/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1588,8 +1588,9 @@ def get_env_type(vm, options_type, context):
return env_type


def get_asan_state():
return "on" if os.environ.get('ASAN') is not None else "off"
def get_asan_state(vm, context):
asan = Execute([vm, '-p', 'process.config.variables.asan'], context).stdout
return "on" if asan == "1" else "off"


def Main():
Expand Down Expand Up @@ -1684,7 +1685,7 @@ def Main():
'system': utils.GuessOS(),
'arch': vmArch,
'type': get_env_type(vm, options.type, context),
'asan': get_asan_state(),
'asan': get_asan_state(vm, context),
}
test_list = root.ListTests([], path, context, arch, mode)
unclassified_tests += test_list
Expand Down

0 comments on commit 5b758b9

Please sign in to comment.