Enable ASAN/UBSAN and fix issues detected by ASAN/UBSAN #1649

aljazdu · 2026-01-23T12:48:32Z

Purpose

ASAN/UBSAN were disabled. Enabling them made the tests fail, as multiple issues were detected. The underlying issues had to be fixed. This PR makes #1485 obsolete.

Specification

ASAN/UBSAN is enabled in the workflow, detected issues are fixed.

Dependencies & Potential Impact

None / not applicable

Deployment Plan

None / not applicable

Testing & Validation

This was tested using HIL tests with enabled ASAN/UBSAN.

…gs for schema names

…be loaded in the LD_PRELOAD

tests/Dockerfile

asahtik · 2026-01-27T11:14:46Z

src/bspatch/bspatch.c

+        }

        /* Adjust pointers */
        newpos += ctrl[1];


TBH I'm not really sure what's going on in this file, but should newpos and oldpos be updated in the if block?

asahtik · 2026-01-27T11:31:12Z

CMakeLists.txt

 include(cmake/depthaiOptions.cmake)
+if(CMAKE_TOOLCHAIN_FILE)
+    message(STATUS "Including toolchain file: ${CMAKE_TOOLCHAIN_FILE}")
+    include("${CMAKE_TOOLCHAIN_FILE}")


Is this necessary? Isn't CMAKE_TOOLCHAIN_FILE automatically included?

asahtik

Left some comments. I'm not an expert in cmake so feel free to push back.

Copilot

Pull request overview

This PR enables ASAN (AddressSanitizer) and UBSAN (UndefinedBehaviorSanitizer) in the CI/CD workflow and fixes multiple issues detected by these sanitizers. The changes include bug fixes for undefined behavior, CMake configuration updates to support sanitizer toolchains, test infrastructure updates to suppress false positives and known external library leaks, and dependency updates to sanitizer-enabled versions.

Changes:

Fixed undefined behavior issues in core code (integer overflow in readIntLE, incorrect OpenCV template type usage, logic issues in bspatch)
Updated CMake configuration to support ASAN/UBSAN/TSAN toolchains with proper flag management
Added test infrastructure for leak detection suppressions and ASAN options configuration
Updated dependencies (XLink, RVC4 device firmware) to versions compatible with sanitizers

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/src/onhost_tests/replay_test.cpp	Added ASAN options to disable container overflow detection for protobuf false positives
tests/src/ondevice_tests/neural_depth_node_test.cpp	Added ASAN options to disable container overflow detection for protobuf false positives
tests/run_tests.py	Added LSAN suppressions file loading for test execution
tests/lsan.supp	Created suppressions file for known external library leaks (Mesa, GLib)
tests/Dockerfile	Fixed CMAKE_TOOLCHAIN_FILE variable name typo
src/utility/ProtoSerialize.cpp	Replaced descriptor()->full_name() calls with hardcoded strings to avoid ASAN issues
src/pipeline/datatype/StreamMessageParser.cpp	Fixed readIntLE to use proper unsigned casts and bit operations instead of multiplication
src/bspatch/bspatch.c	Added NULL check before memcpy in extra block handling
examples/cpp/SpatialLocationCalculator/spatial_location_calculator.cpp	Fixed OpenCV template type from float to uint16_t for depth frame access
cmake/toolchain/tsan.cmake	Added sanitizer flag configuration and debug message
cmake/toolchain/asan-ubsan.cmake	Added sanitizer flag configuration
cmake/sanitizers/FindTSan.cmake	Removed extra blank line
cmake/sanitizers/FindASan.cmake	Added debug messages for sanitizer configuration
cmake/depthaiDependencies.cmake	Updated XLink dependency to sanitizer-compatible version
cmake/Depthai/DepthaiDeviceRVC4Config.cmake	Updated RVC4 device firmware version
bindings/python/CMakeLists.txt	Added libstdc++ to LD_PRELOAD for GCC ASAN support
CMakeLists.txt	Added early toolchain file inclusion and removed redundant sanitizer flag settings
.github/workflows/test_child.yml	Minor comment spacing adjustment

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-27T11:43:16Z

CMakeLists.txt

+if(CMAKE_TOOLCHAIN_FILE)
+    message(STATUS "Including toolchain file: ${CMAKE_TOOLCHAIN_FILE}")
+    include("${CMAKE_TOOLCHAIN_FILE}")
+else()
+    message(STATUS "No toolchain file specified, skipping include.")
+endif()


Manually including the toolchain file before the project() command is problematic. CMake automatically processes the toolchain file specified via CMAKE_TOOLCHAIN_FILE before the project() command runs. This manual include() will cause the toolchain file to be processed twice: once here via include(), and once automatically by CMake when project() is called. This can lead to flags being set multiple times and unexpected behavior. The toolchain file should only be specified via -DCMAKE_TOOLCHAIN_FILE= on the command line or in presets, and never manually included.

Suggested change

if(CMAKE_TOOLCHAIN_FILE)

message(STATUS "Including toolchain file: ${CMAKE_TOOLCHAIN_FILE}")

include("${CMAKE_TOOLCHAIN_FILE}")

else()

message(STATUS "No toolchain file specified, skipping include.")

endif()

Copilot · 2026-01-27T11:43:17Z

src/bspatch/bspatch.c

+        if(ctrl[1] > 0 && p_decompressed_block[EXTRA_BLOCK] != NULL) {
+            memcpy(new + newpos, p_decompressed_block[EXTRA_BLOCK], ctrl[1]);
+            p_decompressed_block[EXTRA_BLOCK] += ctrl[1];
+        }

        /* Adjust pointers */
        newpos += ctrl[1];


There's a logic inconsistency with the NULL check. If ctrl[1] > 0 but p_decompressed_block[EXTRA_BLOCK] is NULL, the memcpy and pointer increment at line 218 are skipped. However, at line 222, newpos is unconditionally incremented by ctrl[1]. This means the output position advances even though no data was copied, potentially leaving uninitialized data in the output buffer. Either the NULL check should be removed (as the bounds check at line 210 should catch any issues), or the position adjustment at line 222 should be moved inside the conditional block at line 216.

Copilot · 2026-01-27T11:43:17Z

src/utility/ProtoSerialize.cpp

