fortify: account for new clang diagnostics
Clang recently grew its own diagnostics for memcpy and such. These are
generally higher-quality than what we can do with diagnose_if, since
clang is happy to include e.g., sizes of things per-callsite. Move to
those instead where applicable.
Bug: 131861088, 123644155
Test: blueline internal-master checkbuild; treehugger
Change-Id: I701f5a8b247ba2948ca47fdc60ff5198b564c03e
diff --git a/libc/include/bits/fortify/stdio.h b/libc/include/bits/fortify/stdio.h
index e766b20..8bb5b68 100644
--- a/libc/include/bits/fortify/stdio.h
+++ b/libc/include/bits/fortify/stdio.h
@@ -37,10 +37,9 @@
#if defined(__BIONIC_FORTIFY) && !defined(__BIONIC_NO_STDIO_FORTIFY)
#if __ANDROID_API__ >= __ANDROID_API_J_MR1__
+/* No diag -- clang diagnoses misuses of this on its own. */
__BIONIC_FORTIFY_INLINE __printflike(3, 0)
int vsnprintf(char* const __pass_object_size dest, size_t size, const char* format, va_list ap)
- __clang_error_if(__bos_unevaluated_lt(__bos(dest), size),
- "in call to 'vsnprintf', size is larger than the destination buffer")
__overloadable {
return __builtin___vsnprintf_chk(dest, size, 0, __bos(dest), format, ap);
}
@@ -52,10 +51,9 @@
#endif /* __ANDROID_API__ >= __ANDROID_API_J_MR1__ */
#if __ANDROID_API__ >= __ANDROID_API_J_MR1__
+/* No diag -- clang diagnoses misuses of this on its own. */
__BIONIC_FORTIFY_VARIADIC __printflike(3, 4)
int snprintf(char* const __pass_object_size dest, size_t size, const char* format, ...)
- __clang_error_if(__bos_unevaluated_lt(__bos(dest), size),
- "in call to 'snprintf', size is larger than the destination buffer")
__overloadable {
va_list va;
va_start(va, format);
diff --git a/libc/include/bits/fortify/string.h b/libc/include/bits/fortify/string.h
index 70e4476..9be2b1a 100644
--- a/libc/include/bits/fortify/string.h
+++ b/libc/include/bits/fortify/string.h
@@ -41,11 +41,10 @@
extern void* __memrchr_real(const void*, int, size_t) __RENAME(memrchr);
#if __ANDROID_API__ >= __ANDROID_API_J_MR1__
+/* No diag -- clang diagnoses misuses of this on its own. */
__BIONIC_FORTIFY_INLINE
void* memcpy(void* const dst __pass_object_size0, const void* src, size_t copy_amount)
- __overloadable
- __clang_error_if(__bos_unevaluated_lt(__bos0(dst), copy_amount),
- "'memcpy' called with size bigger than buffer") {
+ __overloadable {
size_t bos_dst = __bos0(dst);
if (__bos_trivially_not_lt(bos_dst, copy_amount)) {
return __builtin_memcpy(dst, src, copy_amount);
@@ -53,11 +52,9 @@
return __builtin___memcpy_chk(dst, src, copy_amount, bos_dst);
}
+/* No diag -- clang diagnoses misuses of this on its own. */
__BIONIC_FORTIFY_INLINE
-void* memmove(void* const dst __pass_object_size0, const void* src, size_t len)
- __overloadable
- __clang_error_if(__bos_unevaluated_lt(__bos0(dst), len),
- "'memmove' called with size bigger than buffer") {
+void* memmove(void* const dst __pass_object_size0, const void* src, size_t len) __overloadable {
size_t bos_dst = __bos0(dst);
if (__bos_trivially_not_lt(bos_dst, len)) {
return __builtin_memmove(dst, src, len);
@@ -117,19 +114,15 @@
return __builtin___strcat_chk(dst, src, __bos(dst));
}
+/* No diag -- clang diagnoses misuses of this on its own. */
__BIONIC_FORTIFY_INLINE
-char* strncat(char* const dst __pass_object_size, const char* src, size_t n)
- __overloadable
- __clang_error_if(__bos_unevaluated_lt(__bos(dst), n),
- "'strncat' called with size bigger than buffer") {
+char* strncat(char* const dst __pass_object_size, const char* src, size_t n) __overloadable {
return __builtin___strncat_chk(dst, src, n, __bos(dst));
}
+/* No diag -- clang diagnoses misuses of this on its own. */
__BIONIC_FORTIFY_INLINE
-void* memset(void* const s __pass_object_size0, int c, size_t n)
- __overloadable
- __clang_error_if(__bos_unevaluated_lt(__bos0(s), n),
- "'memset' called with size bigger than buffer")
+void* memset(void* const s __pass_object_size0, int c, size_t n) __overloadable
/* If you're a user who wants this warning to go away: use `(&memset)(foo, bar, baz)`. */
__clang_warning_if(c && !n, "'memset' will set 0 bytes; maybe the arguments got flipped?") {
size_t bos = __bos0(s);
@@ -165,11 +158,10 @@
#endif /* __ANDROID_API__ >= __ANDROID_API_M__ */
#if __ANDROID_API__ >= __ANDROID_API_L__
+/* No diag -- clang diagnoses misuses of this on its own. */
__BIONIC_FORTIFY_INLINE
char* stpncpy(char* const dst __pass_object_size, const char* const src __pass_object_size, size_t n)
- __overloadable
- __clang_error_if(__bos_unevaluated_lt(__bos(dst), n),
- "'stpncpy' called with size bigger than buffer") {
+ __overloadable {
size_t bos_dst = __bos(dst);
size_t bos_src = __bos(src);
@@ -181,11 +173,10 @@
return __stpncpy_chk2(dst, src, n, bos_dst, bos_src);
}
+/* No diag -- clang diagnoses misuses of this on its own. */
__BIONIC_FORTIFY_INLINE
char* strncpy(char* const dst __pass_object_size, const char* const src __pass_object_size, size_t n)
- __overloadable
- __clang_error_if(__bos_unevaluated_lt(__bos(dst), n),
- "'strncpy' called with size bigger than buffer") {
+ __overloadable {
size_t bos_dst = __bos(dst);
size_t bos_src = __bos(src);
diff --git a/tests/clang_fortify_tests.cpp b/tests/clang_fortify_tests.cpp
index 6923302..2838ab2 100644
--- a/tests/clang_fortify_tests.cpp
+++ b/tests/clang_fortify_tests.cpp
@@ -35,12 +35,14 @@
//
// Similarly, there are a few overload tricks we have to emit errors. Ignore any notes from those.
// expected-note@* 0+{{candidate function}}
-
-// FIXME(b/138701943): Silence warnings produced by -Wfortify-source since they're expected.
-// expected-warning@* 0+{{will always overflow}}
-// expected-warning@* 0+{{size argument is too large}}
+//
+// And finally, all explicitly-unavailable-here complaints from headers are
+// uninteresting
// expected-note@* 0+{{has been explicitly marked unavailable here}}
+// Note that some of these diags come from clang itself, while others come from
+// `diagnose_if`s sprinkled throughout Bionic.
+
#ifndef _FORTIFY_SOURCE
#error "_FORTIFY_SOURCE must be defined"
#endif
@@ -60,12 +62,14 @@
#define __clang_error_if(...)
#undef __clang_warning_if
#define __clang_warning_if(...)
+#pragma clang diagnostic ignored "-Wfortify-source"
// SOMETIMES_CONST allows clang to emit eager diagnostics when we're doing compilation tests, but
// blocks them otherwise. This is needed for diagnostics emitted with __enable_if.
#define SOMETIMES_CONST volatile
#else
#define SOMETIMES_CONST const
+#pragma clang diagnostic error "-Wfortify-source"
#endif
#include <err.h>
@@ -160,13 +164,13 @@
{
char large_buffer[sizeof(small_buffer) + 1] = {};
- // expected-error@+1{{size bigger than buffer}}
+ // expected-error@+1{{will always overflow}}
EXPECT_FORTIFY_DEATH(memcpy(small_buffer, large_buffer, sizeof(large_buffer)));
- // expected-error@+1{{size bigger than buffer}}
+ // expected-error@+1{{will always overflow}}
EXPECT_FORTIFY_DEATH(memmove(small_buffer, large_buffer, sizeof(large_buffer)));
// expected-error@+1{{size bigger than buffer}}
EXPECT_FORTIFY_DEATH(mempcpy(small_buffer, large_buffer, sizeof(large_buffer)));
- // expected-error@+1{{size bigger than buffer}}
+ // expected-error@+1{{will always overflow}}
EXPECT_FORTIFY_DEATH(memset(small_buffer, 0, sizeof(large_buffer)));
// expected-warning@+1{{arguments got flipped?}}
EXPECT_NO_DEATH(memset(small_buffer, sizeof(small_buffer), 0));
@@ -184,13 +188,13 @@
EXPECT_FORTIFY_DEATH(strcpy(small_buffer, large_string));
// expected-error@+1{{string bigger than buffer}}
EXPECT_FORTIFY_DEATH(stpcpy(small_buffer, large_string));
- // expected-error@+1{{size bigger than buffer}}
+ // expected-error@+1{{size argument is too large}}
EXPECT_FORTIFY_DEATH(strncpy(small_buffer, large_string, sizeof(large_string)));
- // expected-error@+1{{size bigger than buffer}}
+ // expected-error@+1{{size argument is too large}}
EXPECT_FORTIFY_DEATH(stpncpy(small_buffer, large_string, sizeof(large_string)));
// expected-error@+1{{string bigger than buffer}}
EXPECT_FORTIFY_DEATH(strcat(small_buffer, large_string));
- // expected-error@+1{{size bigger than buffer}}
+ // expected-error@+1{{size argument is too large}}
EXPECT_FORTIFY_DEATH(strncat(small_buffer, large_string, sizeof(large_string)));
// expected-error@+1{{size bigger than buffer}}
EXPECT_FORTIFY_DEATH(strlcpy(small_buffer, large_string, sizeof(large_string)));
@@ -227,12 +231,12 @@
EXPECT_FORTIFY_DEATH_STRUCT(stpcpy(split.tiny_buffer, small_string));
#if _FORTIFY_SOURCE > 1
- // expected-error@+2{{size bigger than buffer}}
+ // expected-error@+2{{size argument is too large}}
#endif
EXPECT_FORTIFY_DEATH_STRUCT(strncpy(split.tiny_buffer, small_string, sizeof(small_string)));
#if _FORTIFY_SOURCE > 1
- // expected-error@+2{{size bigger than buffer}}
+ // expected-error@+2{{size argument is too large}}
#endif
EXPECT_FORTIFY_DEATH_STRUCT(stpncpy(split.tiny_buffer, small_string, sizeof(small_string)));
@@ -242,7 +246,7 @@
EXPECT_FORTIFY_DEATH_STRUCT(strcat(split.tiny_buffer, small_string));
#if _FORTIFY_SOURCE > 1
- // expected-error@+2{{size bigger than buffer}}
+ // expected-error@+2{{size argument is too large}}
#endif
EXPECT_FORTIFY_DEATH_STRUCT(strncat(split.tiny_buffer, small_string, sizeof(small_string)));
@@ -491,11 +495,11 @@
FORTIFY_TEST(stdio) {
char small_buffer[8] = {};
{
- // expected-error@+1{{size is larger than the destination buffer}}
+ // expected-error@+1{{size argument is too large}}
EXPECT_FORTIFY_DEATH(snprintf(small_buffer, sizeof(small_buffer) + 1, ""));
va_list va;
- // expected-error@+2{{size is larger than the destination buffer}}
+ // expected-error@+2{{size argument is too large}}
// expected-warning@+1{{format string is empty}}
EXPECT_FORTIFY_DEATH(vsnprintf(small_buffer, sizeof(small_buffer) + 1, "", va));
