CMake and Sanitizers

Hello!

I work on the Sanitizers, and in particular on HWASan on Android. I would like to make it easier to use, so that the developer only needs to specify the intent to use HWASan once (e.g. when using Gradle, in the Gradle build file, by passing -DCMAKE_BUILD_TYPE=DebugHwasan or similar to CMake). That would get translated by CMake to the relevant CFLAGS (-fsanitize=hwaddress -fno-omit-frame-pointer) for all binaries and libraries built for AArch64. It could potentially also put the appropriate wrap.sh in the library path (in the install step).

We could also generalise this to other Sanitizers (e.g. UBSan).

I am not particularly versed in CMake, so: does this make sense? Is this the right approach? Is this feasible?

1 Like

Whether you enable one or more sanitizers is at least partially independent of the build type. One might want to enable sanitizers with and without optimisations enabled (or indeed with different levels of optimisations), since some checks are only enabled or effective at certain optimisation levels. You may also want to enable more than one sanitizer simultaneously, such as AddressSanitizer and UndefinedBehaviorSanitizer. Then there’s also the complication of fuzzers, LLVM’s recent code coverage “sanitizer”, and so on. Trying to cover all the possible combinations with custom build types isn’t viable.

The approach I recommend is to put the logic in a toolchain file. If you are using a package manager like vcpkg or conan, this is basically a requirement, since you will likely need the sanitizer flags applied to the dependencies too (at least for some sanitizers). Since toolchain files are provided by the developer or the project, this is outside what CMake itself can provide.

I understand it may be helpful to have some sort of support that facilitates the use of santizers (basically make it easier for developers to write their own toolchain file). I think it would be better to maintain that outside of CMake though, so that it isn’t tied to the CMake release schedule. Also, implementing support for sanitizers outside of CMake would allow it to be used with older CMake releases. There are already some freely available Find modules out there which do this (of varying quality).

1 Like

Toolchain files probably shouldn’t be defining install rules, right? If the code is meant to be redistributed, for some platforms the sanitizer runtimes will need to be packaged along with the other binaries.

Correct, toolchain files wouldn’t normally add install logic. Maybe I’m missing something about the problem you’re trying to solve. I wouldn’t have expected sanitizers to cross into the installation space (it would normally be something purely driven by the consuming project).

The main thing we want to do is avoiding doing something in a way that CMake maintainers would not recommend. It’s happened before and digging Android out of that hole is something that’s taken several years and still isn’t finished.

If “CMake shouldn’t have any part in this” is the answer, that’s totally fine with us, we just wanted to be sure that wouldn’t be a mistake :slight_smile:

CC @brad.king, who’s been on the receiving end of our earlier mistakes and so probably has an opinion here.

I wouldn’t have expected sanitizers to cross into the installation space (it would normally be something purely driven by the consuming project).

Maybe they don’t. “Packaging” might have been a better word than “install”. For Android, the sanitizer runtimes may need to be packaged in the APK (it varies by sanitizer). One way we could handle that is for CMake to define install rules for the runtimes. Another is for the packager itself to infer that a sanitizer is being used and do that. The latter approach is how other parts of the build (like the C++ stdlib, as well as the build outputs themselves) are currently handled, and that doesn’t work very well IMO.

But you’re probably right and YAGNI.

Maybe I’m missing something about the problem you’re trying to solve.

This is probably more information than you want, but providing it just in case:

Android builds are typically a CMake build that’s invoked by a gradle plugin. The build.gradle file is where build variants need to be defined. Gradle needs to configure CMake appropriately, and either CMake needs to inform the gradle plugin which runtimes need to be installed (via https://cmake.org/cmake/help/latest/manual/cmake-file-api.7.html), or the plugin needs to be responsible for that itself.

My users find the behavior of CXXFLAGS and CMAKE_CXX_FLAGS surprising (the former cannot override the latter because they’re prepended rather than appended; the latter completely overrides the default flags for the platform; the behavior they want is to append to the defaults), so we prefer to implement this kind of thing indirectly, such as by having gradle pass -DANDROID_SANITIZE=address,undefined, which our toolchain file (which is that unrecommended approach we’ve been trying to fix I mentioned above) would then interpret and configure appropriately.

Like I said above, we’re okay with that approach, but wanted to be sure it was actually recommended before we ship it.

Having a toolchain file provide options for system-wide behavior that it translates to CMAKE_CXX_FLAGS is fine with me.

FWIW, that’s because it came from an existing $CXX $CXXFLAGS rest-of-compiler-command convention used in Makefiles. For reference, CMake Issue 23378 tracks Android’s use case.

Works for us, thanks for confirming!