Skip to content

Commit 3e8a4ef

Browse files
authored
Add AddressSanitizer (ASan) memory testing for native addon (#1430)
Add ASan-based memory testing infrastructure to detect memory errors (leaks, use-after-free, type mismatches) in the C++ N-API addon code. Bug fixes found and resolved by ASan: - src/executor.cpp: Fix new-delete-type-mismatch in Executor::Stop(). The uv_close callback deleted via uv_handle_t* (96 bytes) but the object was allocated as uv_async_t (128 bytes). Fixed by casting back to uv_async_t* before delete. - src/rcl_context_bindings.cpp: Fix argv memory leak in Init(). RCPPUTILS_SCOPE_EXIT for FreeArgs was placed after early return paths, so argv was never freed on error. Moved it immediately after allocation. New files: - scripts/run_asan_test.sh: ASan test runner that builds with -fsanitize=address, sets up LD_PRELOAD/LSAN_OPTIONS, temporarily hides prebuilds to load the debug build, and auto-excludes tests from blocklist.json and asan_blocklist.json. Supports --no-build, --exclude, and specific test file arguments. - test/asan_blocklist.json: Tests excluded under ASan due to LD_PRELOAD incompatibility with child processes (npm, colcon, ros2 CLI) or known third-party issues (ref-napi heap-use-after-free). - .github/workflows/linux-x64-asan-test.yml: Standalone CI workflow running ASan tests on push/PR to develop (Jazzy + Node 24.X + x64). Other changes: - package.json: Add "test:asan" npm script. - suppr.txt: Add napi_create_reference and napi_create_external suppressions for module-level singleton allocations. - README.md: Add ASan CI badge to status table. Fix: #1429
1 parent 94de088 commit 3e8a4ef

8 files changed

Lines changed: 180 additions & 6 deletions

File tree

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
name: ASan Memory Test
2+
3+
on:
4+
push:
5+
branches:
6+
- develop
7+
pull_request:
8+
branches:
9+
- develop
10+
workflow_dispatch:
11+
12+
defaults:
13+
run:
14+
shell: bash
15+
16+
jobs:
17+
asan:
18+
runs-on: ubuntu-latest
19+
container:
20+
image: ubuntu:noble
21+
steps:
22+
- name: Setup Node.js 24.X
23+
uses: actions/setup-node@v6
24+
with:
25+
node-version: 24.X
26+
architecture: x64
27+
28+
- name: Setup ROS2 Jazzy
29+
uses: ros-tooling/setup-ros@v0.7
30+
with:
31+
required-ros-distributions: jazzy
32+
33+
- name: Install test dependencies
34+
run: |
35+
sudo apt install -y ros-jazzy-test-msgs ros-jazzy-mrpt-msgs
36+
37+
- uses: actions/checkout@v6
38+
39+
- name: Install npm dependencies
40+
run: |
41+
source /opt/ros/jazzy/setup.bash
42+
npm i
43+
44+
- name: Build and test with AddressSanitizer
45+
run: |
46+
source /opt/ros/jazzy/setup.bash
47+
npm run test:asan

README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22

33
[![npm](https://img.shields.io/npm/v/rclnodejs.svg)](https://www.npmjs.com/package/rclnodejs) [![Coverage Status](https://coveralls.io/repos/github/RobotWebTools/rclnodejs/badge.svg?branch=develop)](https://coveralls.io/github/RobotWebTools/rclnodejs?branch=develop) [![npm](https://img.shields.io/npm/dm/rclnodejs)](https://www.npmjs.com/package/rclnodejs) [![GitHub license](https://img.shields.io/github/license/RobotWebTools/rclnodejs.svg)](https://github.com/RobotWebTools/rclnodejs/blob/develop/LICENSE) [![node](https://img.shields.io/node/v/rclnodejs.svg)](https://nodejs.org/en/download/releases/) [![npm type definitions](https://img.shields.io/npm/types/rclnodejs)](https://www.npmjs.com/package/rclnodejs) [![code style: prettier](https://img.shields.io/badge/code_style-prettier-ff69b4.svg?style=flat-square)](https://github.com/prettier/prettier)
44

5-
| **ROS Distro\*** | **Status** |
6-
| :----------------------------------: | :--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------: |
7-
| Rolling<br>Kilted<br>Jazzy<br>Humble | [![Linux](https://github.com/RobotWebTools/rclnodejs/actions/workflows/linux-x64-push-test.yml/badge.svg?branch=develop)](https://github.com/RobotWebTools/rclnodejs/actions/workflows/linux-x64-push-test.yml?query=branch%3Adevelop)<br>[![Linux](https://github.com/RobotWebTools/rclnodejs/actions/workflows/linux-arm64-push-test.yml/badge.svg?branch=develop)](https://github.com/RobotWebTools/rclnodejs/actions/workflows/linux-arm64-push-test.yml?query=branch%3Adevelop)<br>[![Windows](https://github.com/RobotWebTools/rclnodejs/actions/workflows/windows-push-test.yml/badge.svg?branch=develop)](https://github.com/RobotWebTools/rclnodejs/actions/workflows/windows-push-test.yml?query=branch%3Adevelop) |
5+
| **ROS Distro\*** | **Status** |
6+
| :----------------------------------: | :-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------: |
7+
| Rolling<br>Kilted<br>Jazzy<br>Humble | [![Linux](https://github.com/RobotWebTools/rclnodejs/actions/workflows/linux-x64-push-test.yml/badge.svg?branch=develop)](https://github.com/RobotWebTools/rclnodejs/actions/workflows/linux-x64-push-test.yml?query=branch%3Adevelop)<br>[![Linux](https://github.com/RobotWebTools/rclnodejs/actions/workflows/linux-arm64-push-test.yml/badge.svg?branch=develop)](https://github.com/RobotWebTools/rclnodejs/actions/workflows/linux-arm64-push-test.yml?query=branch%3Adevelop)<br>[![Windows](https://github.com/RobotWebTools/rclnodejs/actions/workflows/windows-push-test.yml/badge.svg?branch=develop)](https://github.com/RobotWebTools/rclnodejs/actions/workflows/windows-push-test.yml?query=branch%3Adevelop)<br>[![ASan](https://github.com/RobotWebTools/rclnodejs/actions/workflows/linux-x64-asan-test.yml/badge.svg?branch=develop)](https://github.com/RobotWebTools/rclnodejs/actions/workflows/linux-x64-asan-test.yml?query=branch%3Adevelop) |
88

99
> **Note:** rclnodejs development and maintenance is limited to the ROS 2 LTS releases and the Rolling development branch
1010

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
"test": "nyc node --expose-gc ./scripts/run_test.js && tsd && npm install --no-save electron && node test/electron/run_test.js",
2929
"test-idl": "nyc node --expose-gc ./scripts/run_test.js --idl",
3030
"lint": "eslint && node ./scripts/cpplint.js",
31+
"test:asan": "bash scripts/run_asan_test.sh",
3132
"format": "clang-format -i -style=file ./src/*.cpp ./src/*.h && npx --yes prettier --write \"{lib,rosidl_gen,rostsd_gen,rosidl_parser,types,example,test,scripts,benchmark,rostsd_gen}/**/*.{js,md,ts}\" ./*.{js,md,ts}",
3233
"prepare": "husky",
3334
"coverage": "cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js",

scripts/run_asan_test.sh

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
#!/bin/bash
2+
# Run tests with AddressSanitizer (ASan/LSan) to detect memory leaks and errors
3+
# in the native N-API addon.
4+
#
5+
# Requires: ROS2 sourced, g++ with libasan
6+
#
7+
# Usage:
8+
# bash scripts/run_asan_test.sh # rebuild + run all tests
9+
# bash scripts/run_asan_test.sh test/test-node.js # rebuild + run specific test(s)
10+
# bash scripts/run_asan_test.sh --no-build test/test-node.js # skip rebuild
11+
# bash scripts/run_asan_test.sh --exclude test/test-serialization.js # exclude specific test(s)
12+
13+
set -e
14+
15+
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
16+
PROJECT_DIR="$(cd "$SCRIPT_DIR/.." && pwd)"
17+
cd "$PROJECT_DIR"
18+
19+
# Parse flags
20+
DO_BUILD=1
21+
ARGS=()
22+
EXCLUDES=()
23+
NEXT_IS_EXCLUDE=0
24+
for arg in "$@"; do
25+
if [[ $NEXT_IS_EXCLUDE -eq 1 ]]; then
26+
EXCLUDES+=("$arg")
27+
NEXT_IS_EXCLUDE=0
28+
elif [[ "$arg" == "--no-build" ]]; then
29+
DO_BUILD=0
30+
elif [[ "$arg" == "--exclude" ]]; then
31+
NEXT_IS_EXCLUDE=1
32+
else
33+
ARGS+=("$arg")
34+
fi
35+
done
36+
37+
if [[ $NEXT_IS_EXCLUDE -eq 1 ]]; then
38+
echo "Error: --exclude requires a test file argument"
39+
echo "Usage: bash scripts/run_asan_test.sh --exclude test/test-foo.js"
40+
exit 1
41+
fi
42+
43+
# Step 1: Build with ASan
44+
if [[ $DO_BUILD -eq 1 ]]; then
45+
echo "=== Building with AddressSanitizer ==="
46+
CXXFLAGS="-fsanitize=address" npx node-gyp -j 16 rebuild --debug
47+
fi
48+
49+
# Step 2: Locate libasan
50+
LIBASAN=$(g++ -print-file-name=libasan.so)
51+
if [[ "$LIBASAN" == "libasan.so" ]]; then
52+
# g++ returned just the name, try to find the full path
53+
LIBASAN=$(find /usr/lib* -name "libasan.so*" -type f 2>/dev/null | head -1)
54+
fi
55+
56+
if [[ -z "$LIBASAN" || ! -f "$LIBASAN" ]]; then
57+
echo "Warning: Could not find libasan.so, proceeding without LD_PRELOAD"
58+
echo "If you see 'ASan not the first DSO' errors, install libasan: sudo apt install libasan6"
59+
LIBASAN=""
60+
fi
61+
62+
# Step 3: Set up environment
63+
export LSAN_OPTIONS="suppressions=$PROJECT_DIR/suppr.txt"
64+
if [[ -n "$LIBASAN" ]]; then
65+
export LD_PRELOAD="$LIBASAN"
66+
fi
67+
68+
# Step 3.5: Temporarily hide prebuilds so the debug build is loaded
69+
PREBUILDS_DIR="$PROJECT_DIR/prebuilds"
70+
PREBUILDS_BAK="$PROJECT_DIR/.prebuilds_asan_bak"
71+
MOVED_PREBUILDS=0
72+
if [[ -d "$PREBUILDS_DIR" ]]; then
73+
if [[ -d "$PREBUILDS_BAK" ]]; then
74+
echo "Error: $PREBUILDS_BAK already exists (previous interrupted run?)."
75+
echo "Please remove it manually and retry: rm -rf $PREBUILDS_BAK"
76+
exit 1
77+
fi
78+
mv "$PREBUILDS_DIR" "$PREBUILDS_BAK"
79+
MOVED_PREBUILDS=1
80+
fi
81+
82+
# Restore prebuilds on exit (even on failure)
83+
cleanup() {
84+
if [[ $MOVED_PREBUILDS -eq 1 && -d "$PREBUILDS_BAK" ]]; then
85+
mv "$PREBUILDS_BAK" "$PREBUILDS_DIR"
86+
fi
87+
}
88+
trap cleanup EXIT
89+
90+
# Step 4: Build mocha exclude args from --exclude flags, blocklist.json, and asan_blocklist.json
91+
MOCHA_EXCLUDES=()
92+
for exc in "${EXCLUDES[@]}"; do
93+
MOCHA_EXCLUDES+=(--ignore "$exc")
94+
done
95+
96+
# Auto-exclude tests from blocklist.json (Linux entries) and asan_blocklist.json
97+
for blocklist in "$PROJECT_DIR/test/blocklist.json" "$PROJECT_DIR/test/asan_blocklist.json"; do
98+
if [[ -f "$blocklist" ]]; then
99+
while IFS= read -r test_file; do
100+
MOCHA_EXCLUDES+=(--ignore "test/$test_file")
101+
done < <(node -e "
102+
const bl = require('$blocklist');
103+
const entries = bl.Linux || bl;
104+
if (Array.isArray(entries)) entries.forEach(t => console.log(t));
105+
" 2>/dev/null)
106+
fi
107+
done
108+
109+
# Step 5: Run tests
110+
if [[ ${#ARGS[@]} -gt 0 ]]; then
111+
echo "=== Running ASan test: ${ARGS[*]} ==="
112+
node --expose-gc node_modules/.bin/mocha -r test/gc-on-exit.js "${MOCHA_EXCLUDES[@]}" "${ARGS[@]}"
113+
else
114+
echo "=== Running all ASan tests ==="
115+
node --expose-gc node_modules/.bin/mocha -r test/gc-on-exit.js "${MOCHA_EXCLUDES[@]}" 'test/test-*.js'
116+
fi
117+
118+
echo "=== ASan test complete ==="

src/executor.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ void Executor::Stop() {
116116
// Important Notice:
117117
// This might be called after Executor::~Executor()
118118
// Don't free Executor::async_ in Executor's dtor
119-
delete async;
119+
delete reinterpret_cast<uv_async_t*>(async);
120120
handle_closed = true;
121121
});
122122
while (!handle_closed) uv_run(uv_default_loop(), UV_RUN_ONCE);

src/rcl_context_bindings.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ Napi::Value Init(const Napi::CallbackInfo& info) {
5454
Napi::Array jsArgv = info[1].As<Napi::Array>();
5555
size_t argc = jsArgv.Length();
5656
char** argv = AbstractArgsFromNapiArray(jsArgv);
57+
RCPPUTILS_SCOPE_EXIT({ FreeArgs(argv, argc); });
5758

5859
// Set up the domain id.
5960
size_t domain_id = RCL_DEFAULT_DOMAIN_ID;
@@ -82,7 +83,6 @@ Napi::Value Init(const Napi::CallbackInfo& info) {
8283
RCL_RET_OK, rcl_logging_configure(&context->global_arguments, &allocator),
8384
rcl_get_error_string().str);
8485

85-
RCPPUTILS_SCOPE_EXIT({ FreeArgs(argv, argc); });
8686
return env.Undefined();
8787
}
8888

suppr.txt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
11
leak:librcl.so$
22
leak:ref-napi/prebuilds/linux-x64/node.napi.node$
3-
leak:^napi_module_register$
3+
leak:^napi_module_register$
4+
leak:^napi_create_reference$
5+
leak:^napi_create_external$

test/asan_blocklist.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
[
2+
"test-serialization.js",
3+
"test-message-generation-bin.js",
4+
"test-rosidl-message-generator.js",
5+
"test-type-description-service.js"
6+
]

0 commit comments

Comments
 (0)