Page MenuHomePhabricator

[CMAKE] Allow to use sanitizers with cmake
ClosedPublic

Authored by Fabien on May 14 2019, 08:21.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rABCf078eb620fec: [CMAKE] Allow to use sanitizers with cmake
Summary

To use any sanitizer(s), add it to the ENABLE_SANITIZERS list on
the CMake command line.
Examples:

cmake -GNinja .. -DENABLE_SANITIZERS=undefined
cmake -GNinja .. -DENABLE_SANITIZERS=address;leak

This requires an additional dependency to work:

ECM (Extra Cmake Modules, https://api.kde.org/ecm/index.html)

To install on Debian/Ubuntu:

sudo apt install extra-cmake-modules

This dependency is only required if the ENABLE_SANITIZERS variable
is set, it is not necessary to do a standard build.

The ECM package will check for compiler compatibility and set the
appropriated options for each sanitizer. It will eventually remove the
optimization options that may conflict with the selected sanitizers.

Test Plan

With ECM *not* installed:

mkdir buildcmake && cd buildcmake
cmake -GNinja ..
ninja

The build should be successful.

rm -rf *
cmake -GNinja .. -DENABLE_SANITIZERS=undefined

CMake should return en error:

ECM is required to enable the sanitizers

(https://api.kde.org/ecm/index.html)

Install ECM (instructions for Debian/Ubuntu):

sudo apt install extra-cmake-modules

Then

rm -rf *
cmake -GNinja .. -DENABLE_SANITIZERS=undefined
ninja -v

The build should enable the sanitizer (check the build commands for
-fsanitize=undefined)

rm -rf *
cmake -GNinja .. -DENABLE_SANITIZERS="address;leak"

Note: if you need to disable assembly instructions, add
-DUSE_ASM_X86_64=OFF to the CMake command line.

ninja -v

The build should enable the sanitizers (check the build commands for
-fsanitize=address and -fsanitize=leak)

Diff Detail

Repository
rABC Bitcoin ABC
Branch
cmake_ecm
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 5843
Build 9748: Bitcoin ABC Teamcity Staging
Build 9747: arc lint + arc unit

Event Timeline

Fabien created this revision.May 14 2019, 08:21
Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 14 2019, 08:21
deadalnix requested changes to this revision.May 14 2019, 13:22

The fact you should ECM to implement this is of little interest as fr as the API is concerned.

This revision now requires changes to proceed.May 14 2019, 13:22
Fabien edited the summary of this revision. (Show Details)May 14 2019, 14:03
Fabien edited the test plan for this revision. (Show Details)
Fabien planned changes to this revision.
Fabien updated this revision to Diff 8650.May 14 2019, 14:13

Hide ECM to the caller

deadalnix accepted this revision.May 14 2019, 22:41
This revision is now accepted and ready to land.May 14 2019, 22:41
This revision was automatically updated to reflect the committed changes.