AcademySoftwareFoundation / OpenShadingLanguage

Advanced shading language for production GI renderers
BSD 3-Clause "New" or "Revised" License
2.05k stars 348 forks source link

Fix tests passing when test commands fail #1733

Closed brechtvl closed 9 months ago

brechtvl commented 9 months ago

Description

37cbab5b accidentally changed tests to accept failing commands by default.

This can cause tests to pass even though the program crashed, when a previous run created the correct output file.

This reveals existing asan and lsan errors. The asan error was fixed. The memory leaks remain ignored, as the exact cause is unclear.

Tests

N/A

Checklist:

brechtvl commented 9 months ago

I made it pass tests even when there are memory leaks. Better would be to fix these (if possible), but I don't really have the time for that. There is not a lot to go on.

=================================================================
==17951==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 140 byte(s) in 1 object(s) allocated from:
    #0 0x510ab7 in operator new(unsigned long) /llvm-project-llvmorg-13.0.1/compiler-rt/lib/asan/asan_new_delete.cpp:95:3
    #1 0x7f2828947cd8 in std::string::_Rep::_S_create(unsigned long, unsigned long, std::allocator<char> const&) (/lib64/libstdc++.so.6+0xbdcd8)

Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x510d67 in operator new(unsigned long, std::nothrow_t const&) /llvm-project-llvmorg-13.0.1/compiler-rt/lib/asan/asan_new_delete.cpp:101:3
    #1 0x7f28288e6f1d in __cxa_thread_atexit (/lib64/libstdc++.so.6+0x5cf1d)

SUMMARY: AddressSanitizer: 164 byte(s) leaked in 2 allocation(s).
lgritz commented 9 months ago

I don't mind sweeping a leak of 164 bytes under the rug for now, but does this change mask any other failures in the sanitizer run, which we would definitely want to catch?

brechtvl commented 9 months ago

It was masking all memory leaks. I made it more specific now, though it's still pretty broad.