libc fortify: make string.h use diagnose_if

This also has a handful of style fixups, to make this file more
consistent. And removes __bionic_zero_size_is_okay_t, since there's a
better workaround available.

Bug: 12231437
Test: m checkbuild on bionic internal master; CtsBionicTestCases show
no new failures.

Change-Id: I75a020630dbab0ce828563502900cba14ae992d1

diff --git a/libc/bionic/fortify.cpp b/libc/bionic/fortify.cpp
index eaf5387..cbcc976 100644 (file)
--- a/libc/bionic/fortify.cpp
+++ b/libc/bionic/fortify.cpp
@@ -78,8 +78,6 @@
 //   http://gcc.gnu.org/ml/gcc-patches/2004-09/msg02055.html
 //
 
-struct __bionic_zero_size_is_okay_t __bionic_zero_size_is_okay;
-
 int __FD_ISSET_chk(int fd, fd_set* set, size_t set_size) {
   __check_fd_set("FD_ISSET", fd, set_size);
   return FD_ISSET(fd, set);

diff --git a/libc/include/bits/fortify/string.h b/libc/include/bits/fortify/string.h
index 0c1999b..667e21d 100644 (file)
--- a/libc/include/bits/fortify/string.h
+++ b/libc/include/bits/fortify/string.h
@@ -37,37 +37,45 @@ char* __strncpy_chk2(char*, const char*, size_t, size_t, size_t) __INTRODUCED_IN
 size_t __strlcpy_chk(char*, const char*, size_t, size_t) __INTRODUCED_IN(17);
 size_t __strlcat_chk(char*, const char*, size_t, size_t) __INTRODUCED_IN(17);
 
-/* Only used with FORTIFY, but some headers that need it undef FORTIFY, so we
- * have the definition out here.
- */
-struct __bionic_zero_size_is_okay_t {};
-
 #if defined(__BIONIC_FORTIFY)
 // These can share their implementation between gcc and clang with minimal
 // trickery...
 #if __ANDROID_API__ >= __ANDROID_API_J_MR1__
 __BIONIC_FORTIFY_INLINE
-void* memcpy(void* const dst __pass_object_size0, const void*  src, size_t copy_amount)
-        __overloadable {
+void* memcpy(void* const dst __pass_object_size0, const void* src, size_t copy_amount)
+        __overloadable
+        __clang_error_if(__bos0(dst) != __BIONIC_FORTIFY_UNKNOWN_SIZE && __bos0(dst) < copy_amount,
+                         "'memcpy' called with size bigger than buffer") {
     return __builtin___memcpy_chk(dst, src, copy_amount, __bos0(dst));
 }
 
 __BIONIC_FORTIFY_INLINE
-void* memmove(void* const dst __pass_object_size0, const void* src, size_t len) __overloadable {
+void* memmove(void* const dst __pass_object_size0, const void* src, size_t len)
+        __overloadable
+        __clang_error_if(__bos0(dst) != __BIONIC_FORTIFY_UNKNOWN_SIZE && __bos0(dst) < len,
+                         "'memmove' called with size bigger than buffer") {
     return __builtin___memmove_chk(dst, src, len, __bos0(dst));
 }
 #endif /* __ANDROID_API__ >= __ANDROID_API_J_MR1__ */
 
 #if __ANDROID_API__ >= __ANDROID_API_L__
 __BIONIC_FORTIFY_INLINE
-char* stpcpy(char* const dst __pass_object_size, const char* src) __overloadable {
+char* stpcpy(char* const dst __pass_object_size, const char* src)
+        __overloadable
+        __clang_error_if(__bos(dst) != __BIONIC_FORTIFY_UNKNOWN_SIZE &&
+                             __bos(dst) <= __builtin_strlen(src),
+                         "'stpcpy' called with string bigger than buffer") {
     return __builtin___stpcpy_chk(dst, src, __bos(dst));
 }
 #endif /* __ANDROID_API__ >= __ANDROID_API_L__ */
 
 #if __ANDROID_API__ >= __ANDROID_API_J_MR1__
 __BIONIC_FORTIFY_INLINE
-char* strcpy(char* const dst __pass_object_size, const char* src) __overloadable {
+char* strcpy(char* const dst __pass_object_size, const char* src)
+        __overloadable
+        __clang_error_if(__bos(dst) != __BIONIC_FORTIFY_UNKNOWN_SIZE &&
+                             __bos(dst) <= __builtin_strlen(src),
+                         "'strcpy' called with string bigger than buffer") {
     return __builtin___strcpy_chk(dst, src, __bos(dst));
 }
 
@@ -82,7 +90,12 @@ char* strncat(char* const dst __pass_object_size, const char* src, size_t n) __o
 }
 
 __BIONIC_FORTIFY_INLINE
-void* memset(void* const s __pass_object_size0, int c, size_t n) __overloadable {
+void* memset(void* const s __pass_object_size0, int c, size_t n)
+        __overloadable
+        __clang_error_if(__bos0(s) != __BIONIC_FORTIFY_UNKNOWN_SIZE && __bos0(s) < n,
+                         "'memset' called with size bigger than buffer")
+        /* 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?") {
     return __builtin___memset_chk(s, c, n, __bos0(s));
 }
 #endif /* __ANDROID_API__ >= __ANDROID_API_J_MR1__ */
@@ -90,35 +103,9 @@ void* memset(void* const s __pass_object_size0, int c, size_t n) __overloadable
 
 #if defined(__clang__)
 
-#define __error_if_overflows_dst(name, dst, n, what) \
-    __enable_if(__bos0(dst) != __BIONIC_FORTIFY_UNKNOWN_SIZE && \
-                __bos0(dst) < (n), "selected when the buffer is too small") \
-    __errorattr(#name " called with " what " bigger than buffer")
-
-__BIONIC_ERROR_FUNCTION_VISIBILITY
-void* memcpy(void* dst, const void* src, size_t copy_amount) __overloadable
-        __error_if_overflows_dst(memcpy, dst, copy_amount, "size");
-
-__BIONIC_ERROR_FUNCTION_VISIBILITY
-void* memmove(void *dst, const void* src, size_t len) __overloadable
-        __error_if_overflows_dst(memmove, dst, len, "size");
-
-__BIONIC_ERROR_FUNCTION_VISIBILITY
-void* memset(void* s, int c, size_t n) __overloadable
-        __error_if_overflows_dst(memset, s, n, "size");
-
-__BIONIC_ERROR_FUNCTION_VISIBILITY
-char* stpcpy(char* dst, const char* src) __overloadable
-        __error_if_overflows_dst(stpcpy, dst, __builtin_strlen(src), "string");
-
-__BIONIC_ERROR_FUNCTION_VISIBILITY
-char* strcpy(char* dst, const char* src) __overloadable
-        __error_if_overflows_dst(strcpy, dst, __builtin_strlen(src), "string");
-
 #if __ANDROID_API__ >= __ANDROID_API_M__
 __BIONIC_FORTIFY_INLINE
-void* memchr(const void* const s __pass_object_size, int c, size_t n)
-        __overloadable {
+void* memchr(const void* const s __pass_object_size, int c, size_t n) __overloadable {
     size_t bos = __bos(s);
 
     if (bos == __BIONIC_FORTIFY_UNKNOWN_SIZE) {
@@ -129,8 +116,7 @@ void* memchr(const void* const s __pass_object_size, int c, size_t n)
 }
 
 __BIONIC_FORTIFY_INLINE
-void* memrchr(const void* const s __pass_object_size, int c, size_t n)
-        __overloadable {
+void* memrchr(const void* const s __pass_object_size, int c, size_t n) __overloadable {
     size_t bos = __bos(s);
 
     if (bos == __BIONIC_FORTIFY_UNKNOWN_SIZE) {
@@ -173,8 +159,7 @@ char* strncpy(char* const dst __pass_object_size, const char* const src __pass_o
 
 #if __ANDROID_API__ >= __ANDROID_API_J_MR1__
 __BIONIC_FORTIFY_INLINE
-size_t strlcpy(char* const dst __pass_object_size, const char* src, size_t size)
-        __overloadable {
+size_t strlcpy(char* const dst __pass_object_size, const char* src, size_t size) __overloadable {
     size_t bos = __bos(dst);
 
     if (bos == __BIONIC_FORTIFY_UNKNOWN_SIZE) {
@@ -185,8 +170,7 @@ size_t strlcpy(char* const dst __pass_object_size, const char* src, size_t size)
 }
 
 __BIONIC_FORTIFY_INLINE
-size_t strlcat(char* const dst __pass_object_size, const char* src, size_t size)
-        __overloadable {
+size_t strlcat(char* const dst __pass_object_size, const char* src, size_t size) __overloadable {
     size_t bos = __bos(dst);
 
     if (bos == __BIONIC_FORTIFY_UNKNOWN_SIZE) {
@@ -210,15 +194,13 @@ size_t strlen(const char* const s __pass_object_size)
 }
 
 __BIONIC_FORTIFY_INLINE
-size_t strlen(const char* const s __pass_object_size0)
-        __overloadable {
+size_t strlen(const char* const s __pass_object_size0) __overloadable {
     size_t bos = __bos0(s);
 
     if (bos == __BIONIC_FORTIFY_UNKNOWN_SIZE) {
         return __builtin_strlen(s);
     }
 
-    // return __builtin_strlen(s);
     return __strlen_chk(s, bos);
 }
 #endif /* __ANDROID_API__ >= __ANDROID_API_J_MR1__ */
@@ -247,38 +229,6 @@ char* strrchr(const char* const s __pass_object_size, int c) __overloadable {
 }
 #endif /* __ANDROID_API__ >= __ANDROID_API_J_MR2__ */
 
-#if __ANDROID_API__ >= __ANDROID_API_J_MR1__
-/* In *many* cases, memset(foo, sizeof(foo), 0) is a mistake where the user has
- * flipped the size + value arguments. However, there may be cases (e.g. with
- * macros) where it's okay for the size to fold to zero. We should warn on this,
- * but we should also provide a FORTIFY'ed escape hatch.
- */
-__BIONIC_ERROR_FUNCTION_VISIBILITY
-void* memset(void* s, int c, size_t n, struct __bionic_zero_size_is_okay_t ok)
-        __overloadable
-        __error_if_overflows_dst(memset, s, n, "size");
-
-__BIONIC_FORTIFY_INLINE
-void* memset(void* const s __pass_object_size0, int c, size_t n, struct __bionic_zero_size_is_okay_t ok __attribute__((unused)))
-        __overloadable {
-    return __builtin___memset_chk(s, c, n, __bos0(s));
-}
-
-extern struct __bionic_zero_size_is_okay_t __bionic_zero_size_is_okay;
-/* We verify that `c` is non-zero, because as pointless as memset(foo, 0, 0) is,
- * flipping size + count will do nothing.
- */
-__BIONIC_ERROR_FUNCTION_VISIBILITY
-void* memset(void* s, int c, size_t n) __overloadable
-        __enable_if(c && !n, "selected when we'll set zero bytes")
-        __RENAME_CLANG(memset)
-        __warnattr_real("will set 0 bytes; maybe the arguments got flipped? "
-                        "(Add __bionic_zero_size_is_okay as a fourth argument "
-                        "to silence this.)");
-#endif /* __ANDROID_API__ >= __ANDROID_API_J_MR1__ */
-
-#undef __error_zero_size
-#undef __error_if_overflows_dst
 #else // defined(__clang__)
 extern char* __strncpy_real(char*, const char*, size_t) __RENAME(strncpy);
 extern void* __memrchr_real(const void*, int, size_t) __RENAME(memrchr);

diff --git a/tests/fortify_compilation_test.cpp b/tests/fortify_compilation_test.cpp
index bb2b770..7def77e 100644 (file)
--- a/tests/fortify_compilation_test.cpp
+++ b/tests/fortify_compilation_test.cpp
@@ -71,7 +71,7 @@ void test_memcpy() {
 
   // NOLINTNEXTLINE(whitespace/line_length)
   // GCC: warning: call to void* __builtin___memcpy_chk(void*, const void*, {{(long )?}}unsigned int, {{(long )?}}unsigned int) will always overflow destination buffer
-  // CLANG: error: call to unavailable function 'memcpy': memcpy called with size bigger than buffer
+  // CLANG: error: 'memcpy' called with size bigger than buffer
   memcpy(buf, "foobar", sizeof("foobar") + 100);
 }
 
@@ -80,7 +80,7 @@ void test_memmove() {
 
   // NOLINTNEXTLINE(whitespace/line_length)
   // GCC: warning: call to void* __builtin___memmove_chk(void*, const void*, {{(long )?}}unsigned int, {{(long )?}}unsigned int) will always overflow destination buffer
-  // CLANG: error: call to unavailable function 'memmove': memmove called with size bigger than buffer
+  // CLANG: error: 'memmove' called with size bigger than buffer
   memmove(buf, "foobar", sizeof("foobar"));
 }
 
@@ -89,7 +89,7 @@ void test_memset() {
 
   // NOLINTNEXTLINE(whitespace/line_length)
   // GCC: warning: call to void* __builtin___memset_chk(void*, int, {{(long )?}}unsigned int, {{(long )?}}unsigned int) will always overflow destination buffer
-  // CLANG: error: call to unavailable function 'memset': memset called with size bigger than buffer
+  // CLANG: error: 'memset' called with size bigger than buffer
   memset(buf, 0, 6);
 }
 
@@ -98,8 +98,13 @@ void test_strcpy() {
 
   // NOLINTNEXTLINE(whitespace/line_length)
   // GCC: warning: call to {{(char\* __builtin___strcpy_chk\(char\*, const char\*, unsigned int\))|(void\* __builtin___memcpy_chk\(void\*, const void\*, (long )?unsigned int, (long )?unsigned int\))}} will always overflow destination buffer
-  // CLANG: error: call to unavailable function 'strcpy': strcpy called with string bigger than buffer
+  // CLANG: error: 'strcpy' called with string bigger than buffer
   strcpy(buf, "foobar");  // NOLINT(runtime/printf)
+
+  // NOLINTNEXTLINE(whitespace/line_length)
+  // GCC: warning: call to {{(char\* __builtin___strcpy_chk\(char\*, const char\*, unsigned int\))|(void\* __builtin___memcpy_chk\(void\*, const void\*, (long )?unsigned int, (long )?unsigned int\))}} will always overflow destination buffer
+  // CLANG: error: 'strcpy' called with string bigger than buffer
+  strcpy(buf, "quux");
 }
 
 void test_stpcpy() {
@@ -107,8 +112,13 @@ void test_stpcpy() {
 
   // NOLINTNEXTLINE(whitespace/line_length)
   // GCC: warning: call to char* __builtin___stpcpy_chk(char*, const char*, {{(long )?}}unsigned int) will always overflow destination buffer
-  // CLANG: error: call to unavailable function 'stpcpy': stpcpy called with string bigger than buffer
+  // CLANG: error: 'stpcpy' called with string bigger than buffer
   stpcpy(buf, "foobar");
+
+  // NOLINTNEXTLINE(whitespace/line_length)
+  // GCC: warning: call to char* __builtin___stpcpy_chk(char*, const char*, {{(long )?}}unsigned int) will always overflow destination buffer
+  // CLANG: error: 'stpcpy' called with string bigger than buffer
+  stpcpy(buf, "quux");
 }
 
 void test_strncpy() {
@@ -307,7 +317,7 @@ void test_write_size() {
 void test_memset_args_flipped() {
   char from[4] = {0};
   // NOLINTNEXTLINE(whitespace/line_length)
-  // CLANG: 'memset' is deprecated: will set 0 bytes; maybe the arguments got flipped? (Add __bionic_zero_size_is_okay as a fourth argument to silence this.)
+  // CLANG: 'memset' will set 0 bytes; maybe the arguments got flipped?
   memset(from, sizeof(from), 0);
 }