Move clang-tidy, ASAN, UBSAN to clang-9

[RudolfWeeber]

Results so far:
Build with Clang9 on the ICP machines:

• Clang++ cannot be used for the Cuda code, unless we are prepared to patch thrust. The code makes use of va_printf which is not defined (Compilation with Clang fails, va_printf undeclared NVIDIA/thrust#1032). Alternatively, we could include the workaround discussed in the isseu in Espresso. Personally, I'd just use Nvcc.

• The following clang-tidy checks get triggered due to a more modern clang-tidy:

  • bugprone-exception-escape,
  • bugprone-macro-parentheses,
  • bugprone-narrowing-conversions,
  • bugprone-unhandled-self-assignment,
  • modernize-avoid-c-arrays,

  As per the discussion on the Espresso meeting, I'll disable them. If we want, we can address (part of) them separately.

• The ASAN needs ASAN_OPTIONS="allocator_may_return_null=1" to avoid an error from an OpenMPI library

[fweik]

bugprone-unhandled-self-assignment that sounds scary

[RudolfWeeber]

@fweik, I think it was in the (copy or move) constructor of Utils::List

Some of the narrowing warnings should probably also be looked at.

[mkuron]

Personally, I'd just use Nvcc.

That would get rid of clang-tidy checking on CUDA code. NVCC basically has no warnings, so if we can patch thrust we should do that to keep clang.

[fweik]

The bugprone-narrowing-conversions in src/shapes/src/SpheroCylinder.cpp:52:8 is almost certainly an actual bug. Seems like a useful warning to me.

[fweik]

The self-assignment is fixed in #3628, please do not remove the check.

[fweik]

The narrowing thing is fixed in #3629.

[fweik]

modernize-avoid-c-arrays has to be disabled I think: Utils::Array uses c-array as portable storage backend that also works on the GPU, which makes changing it more complicated. It's probably not
worthwhile to implemented a more sophisticated solution for this, because the GPU support for
Array can be removed with the soonish (?) removal of the GPU LB.

[RudolfWeeber]

@fweik, I'll re-enable the checks corresponding to your fixes.
#mkuron,
Getting rid of patched boost/thrust/other-stuff and getting the CI containers simple is my main goal with this project.
Also, peoples' dev environments should be (apt-install)-simple. People should be able to just use clang-tidy and the sanitizers on the ICP machines.

Also, a lot of the GPU stuff will go away, once we are ready with Walberla.

I'll check, if the workaround suggested in the issue mentioned can easily be integrated. If not...

[fweik]

I'll also fix bugprone-exception-escape

[mkuron]

peoples' dev environments should be (apt-install)-simple. People should be able to just use clang-tidy and the sanitizers on the ICP machines.

Sure, but sometimes that is not possible. Clang is a useful tool and helps find actual bugs before they become big problems. Complex tools like that require some maintenance effort; and as long as that only means patching a file, it‘s not worth forgoing correctness in favor of simplicity.

[jngrad]

The va_printf compiler error is fixed in thrust 1.9.5, which should be shipped with CUDA 10.1. I've installed thrust 1.9.5 in a local copy of CUDA 10.0 and the compiler error disappeared, and all GPU tests passed, using commit 58bdbc8 from #3582 and the following CMake command:

cmake .. -DWITH_CUDA=ON -DWITH_CUDA_COMPILER=clang \
  -DCMAKE_BUILD_TYPE=RelWithAssert -DWITH_COVERAGE=OFF \
  -DCMAKE_C_COMPILER=$(which clang-9) \
  -DCMAKE_CXX_COMPILER=$(which clang++-9) \
  -DCMAKE_CXX_FLAGS=--cuda-path=/work/jgrad/cuda-10.0-thrust195 \
  -DCUDA_NVCC_FLAGS=--cuda-path=/work/jgrad/cuda-10.0-thrust195

[RudolfWeeber]

I tried building with cuda 10.1 at ICP installed in /usr/local. The bug was in there, as well. I didn’t investigate further. Let’s see if the workaround has side effects or not.

[jngrad]

That's odd, because /usr/local/cuda-10.0 is the only CUDA 10 version I see on the ICP machines. I just logged in bee and it only has /usr/local/cuda-10.2 (with thrust 1.9.7). I couldn't try compiling on bee because CMake is too old and pip is not installed.

[fweik]

@jngrad you can load a more recent cmake as a module on bee.

[jngrad]

I'm not very used to bee :) I just did module load cmake clang to get the latest CMake, but clang is not found.

[RudolfWeeber]

@jngrad, you're right. Looks like I misread sth

[jngrad]

@RudolfWeeber I ran into issues with Clang++-9 in the Ubuntu 20.04 image with CUDA 10.1 installed. You might want to have a look at the compiler error, it's in #3650.

[jngrad]

I've addressed the majority of the Clang-Tidy 9 warnings. modernize-avoid-c-arrays was disabled since it's triggered in too many places. bugprone-macro-parentheses was temporarily disabled, until we refactor the macros in the rhomboid shape. The two warnings triggered by boost::mpi in Boost <1.96 were configured such that they don't become compiler errors; we can't exclude those warnings with a line filter: #3650 (comment). // NOLINT was added in a few places where the warning was either a false positive, or a true positive for which I couldn't find a quick fix. The changes to the CUDA code are substantial, since Clang-Tidy 6 ignored them in CI for a while.