Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve sanitizer support #1991

Merged
merged 2 commits into from
May 19, 2020
Merged

Conversation

sbeyer
Copy link
Member

@sbeyer sbeyer commented May 18, 2020

This PR adds support for more sanitizers than only AddressSanitizer
in the CMake configuration.

@sbeyer
Copy link
Member Author

sbeyer commented May 19, 2020

Wow, I did not know that CMake 3.5 does not have the list(JOIN ...) command, it was introduced in CMake 3.12. Will fix.

The SANITIZE_ADDRESS option of our CMake configuration activates the
AddressSanitizer (and UBSan in a non-working way) for the whole project
(although, by the way, its documentation pretends that it is only enabled
for tests).

This commit introduces new options SANITIZE_LEAK, SANITIZE_MEMORY,
SANITIZE_UNDEFINED, SANITIZE_THREAD.  Each of these options (including
SANITIZE_ADDRESS) enables only the corresponding sanitizer.

Moreover, we mark all sanitizer options as advanced options, because these
options are only interesting for developers.

Note that some sanitizers are conflicting, that is, not all options can
be enabled simultaneously.  Also, not all sanitizers are available for
all compilers and versions.  We, however, do not check for this, instead
we let the compiler throw its errors in such cases.

The explicit usage of the Google Linker is removed, because it is not
necessary and can lead to problems with clang.

The commit can be considered a rewrite of cmake/modules/SanitizerFlags.cmake.

Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
The UndefinedBehaviorSanitizer includes the "vptr" check.  This
check, however, needs typeinfo for OCC::AccountManager because
otherwise its stub for FileManTest leads to undefined references
when linking.  Adding the -frtti flag to enable run-time typeinfo
did not solve the problem.  I do not know another solution, so this
commit disables the vptr check.

Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
@sbeyer sbeyer force-pushed the improve-sanitizer-support branch from 7e40812 to 7f598b1 Compare May 19, 2020 08:58
Copy link
Member

@er-vin er-vin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@er-vin er-vin merged commit 35d1b8b into nextcloud:master May 19, 2020
@misch7
Copy link
Member

misch7 commented May 20, 2020

/backport to stable-2.6

@backportbot-nextcloud
Copy link

The backport to stable-2.6 failed. Please do this backport manually.

@misch7
Copy link
Member

misch7 commented May 20, 2020

🍒 Cherry-picked to stable-2.6

@misch7 misch7 added this to the 2.6.5 milestone May 20, 2020
@sbeyer sbeyer deleted the improve-sanitizer-support branch May 20, 2020 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants