From e5d66eb86a46b8ef1a8d03b322096328b063d3c0 Mon Sep 17 00:00:00 2001 From: George Burgess IV Date: Mon, 30 Oct 2017 21:41:22 -0700 Subject: [PATCH] libc: Disable FORTIFY if we're using clang-tidy. Having FORTIFY enabled for clang-tidy adds no value, and breaks some heuristics for recognizing standard library functions (see the bug). This also disables FORTIFY for the static analyzer (which we use through clang-tidy), because it presumably tries to recognize standard library functions through similar heuristics. Bug: 36664104 Test: mma with and without the patch to cdefs. New test breaks without. Change-Id: I40c66ff9e638b306878ada006bc2c98f2346e77a --- libc/include/sys/cdefs.h | 9 +++++++-- tests/Android.bp | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/libc/include/sys/cdefs.h b/libc/include/sys/cdefs.h index 36567333b..833ced07f 100644 --- a/libc/include/sys/cdefs.h +++ b/libc/include/sys/cdefs.h @@ -261,8 +261,13 @@ #if defined(_FORTIFY_SOURCE) && _FORTIFY_SOURCE > 0 # if defined(__clang__) -/* FORTIFY's _chk functions effectively disable ASAN's stdlib interceptors. */ -# if !__has_feature(address_sanitizer) +/* + * FORTIFY's _chk functions effectively disable ASAN's stdlib interceptors. + * Additionally, the static analyzer/clang-tidy try to pattern match some + * standard library functions, and FORTIFY sometimes interferes with this. So, + * we turn FORTIFY off in both cases. + */ +# if !__has_feature(address_sanitizer) && !defined(__clang_analyzer__) # define __BIONIC_FORTIFY 1 # endif # elif defined(__OPTIMIZE__) && __OPTIMIZE__ > 0 diff --git a/tests/Android.bp b/tests/Android.bp index 7094d778c..e3baf7496 100644 --- a/tests/Android.bp +++ b/tests/Android.bp @@ -224,6 +224,20 @@ cc_test_library { srcs: ["fortify_compilation_test.cpp"], } +// Ensure we don't use FORTIFY'ed functions with the static analyzer/clang-tidy: +// it can confuse these tools pretty easily. If this builds successfully, then +// __clang_analyzer__ overrode FORTIFY. Otherwise, FORTIFY was incorrectly +// enabled. The library that results from building this is meant to be unused. +cc_test_library { + name: "fortify_disabled_for_tidy", + cflags: [ + "-Werror", + "-D_FORTIFY_SOURCE=2", + "-D__clang_analyzer__", + ], + srcs: ["fortify_compilation_test.cpp"], +} + cc_test_library { name: "libfortify1-tests-clang", defaults: ["bionic_fortify_tests_defaults", "bionic_tests_defaults"],