[RSDK-5863] Add Address Sanitizer (ASan) option for intel realsense module #38

hexbabe · 2024-04-26T14:15:24Z

Originally, we wanted to add CI for ASAN, but since CI runners don't come with Intel Realsenses + it is difficult to software emulate the Realsense + adding mocks & unit tests would be quite unwieldy and difficult, we are just gonna support ASAN/LSAN as a build option.

This PR enables developers to run something like canon -arch arm64 make clean appimage-arm64 SANITIZE=ON to build the module with address sanitizer on. This allows ASAN/LSAN to analyze the code as it runs with viam-server.

Testing

On linux/arm64

Commented out tjDestroy(handle); in camera_realsense.cpp encodeJPEG function to intentionally introduce a similar leak fixed in https://viam.atlassian.net/browse/RSDK-5869.

Ran canon -arch arm64 make clean appimage-arm64 SANITIZE=ON

4/26/2024, 2:12:53 PM error robot_server.modmanager.process.irs-local-mod_/home/rawr-ubuntu/Projects/viam-camera-realsense/packaging/appimages/deploy/viam-camera-realsense-latest-aarch64.AppImage.StdErr pexec/managed_process.go:252 \_ #3 0xaaaade725e94 in viam::realsense::CameraRealSense::get_image(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::shared_ptr<std::unordered_map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::shared_ptr<viam::sdk::ProtoType>, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::shared_ptr<viam::sdk::ProtoType> > > > > const&) (/tmp/.mount_viam-c7oFcxj/usr/bin/viam-camera-realsense+0x105e94)

4/26/2024, 2:12:53 PM error robot_server.modmanager.process.irs-local-mod_/home/rawr-ubuntu/Projects/viam-camera-realsense/packaging/appimages/deploy/viam-camera-realsense-latest-aarch64.AppImage.StdErr pexec/managed_process.go:252 \_ #2 0xaaaade712dc0 in (anonymous namespace)::encodeJPEGToResponse(unsigned char const*, unsigned int, unsigned int) (/tmp/.mount_viam-c7oFcxj/usr/bin/viam-camera-realsense+0xf2dc0)

4/26/2024, 2:12:53 PM error robot_server.modmanager.process.irs-local-mod_/home/rawr-ubuntu/Projects/viam-camera-realsense/packaging/appimages/deploy/viam-camera-realsense-latest-aarch64.AppImage.StdErr pexec/managed_process.go:252 \_ #1 0xffffa7f9ae90 in tjInitCompress (/tmp/.mount_viam-c7oFcxj/usr/lib/aarch64-linux-gnu/libturbojpeg.so.0+0x3ae90)

4/26/2024, 2:12:53 PM error robot_server.modmanager.process.irs-local-mod_/home/rawr-ubuntu/Projects/viam-camera-realsense/packaging/appimages/deploy/viam-camera-realsense-latest-aarch64.AppImage.StdErr pexec/managed_process.go:252 \_ #0 0xaaaade6cac90 in __interceptor_calloc (/tmp/.mount_viam-c7oFcxj/usr/bin/viam-camera-realsense+0xaac90)

4/26/2024, 2:12:53 PM error robot_server.modmanager.process.irs-local-mod_/home/rawr-ubuntu/Projects/viam-camera-realsense/packaging/appimages/deploy/viam-camera-realsense-latest-aarch64.AppImage.StdErr pexec/managed_process.go:252 \_ Direct leak of 939456 byte(s) in 504 object(s) allocated from:

4/26/2024, 2:12:53 PM error robot_server.modmanager.process.irs-local-mod_/home/rawr-ubuntu/Projects/viam-camera-realsense/packaging/appimages/deploy/viam-camera-realsense-latest-aarch64.AppImage.StdErr pexec/managed_process.go:252 \_
4/26/2024, 2:12:53 PM error robot_server.modmanager.process.irs-local-mod_/home/rawr-ubuntu/Projects/viam-camera-realsense/packaging/appimages/deploy/viam-camera-realsense-latest-aarch64.AppImage.StdErr pexec/managed_process.go:252 \_ ==110799==ERROR: LeakSanitizer: detected memory leaks

4/26/2024, 2:12:53 PM error robot_server.modmanager.process.irs-local-mod_/home/rawr-ubuntu/Projects/viam-camera-realsense/packaging/appimages/deploy/viam-camera-realsense-latest-aarch64.AppImage.StdErr pexec/managed_process.go:252 \_ =================================================================

