Some possible leaks in the unit tests?

[noahp]

Interesting library! it seems very thorough, thanks for publishing it (documentation is very thorough, too!).

I noticed there are a few leaks detected by AddressSanitizer when executing the unit tests, see attached patch (sorry for the zip, it's what github permits :disappointed: ). I think it's just due to not freeing the objects generated in the tests (eg some missing calls to ojc_destroy()) but I wasn't sure. The command I used (documented in the patch) is:

CFLAGS="-fsanitize=leak -fsanitize=address -fsanitize=undefined" make test

0001-Support-injecting-CFLAGS-from-env.zip

[ohler55]

Thanks. I'll get that fixed and make sure the tests cleanup properly.

[ohler55]

New version of OjC. It's a complete re-write. Unit tests do not have any memory leaks from my testing. If you're like to try out the new version that would be great.

[noahp]

Nice, that takes care of the leaks! getting some runtime UB and stack overflow now:

CFLAGS="-fsanitize=leak -fsanitize=address -fsanitize=undefined" make -j 6 test

Output:

oj tests started
parse.c:1002:30: runtime error: signed integer overflow: 1234567890123456789 * 10 cannot be represented in type 'long int'
[300] expected null
oj.chunk.null Failed
error at 1:6
[300] unexpected character 'e' in 'v' mode
oj.chunk.false Failed
error at 1:1
[300] unexpected character 'u' in 'v' mode
oj.chunk.true Failed
error at 1:1
[300] unexpected character ',' in 'v' mode
oj.chunk.string Failed
error at 1:7
=================================================================
==50557==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffcbad03944 at pc 0x55de7674990b bp 0x7fa8f96fee30 sp 0x7fa8f96fee20
READ of size 4 at 0x7ffcbad03944 thread T1
    #0 0x55de7674990a in write_chunks (/home/noah/dev/github/ojc/test/oj_tests+0x9190a)
    #1 0x7fa8fd884608 in start_thread /build/glibc-YYA7BZ/glibc-2.31/nptl/pthread_create.c:477
    #2 0x7fa8fce3e102 in __clone (/lib/x86_64-linux-gnu/libc.so.6+0x122102)

Address 0x7ffcbad03944 is located in stack of thread T0 at offset 68 in frame
    #0 0x55de7674a3df in chunk_int_test (/home/noah/dev/github/ojc/test/oj_tests+0x923df)

  This frame has 2 object(s):
    [32, 56) 'data' (line 115) <== Memory access at offset 68 overflows this variable
    [96, 152) 'input' (line 114)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow (/home/noah/dev/github/ojc/test/oj_tests+0x9190a) in write_chunks

0001-patches-to-use-asan.patch.zip

[ohler55]

I know it sounds odd to say, great! But, great, thanks. The first should be an easy fix I think. The chunk tests might take some digging. What is the patch file?

[noahp]

Thanks! The patch is to enable passing the addresssanitizer CFLAGS into the Makefile.

[ohler55]

Updated. Clean over here. I didn't see the stack overflow though so I'd like to get to the bottom of that. I used Ubuntu 18. What are you testing on?

Anyway the develop branch has been updated.

[noahp]

I was testing with ubuntu 20.04 (gcc 9.3.0). Looks good now! I get a test failure but no AddressSanitizer errors anymore :smile:

./oj_tests
oj tests started
[300] expected null
oj.chunk.null Failed
error at 1:6
[300] unexpected character 'e' in 'v' mode
oj.chunk.false Failed
error at 1:1
[300] unexpected character 'u' in 'v' mode
make[1]: *** [Makefile:26: test] Broken pipe
make[1]: Leaving directory '/home/noah/dev/github/ojc/test'
make: *** [Makefile:10: test] Error 2

Closing the issue, thanks!

[ohler55]

I'd really like to know why that test is failing. Darn

[noahp]

Here's a shell script (requires docker) that reproduces the failure, chmod +x it and run it in the repo root. Verified the issue occurs on b279dc4a3d6586763bd4d576f27110d695daa096 .

test.sh.zip

[ohler55]

Thank you. I was not able to get the script to work. You did go above and beyond providing it, thanks you. I'm still trying to resolve the issues which start with not being able to resolve the ubuntu.com addresses. Odd because they do resolve outside the image.

[noahp]

Ah, you may need to add this to /etc/docker/daemon.json:

{
    "dns": ["8.8.8.8"]
}

Then sudo service docker restart.

[ohler55]

I managed to get it running on macOS. Other issues with missing rsync so I'm updating the makefile.

[ohler55]

Update: I have been able to reproduce the error or something like it. You have been super helpful. Now on to fixing it.

[ohler55]

Thanks found the issue and am double checking the fix now.

[ohler55]

Released (by tagging) the changes including your Makefile changes.