Have a mixed felling of enabling assert() for release build configuration, but if we document specific behavior in the Wiki (where we would advertise this config for developers i am imagining) think it's fine.

If we don't care much about benchmarkability, what about enabling WITH_CYCLES_DEBUG ?
Are there any downsides enabling WITH_MEM_VALGRIND ?

Afaik, valgrind is not compatible with asan? So indeed we cannot include it here (or we'd need two different 'developer' settings :p ).

Otherwise, seems fine, though I have mixed feelings about the WITH_ASSERT_ABORT one, we still have quiet a few usages of asserts as 'soft errors', as in 'things that should not happen, but might still not be critical issues', "crashing" on those can be fairly annoying. That’s probably a bit of an orthogonal topic (I know e.g. I tend to use assert when I should rather use error logging), but for the time being, not sure that we should enable that one by default?

[short version]
You can safely ignore my wish for release mode assertions in regards to this diff.
[/Short version]

[Long Version]
I can give some background on the release mode asserts, this is not something meant for developers, when running the nightly tests there have been a few instances where the result is either unexpected (the long failing win32 cycles sss tests for instance) or an occasional random crash. but when you take a closer look and load up the problem in the debugger you almost instantly hit an assert checking for the issue you're experiencing. There's 4000+ asserts in our code, it be real nice if they alerted us to issues proactively rather than a debugging aid once someone reported an issue we already knew about but silently let slip by.

Ok, great just run the tests on debug builds problem solved!

Sadly it is not..

1: we have 10 Gb of space on the nightly build systems, I cannot do a debug build I just run out of space

All right fine, then just don't automate the testing and run a debug test locally, fair enough..

2: Debug builds are *slow* about 2 hours(or more) for a full ctest, and during those 2 hours the machine is having high resource usage, even using a browser while you wait is not great.

Start the tests, go for coffee, or watch a movie, run them at night or something!

3: By default, an assert pops up a dialog, you have to click away before things will continue. Even worse if you do get the tests on the azure environment, there is no screen or keyboard, so once the popup you cannot see pops up, it'll just sit there until the build time expires and your session gets killed.

So not only can I not use automated testing, I have to run them locally, It'll take 2 hours, My machine is unusable while they run, and I have to babysit it in case of random popups.

Soooo....yeahhh..... that's not great, there is a 0% chance anyone would do this voluntarily on a regular basis

I have a patch almost ready that'll address these issues, however, since the problems it will solve are so specific, I do not think any of it should be enabled for all developers hence we can safely ignore the whole concept of release mode asserts for this diff.
[/Long Version]

We indeed don't need to worry about release mode asserts for committing this diff. The reason I mentioned it here was mostly as a to do item. Since this includes asan, it should logically also include asserts.

WITH_ASSERT_ABORT is already ON by default, and asserts should be reserved for issues that are actual bugs. The main reason it's in this config is so that running this on an existing cmake cache restores all the options to what they should be for developers.

It also seems that WITH_PYTHON_SAFETY is breaking the freestyle render tests, and that WITH_COMPILER_ASAN finds some issues in Cycles tests. I'll look into fixing those.

We have all these useful tests and tools to catch issues early, but then end up disabling them for various reasons. That's what I'm trying to solve here.

In D5149#116953, @Brecht Van Lommel (brecht) wrote:

It also seems that WITH_PYTHON_SAFETY is breaking the freestyle render tests, and that WITH_COMPILER_ASAN finds some issues in Cycles tests. I'll look into fixing those.

We have all these useful tests and tools to catch issues early, but then end up disabling them for various reasons. That's what I'm trying to solve here.

ASAN leak check on exit reports leaks which are out of our control, eg:

Direct leak of 1685265 byte(s) in 101 object(s) allocated from:
    #0 0x7f1ce79a0ada in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x7f1cdefd29db  (/usr/lib/libnvidia-glcore.so.430.26+0xe029db)

Direct leak of 1413382 byte(s) in 743 object(s) allocated from:
    #0 0x7f1ce79a0ada in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x7f1ce7067508 in PyObject_Malloc (/usr/lib/libpython3.7m.so.1.0+0xe7508)

This causes tests & generating docs to fail because of the nonzero exit code.

We could disable leak-check-at-exit cases when Blender's exit-code is used.

eg: ASAN_OPTIONS=leak_check_at_exit=0:$ASAN_OPTIONS (the trailing : is ignored in the case ASAN_OPTIONS isn't defined).

The Freestyle and WITH_PYTHON_SAFETY thing is actually separate, it also happens without asan. Freestyle lines are not rendered for some reason.

Agree it should not fail the tests on memory leaks now. Maybe later if we have suppressions or can issue warnings instead of failures, not sure. I'd use LSAN_OPTIONS="exitcode=0" so it still prints leaks but does not call exit().

In D5149#117067, @Brecht Van Lommel (brecht) wrote:

I'd use LSAN_OPTIONS="exitcode=0" so it still prints leaks but does not call exit().

LGTM, as long as it's not making failure cases succeed,

env LSAN_OPTIONS="exitcode=0" blender -b --python-expr "import sys; sys.exit(5)"
echo $?

Prints 5.

One annoying thing with asan builds is that Blender prints memory leaks in the console whenever you run it. I have my environment configured to suppress them.

export LSAN_OPTIONS="print_suppressions=false:suppressions=/home/brecht/tools/dev-tools/asan/lsan-suppressions.txt

lsan-suppressions.txt:

leak:_PyObject_Malloc
leak:_PyObject_Realloc
leak:PyThread_allocate_lock
leak:imb_exitopenexr
leak:imb_filetypes_exit
leak:<unknown module>
leak:libnvidia-glcore.so
leak:libX11.so
leak:libglib-2.0.so

But every developer would need to set that up too if they want those out of the way. I'm not sure if there is some way to suppress them from within Blender, or if that's something we even want to do.

Another issue is that WITH_DOC_MANPAGE fails, because it runs blender --help which returns a non-zero exit code due to leaks in OpenEXR. That one could be disabled for the developer config I think.

This is ready now as far as I'm concerned, along with D5404: Build: disable address sanitizer for Cycles kernels with GCC.

After the most recent fixes in master I can reliably build and pass all tests with RelWithDebInfo and Debug configurations.

Two main things to improve after this would be:

• Improve build and test performance (big gtest binaries, use subset of cycles tests for most blender devs, ..?)
• Enable asserts in RelWithDebInfo configuration