CI: add workflow with sanitizers check

[alexander-e1off]

When PR is approved, run sanitizers check.

[678098]

Some comments

[678098]

Suggested change

if: github.event.review.state == 'APPROVED'

These checks are not necessary for run_*san, since they already depend on build_dependencies which checks for this condition

[678098]

There are 4 *.cmake files which are almost identical.

diff asan.txt msan.txt

1c1
< # Compiler-less toolchain for building with {Clang, libc++} + AddressSanitizer.
---
> # Compiler-less toolchain for building with {Clang, libc++} + MemorySanitizer.
27a28,29
> set(MSAN_SUPPRESSION_LIST_PATH "$ENV{DIR_SRC_BMQ}/etc/msansup.txt")
> 
35d36
<        "-fno-optimize-sibling-calls "
36a38,39
>        "-fsanitize=memory "
>        "-fsanitize-blacklist=${MSAN_SUPPRESSION_LIST_PATH} "
97,99c100,107
< set( TOOLCHAIN_DEBUG_FLAGS
<      "-fsanitize=address "
<      )
---
> # Conditionally add flags helpful for debugging MemorySanitizer issues.
> if (DEBUG_MEMORY_SANITIZER)
>     string(CONCAT TOOLCHAIN_DEBUG_FLAGS
>            "${TOOLCHAIN_SHARED_FLAGS} "
>            "-fsanitize-memory-track-origins=2 "
>            "-fno-optimize-sibling-calls "
>           )
> endif()

diff asan.txt tsan.txt

1c1
< # Compiler-less toolchain for building with {Clang, libc++} + AddressSanitizer.
---
> # Compiler-less toolchain for building with {Clang, libc++} + ThreadSanitizer.
96a97,99
> set(TSAN_SUPPRESSION_LIST_PATH "$ENV{DIR_SRC_BMQ}/etc/tsansup.txt")
> set(THREAD_SANITIZER TRUE)
> 
98c101
<      "-fsanitize=address "
---
>      "-fsanitize=thread "
105a109
>

diff asan.txt ubsan.txt

96a97
> # Add MemorySanitizer-related flags.
98,99c99,100
<      "-fsanitize=address "
<      )
---
>   "-fsanitize=undefined "
>   )

Does it make sense to combine sanitizers configuration in one cmake file?

[alexander-e1off]

My initial thought was to keep separate files for better readability, but since the differences are small it's worth to put sanitizers specific config in one file.

[678098]

Suggested change

	# Compiler-less toolchain for building with {Clang, libc++} + AddressSanitizer.
	# Compiler-less toolchain for building with {Clang, libc++} + UndefinedBehavoiurSanitizer.

[678098]

Have a few more questions, if some of these are too complex or obscure we might skip them

[678098]

This setting doesn't have any effect now and should be removed

[alexander-e1off]

Removed.

[678098]

Do you know why we don't enable -fno-optimize-sibling-calls for a general case launch of memory sanitizer here? Is the check too slow without it? This option is included for other sanitizers unconditionally.

At least docs advice to enable it for better understanding results:

https://releases.llvm.org/8.0.0/tools/clang/docs/MemorySanitizer.html#usage

[alexander-e1off]

MemorySanitizer is the slowest one, but I checked that this option does not affect at the speed. So, made this option shared for all sanitizers.

[678098]

We set these suppressions in two different places, here and in the .cmake file, e.g.:

set(TSAN_SUPPRESSION_LIST_PATH "$ENV{DIR_SRC_BMQ}/etc/tsansup.txt")

Is it possible to move all these suppressions to this common json config?

[alexander-e1off]

Removed it from cmake file. Now all suppressions that are passed at runtime are located in json config.

[678098]

If we build everything in Debug, why do we specify this setting for "release with debug info"?

[alexander-e1off]

Removed.

[678098]

Same question

[678098]

@alexander-e1off the PR looks good, let's test it!

[678098]

Oh, this setting is still in the .cmake file

[alexander-e1off]

Removed it

[alexander-e1off]

Oh, sorry, it is needed, because this file is passed at compile time.

[678098]

Is the state case sensitive?

[alexander-e1off]

In github actions doc it is in uppercase , but I saw also in lowercase, e.g. here