integration/tests/all_test.go

packaging/appimages/realsense-integration-tests-aarch64.yml

randhid

Adding Bijan/nick s because I can't give it a thorough read through,, but lgtm!

randhid · 2024-04-26T16:53:33Z

For context @bhaney @nicksanford the criterea for this is to ensure that our devs don't introduce memory leaks, we're not going to battle test real sense's code for them and address issues on their side if there is ever a customer blocking bug.

CMakeLists.txt

nicksanford

@hexbabe can you please add in the PR description:

What manual testing have you done for the asan build?
On which CPU / Board / OS platforms?

nicksanford · 2024-04-26T17:04:15Z

CMakeLists.txt

+    add_compile_options(-fsanitize=address)
+    add_link_options(-fsanitize=address)
+    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -static-libasan")
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -static-libasan")
+    set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -static-libasan")
+    target_compile_options(viamrealsense PRIVATE -fsanitize=address)
+    target_link_options(viamrealsense PRIVATE -fsanitize=address)
+    target_compile_options(viam-camera-realsense PRIVATE -fsanitize=address)
+    target_link_options(viam-camera-realsense PRIVATE -fsanitize=address)


How do you know these are the correct options to enable ASan? Is there a guide you are working from?

You might want to consider adding:
-fsanitize-address-use-after-scope and -fno-omit-frame-pointer

see: https://github.com/viamrobotics/viam-cartographer/blob/ee4ea28bc5f981d9ddbd93aa7db6a2d1fb95e808/Makefile#L121

packaging/appimages/viam-camera-realsense-aarch64.yml

nicksanford · 2024-04-26T17:10:48Z

README.md

@@ -164,3 +164,6 @@ If you would like to try to gather all of the dependencies yourself and not use

 then do `make viam-camera-realsense` to compile the binary, and `make appimage` to create the AppImage.

+## Building with Address Sanitizer
+
+When developing, you also have the option to build the module with ASAN/LSAN enabled to help test for memory leaks. You can do so by running `canon -arch arm64 make clean appimage-arm64 SANITIZE=ON`. ASAN/LSAN logs will then be included as error logs in your robot logs.


Can you please add which platforms ASAN is supported on?

CMakeLists.txt

nicksanford · 2024-04-26T17:44:05Z

What C compiler(s) do we support on the intel realsense?

nicksanford · 2024-04-26T17:45:20Z

Can you confirm that there are no new dynamic lib deps added to the module after adding sanitizer?

hexbabe · 2024-04-26T17:51:13Z

What C compiler(s) do we support on the intel realsense?

From logs: -- The CXX compiler identification is GNU 12.2.0 So gcc.

The Realsense build process uses Docker with the image defined here

nicksanford · 2024-04-26T21:35:13Z

@hexbabe can you please test if the debug build works on all platforms the intel realsense module currently targets & add to the README if the debug build doesn't work on them?

nicksanford · 2024-04-29T16:02:00Z

@hexbabe can you please confirm that tests FAIL if there is a memory leak introduced on all platforms we support? I don't mean in CI, I mean wherever we support running tests (like on your laptop).

hexbabe · 2024-04-29T20:07:36Z

@hexbabe can you please confirm that tests FAIL if there is a memory leak introduced on all platforms we support? I don't mean in CI, I mean wherever we support running tests (like on your laptop).

@nicksanford

I am currently unable to get this ASAN/LSAN debug build to work on amd64. (For manager/PM mode Nick) Thoughts on how much time I should allocate to trying to get it to work?

If after the allocated time I am still unable, I can put on the README that the debug build is currently only verified to work on linux/arm64

…fno-omit-frame-pointer

hexbabe · 2024-04-29T20:39:44Z

@nicksanford I was wrong. By default, ASAN/LSAN does non-zero exit when a leak is detected (I think I was mislead bc viam-server would just restart the module).

Running: /home/rawr-ubuntu/Projects/viam-camera-realsense/packaging/appimages/deploy/realsense-integration-tests-latest-aarch64.AppImage -module /home/rawr-ubuntu/Projects/viam-camera-realsense/packaging/appimages/deploy/viam-camera-realsense-latest-aarch64.AppImage sorry for absolute paths lol this is the command to run integration tests on hardware

Logs:

...
    logger.go:130: 2024-04-29T20:38:01.003Z     ERROR   process.viam_realsense_/home/rawr-ubuntu/Projects/viam-camera-realsense/packaging/appimages/deploy/viam-camera-realsense-latest-aarch64.AppImage.StdErr  pexec/managed_process.go:242
        \_     #20 0xffff882dfad4 in grpc::ThreadManager::WorkerThread::Run() (/tmp/.mount_viam-ce4Br6K/lib/aarch64-linux-gnu/libgrpc++.so.1.51+0xafad4)
    logger.go:130: 2024-04-29T20:38:01.003Z     ERROR   process.viam_realsense_/home/rawr-ubuntu/Projects/viam-camera-realsense/packaging/appimages/deploy/viam-camera-realsense-latest-aarch64.AppImage.StdErr  pexec/managed_process.go:242
        \_     #21 0xffff878ea404  (/tmp/.mount_viam-ce4Br6K/lib/aarch64-linux-gnu/libgpr.so.29+0xa404)
    logger.go:130: 2024-04-29T20:38:01.003Z     ERROR   process.viam_realsense_/home/rawr-ubuntu/Projects/viam-camera-realsense/packaging/appimages/deploy/viam-camera-realsense-latest-aarch64.AppImage.StdErr  pexec/managed_process.go:242
        \_     #22 0xffff85a9ee54  (/tmp/.mount_viam-ce4Br6K/runtime/compat/lib/aarch64-linux-gnu/libc.so.6+0x7ee54)
    logger.go:130: 2024-04-29T20:38:01.003Z     ERROR   process.viam_realsense_/home/rawr-ubuntu/Projects/viam-camera-realsense/packaging/appimages/deploy/viam-camera-realsense-latest-aarch64.AppImage.StdErr  pexec/managed_process.go:242
        \_     #23 0xffff85b07f98  (/tmp/.mount_viam-ce4Br6K/runtime/compat/lib/aarch64-linux-gnu/libc.so.6+0xe7f98)
    logger.go:130: 2024-04-29T20:38:01.003Z     ERROR   process.viam_realsense_/home/rawr-ubuntu/Projects/viam-camera-realsense/packaging/appimages/deploy/viam-camera-realsense-latest-aarch64.AppImage.StdErr  pexec/managed_process.go:242
        \_ 
    logger.go:130: 2024-04-29T20:38:01.003Z     ERROR   process.viam_realsense_/home/rawr-ubuntu/Projects/viam-camera-realsense/packaging/appimages/deploy/viam-camera-realsense-latest-aarch64.AppImage.StdErr  pexec/managed_process.go:242
        \_ SUMMARY: AddressSanitizer: 14436 byte(s) leaked in 18 allocation(s).
    --- FAIL: TestCameraServer/shutdown_the_robot (1.34s)
        all_test.go:40: Expected: nil
            Actual:   'error closing module manager: error while stopping module viam_realsense: exit status 1'
FAIL

So no need to used the observed test logger

nicksanford · 2024-04-29T20:41:43Z

@hexbabe can you please confirm that tests FAIL if there is a memory leak introduced on all platforms we support? I don't mean in CI, I mean wherever we support running tests (like on your laptop).

@nicksanford

I am currently unable to get this ASAN/LSAN debug build to work on amd64. (For manager/PM mode Nick) Thoughts on how much time I should allocate to trying to get it to work?

If after the allocated time I am still unable, I can put on the README that the debug build is currently only verified to work on linux/arm64

If you are hitting friction, then don't worry about amd64 support for now, just document that the ASAN build doesn't work on amd64.

.gitignore

nicksanford · 2024-04-29T21:13:59Z

@hexbabe please remove:
viam-camera-realsense and packaging/appimages/viam-camera-realsense-latest-x86_64.AppImage.zsync

hexbabe · 2024-04-30T14:32:22Z

@hexbabe please remove: viam-camera-realsense and packaging/appimages/viam-camera-realsense-latest-x86_64.AppImage.zsync

Removed all artifacts

Make init changes to cmakelists

a5ddeba

hexbabe changed the title ~~[RSDK-5863] Add Address Sanitizer (ASan) to CI for intel realsense module~~ [RSDK-5863] Add Address Sanitizer (ASan) for intel realsense module Apr 26, 2024

Add rest of changes needed

7c9d46f

hexbabe marked this pull request as ready for review April 26, 2024 15:39

hexbabe changed the title ~~[RSDK-5863] Add Address Sanitizer (ASan) for intel realsense module~~ [RSDK-5863] Add Address Sanitizer (ASan) option for intel realsense module Apr 26, 2024

hexbabe commented Apr 26, 2024

View reviewed changes

integration/tests/all_test.go Show resolved Hide resolved

packaging/appimages/realsense-integration-tests-aarch64.yml Show resolved Hide resolved

Update README and var name

3820fb2

hexbabe requested a review from randhid April 26, 2024 16:45

randhid approved these changes Apr 26, 2024

View reviewed changes

randhid requested review from bhaney and nicksanford April 26, 2024 16:52

hexbabe commented Apr 26, 2024

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

nicksanford reviewed Apr 26, 2024

View reviewed changes

Add extra sanitizer deps in script conditionally

6cffa7e

Update cmakelists to include -fsanitize-address-use-after-scope and -…

bb73ae4

…fno-omit-frame-pointer

nicksanford reviewed Apr 29, 2024

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

hexbabe added 2 commits April 29, 2024 16:50

Update README with extra details and supported platforms note

c75bcf9

Remove extra dir in .gitignore

2da9ce8

hexbabe added 2 commits April 30, 2024 10:26

Remove artifacts from git

f9ce1c8

Remove changes to x86 appimage recipe since we're not supporting it

4408f2a

nicksanford approved these changes Apr 30, 2024

View reviewed changes

hexbabe merged commit 619d91c into main Apr 30, 2024
3 checks passed

hexbabe deleted the RSDK-5863 branch April 30, 2024 15:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RSDK-5863] Add Address Sanitizer (ASan) option for intel realsense module #38

[RSDK-5863] Add Address Sanitizer (ASan) option for intel realsense module #38

hexbabe commented Apr 26, 2024 •

edited

Loading

randhid left a comment

randhid commented Apr 26, 2024

nicksanford left a comment •

edited

Loading

nicksanford Apr 26, 2024

nicksanford Apr 26, 2024

hexbabe Apr 29, 2024

nicksanford Apr 26, 2024

hexbabe Apr 30, 2024

nicksanford commented Apr 26, 2024

nicksanford commented Apr 26, 2024

hexbabe commented Apr 26, 2024 •

edited

Loading

nicksanford commented Apr 26, 2024

nicksanford commented Apr 29, 2024

hexbabe commented Apr 29, 2024 •

edited

Loading

hexbabe commented Apr 29, 2024 •

edited

Loading

nicksanford commented Apr 29, 2024

nicksanford commented Apr 29, 2024

hexbabe commented Apr 30, 2024

[RSDK-5863] Add Address Sanitizer (ASan) option for intel realsense module #38

[RSDK-5863] Add Address Sanitizer (ASan) option for intel realsense module #38

Conversation

hexbabe commented Apr 26, 2024 • edited Loading

Testing

randhid left a comment

Choose a reason for hiding this comment

randhid commented Apr 26, 2024

nicksanford left a comment • edited Loading

Choose a reason for hiding this comment

nicksanford Apr 26, 2024

Choose a reason for hiding this comment

nicksanford Apr 26, 2024

Choose a reason for hiding this comment

hexbabe Apr 29, 2024

Choose a reason for hiding this comment

nicksanford Apr 26, 2024

Choose a reason for hiding this comment

hexbabe Apr 30, 2024

Choose a reason for hiding this comment

nicksanford commented Apr 26, 2024

nicksanford commented Apr 26, 2024

hexbabe commented Apr 26, 2024 • edited Loading

nicksanford commented Apr 26, 2024

nicksanford commented Apr 29, 2024

hexbabe commented Apr 29, 2024 • edited Loading

hexbabe commented Apr 29, 2024 • edited Loading

nicksanford commented Apr 29, 2024

nicksanford commented Apr 29, 2024

hexbabe commented Apr 30, 2024

hexbabe commented Apr 26, 2024 •

edited

Loading

nicksanford left a comment •

edited

Loading

hexbabe commented Apr 26, 2024 •

edited

Loading

hexbabe commented Apr 29, 2024 •

edited

Loading

hexbabe commented Apr 29, 2024 •

edited

Loading