pflarue / ardop

Source for various version of ARDOP
Other
23 stars 4 forks source link

suggestion: bug hunting with ASAN and UBSAN #37

Open cbs228 opened 1 month ago

cbs228 commented 1 month ago

If you build PR #36 with

make clean
make \
  CFLAGS='-g -O1 -fPIC -MMD -fsanitize=address,undefined -fsanitize-recover=address' \
  LDFLAGS='-fsanitize=address,undefined -fsanitize-recover=address'

you can use the Address Sanitizer and the Undefined Behavior Sanitizer.

The sanitizers help detect invalid memory usage that can lead to otherwise hard-to-diagnose crashes. There is a 2× runtime speed penalty for using them. I do not recommend distributing binaries with the sanitizers enabled.

The sanitizers work best with some optimization (-O1).

Here is one bug I found which occurred at the end of an ARQ session. I have no idea what triggered it.

ASAN error, click triangle to show ```txt SoundInput.c:683:11: runtime error: index 3 out of bounds for type 'float [3]' SoundInput.c:683:15: runtime error: store to address 0x5b8bd17a66ac with insufficient space for an object of type 'float' 0x5b8bd17a66ac: note: pointer points here 73 21 b0 3f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ^ ================================================================= ==407102==ERROR: AddressSanitizer: global-buffer-overflow on address 0x5b8bd17a66ac at pc 0x5b8bd15e4365 bp 0x7fff21f77090 sp 0x7fff21f77080 WRITE of size 4 at 0x5b8bd17a66ac thread T0 #0 0x5b8bd15e4364 in Filter75Hz /build/ardop/ARDOPC/SoundInput.c:683 #1 0x5b8bd15e96c4 in EnvelopeCorrelator /build/ardop/ARDOPC/SoundInput.c:2035 #2 0x5b8bd15e9d30 in Acquire2ToneLeaderSymbolFraming /build/ardop/ARDOPC/SoundInput.c:1982 #3 0x5b8bd1606b99 in ProcessNewSamples /build/ardop/ARDOPC/SoundInput.c:981 #4 0x5b8bd15a0fc6 in PollReceivedSamples ../ARDOPCommonCode/ALSASound.c:1878 #5 0x5b8bd15b0bb3 in ardopmain /build/ardop/ARDOPC/ARDOPC.c:826 #6 0x5b8bd15a2fae in main ../ARDOPCommonCode/ALSASound.c:760 #7 0x78712182814f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 #8 0x787121828208 in __libc_start_main_impl ../csu/libc-start.c:360 #9 0x5b8bd15799a4 in _start (/build/ardop/ARDOPC/ardopcf+0x1109a4) (BuildId: b57e3c03581dde8bf4e292bd90dc0b086b6e9900) 0x5b8bd17a66ac is located 52 bytes before global variable 'dblCoef' defined in 'SoundInput.c:585:15' (0x5b8bd17a66e0) of size 68 0x5b8bd17a66ac is located 0 bytes after global variable 'dblCoef' defined in 'SoundInput.c:660:15' (0x5b8bd17a66a0) of size 12 SUMMARY: AddressSanitizer: global-buffer-overflow /build/ardop/ARDOPC/SoundInput.c:683 in Filter75Hz Shadow bytes around the buggy address: 0x5b8bd17a6400: 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 04 f9 f9 f9 0x5b8bd17a6480: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 0x5b8bd17a6500: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 0x5b8bd17a6580: f9 f9 f9 f9 03 f9 f9 f9 f9 f9 f9 f9 03 f9 f9 f9 0x5b8bd17a6600: f9 f9 f9 f9 03 f9 f9 f9 f9 f9 f9 f9 03 f9 f9 f9 =>0x5b8bd17a6680: f9 f9 f9 f9 00[04]f9 f9 f9 f9 f9 f9 00 00 00 00 0x5b8bd17a6700: 00 00 00 00 04 f9 f9 f9 f9 f9 f9 f9 00 00 f9 f9 0x5b8bd17a6780: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 0x5b8bd17a6800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x5b8bd17a6880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x5b8bd17a6900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==407102==ABORTING ```

This ASAN error occurs on:

Making ASAN (and UBSAN) a regular part of the testing regimen will help us detect unsafe code faster. It's a lot easier than guessing!

Runtime Libraries

The sanitizers require the libasan.so and libubsan.so libraries for your gcc. These are frequently automatically installed alongside gcc. If your distro does not provide them by default, you can see which libraries your gcc needs with:

gcc -print-file-name=libasan.so

Then find the correct package for your operating system that provides this file.

Continue instead of abort

If you run with

ASAN_OPTIONS=halt_on_error=0 ./ardopcf

then ASAN will attempt to continue rather than abort when it detects an error. Be warned, the program may crash anyway. Follow-up ASAN reports from the same run might be wrong.

pflarue commented 1 month ago

I have no experience with ASAN or UBSAN. I will have to do some study to better understand these tools. Unfortunately, I am not surprised that invalid memory usage such as the example you identified may occur. If these tools can help us to identify errors causing such behavior, they will prove very useful.

If you identify any additional errors with these tools, and especially if you are able to describe how to reproduce them, please create an Issue for each that you believe represents a unique bug.

As with all bugs whose cause is not yet clear, please identify the operating system and the version of ardopcf being used. If using anything other than a released version of ardopcf, please describe your build in terms of the last commit used. I understand that in some cases, this may reference a commit only found in another fork.

cbs228 commented 1 month ago

As with all bugs whose cause is not yet clear, please identify the operating system and the version of ardopcf being used

Added to issue description.

please create an Issue for each that you believe represents a unique bug

Can do. I think we can track this specific ASAN finding with this issue. We can close this issue when it's found and fixed.

pflarue commented 1 month ago

Thanks for adding those details.

I agree that we'll use this Issue to track this specific bug.

Dinsmoor commented 1 month ago

This is really cool. I believe that this testing method will be most useful once the entire codebase is done being pruned and manually reviewed.

pflarue commented 15 hours ago

@cbs228, I tried making ardopcf from the current develop branch (commit 820d29a) using the commands you suggested at the top of this Issue. It seemed to compile OK, but when I try to run ardopcf -h I get: ==1188==ASan runtime does not come first in initial library list; you should either link runtime to your application or manually preload it with LD_PRELOAD.

Since you are more familiar with these tools, can you suggest what I am doing wrong. Perhaps this is related to changes to the Makefile since you created this issue?

pflarue commented 15 hours ago

Pull request #67 (merging branch bugfix-issue37 into develop branch) corrects the bug reported above.

However, this Issue will be kept open until I figure out how to do further testing with ASAN and UBSAN.

cbs228 commented 13 hours ago

ASan runtime does not come first in initial library list

When you build with ASAN, everything that uses the object file or library needs to link libasan. But the linking requires special handling; it's not just -lasan. You need to make sure you're providing -fsanitize=address,undefined as both a compiler flag and a linker flag. This means as a CPPFLAG and as an LDFLAG. If you do this, ldd ardopcf should show libasan as the first dependency.

It's possible that some Makefile changes broke how these arguments work. I'll look at it later and see.

pflarue commented 13 hours ago

@cbs228, Thanks. I appreciate your help and I think that using these tools will be very useful as we try to find and eliminate bugs in ardopcf.

cbs228 commented 10 hours ago

On Linux, I have been using:

make \
  CFLAGS='-g -MMD -O1 -fno-omit-frame-pointer -fsanitize=address,undefined' \
  LDFLAGS='-Xlinker -Map=output.map -fsanitize=address,undefined' \
  all

which results in

$ ldd ardopcf
    linux-vdso.so.1 (0x00007ffee05f3000)
    libasan.so.8 => /lib/x86_64-linux-gnu/libasan.so.8 (0x000070e697a00000)
    libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x000070e69857f000)
    ⋮

(I was a little wrong, the kernel service library is actually first. But ASAN is loaded before libc and anything else.)

If that works for you, you can close this issue.