+    if(schemaName == "dai.proto.encoded_frame.EncodedFrame") {
        return DatatypeEnum::EncodedFrame;
-    } else if(schemaName == proto::imu_data::IMUData::descriptor()->full_name()) {
+    } else if(schemaName == "dai.proto.imu_data.IMUData") {
        return DatatypeEnum::IMUData;
-    } else if(schemaName == proto::image_annotations::ImageAnnotations::descriptor()->full_name()) {
+    } else if(schemaName == "dai.proto.image_annotations.ImageAnnotations") {
        return DatatypeEnum::ImgAnnotations;
-    } else if(schemaName == proto::img_detections::ImgDetections::descriptor()->full_name()) {
+    } else if(schemaName == "dai.proto.img_detections.ImgDetections") {
        return DatatypeEnum::ImgDetections;
-    } else if(schemaName == proto::img_frame::ImgFrame::descriptor()->full_name()) {
+    } else if(schemaName == "dai.proto.img_frame.ImgFrame") {
        return DatatypeEnum::ImgFrame;
-    } else if(schemaName == proto::point_cloud_data::PointCloudData::descriptor()->full_name()) {
+    } else if(schemaName == "dai.proto.point_cloud_data.PointCloudData") {
        return DatatypeEnum::PointCloudData;
-    } else if(schemaName == proto::spatial_img_detections::SpatialImgDetections::descriptor()->full_name()) {
+    } else if(schemaName == "dai.proto.spatial_img_detections.SpatialImgDetections") {
        return DatatypeEnum::SpatialImgDetections;


Replacing descriptor()->full_name() with hardcoded string literals. While this may fix an ASAN/UBSAN issue (likely related to static initialization order or descriptor lifetime), it makes the code more fragile. If the protobuf schema names change in the future, these hardcoded strings will not be updated automatically by the compiler, potentially causing runtime failures. Consider whether there's a safer alternative, such as caching the descriptor names after initialization or using constexpr string constants defined alongside the proto definitions.

Copilot · 2026-01-27T11:43:17Z

cmake/sanitizers/FindASan.cmake

+    message(STATUS "SANITIZE_THREAD  = ${SANITIZE_THREAD}")
+    message(STATUS "SANITIZE_ADDRESS  = ${SANITIZE_ADDRESS}")


These debug STATUS messages were added but should be removed before merging to production. Debug messages like these are useful during development but add noise to the build output in production builds. Consider using CMAKE_MESSAGE_LOG_LEVEL or removing these messages once the sanitizer configuration is verified to work correctly.

Suggested change

message(STATUS "SANITIZE_THREAD = ${SANITIZE_THREAD}")

message(STATUS "SANITIZE_ADDRESS = ${SANITIZE_ADDRESS}")

Copilot · 2026-01-27T11:43:18Z

cmake/toolchain/tsan.cmake

@@ -1,3 +1,4 @@
+message(STATUS ">>> Toolchain loaded: ${CMAKE_CURRENT_LIST_FILE}")


This debug STATUS message was added but should be removed before merging to production. Debug messages like this are useful during development but add noise to the build output in production builds.

Suggested change

message(STATUS ">>> Toolchain loaded: ${CMAKE_CURRENT_LIST_FILE}")

Danilo and others added 22 commits December 11, 2025 10:54

add variables for device sanitizers

33059d4

fix including sanitizers

20269cc

remove debug messages

1ef8542

add debug lines

e0d48fb

run santizier tests

c5ef6f3

adding back deopthai option

7535561

empty commit

b31a3b5

Update test_child.yml

b6289b7

Merge branch 'device_sanitizers' into fix/asan_ubsan_sanitizing

8fa02f2

Added a nullcheck before memcpy inside bspatch.c

a95c62f

Fixed integer overflow inside StreamMessageParser - readIntLE()

daf2bc7

Updated Xlink commit, schemaNameToDatatype() now uses hardcoded strin…

f88842e

…gs for schema names

Temporarily disabled tsan tests

033a30b

Bump XLink commit

bc30fe4

Bump FW

64c25bc

Added suppression for external libGLX_mesa.so, libstdc++.so will now …

725cb85

…be loaded in the LD_PRELOAD

Updated supp file, minor fix in SpatialLocationCalculator example

3b69eaa

Bump FW

1d69cbc

Merge branch 'develop' into fix/asan_ubsan_sanitizing

041abd6

Merge branch 'develop' into fix/asan_ubsan_sanitizing

36147fa

Bump FW

7b8bdef

Disabled container-overflow detection inside neural_depth_node_test

7389a03

aljazdu self-assigned this Jan 23, 2026

aljazdu requested review from danilo-pejovic and moratom January 23, 2026 12:48

aljazdu added the testable PR is ready to be tested label Jan 23, 2026

moratom requested a review from asahtik January 26, 2026 09:57

asahtik reviewed Jan 27, 2026

View reviewed changes

tests/Dockerfile Show resolved Hide resolved

asahtik reviewed Jan 27, 2026

View reviewed changes

asahtik requested a review from Copilot January 27, 2026 11:35

Copilot started reviewing on behalf of asahtik January 27, 2026 11:35 View session

Copilot AI reviewed Jan 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable ASAN/UBSAN and fix issues detected by ASAN/UBSAN #1649

Enable ASAN/UBSAN and fix issues detected by ASAN/UBSAN #1649

Uh oh!

aljazdu commented Jan 23, 2026

Uh oh!

Uh oh!

asahtik Jan 27, 2026

Uh oh!

asahtik Jan 27, 2026

Uh oh!

asahtik left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		message(STATUS "SANITIZE_THREAD = ${SANITIZE_THREAD}")
		message(STATUS "SANITIZE_ADDRESS = ${SANITIZE_ADDRESS}")

		@@ -1,3 +1,4 @@
		message(STATUS ">>> Toolchain loaded: ${CMAKE_CURRENT_LIST_FILE}")

Enable ASAN/UBSAN and fix issues detected by ASAN/UBSAN #1649

Are you sure you want to change the base?

Enable ASAN/UBSAN and fix issues detected by ASAN/UBSAN #1649

Uh oh!

Conversation

aljazdu commented Jan 23, 2026

Purpose

Specification

Dependencies & Potential Impact

Deployment Plan

Testing & Validation

Uh oh!

Uh oh!

asahtik Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

asahtik Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

asahtik left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants