libc: add const-correct string.h overloads

libcxx provides const-correct overloads for a few string.h functions.
These overloads use clang's enable_if attribute, so they're preferred
over our FORTIFY'ed equivalents.

This weakens _FORTIFY_SOURCE=2 when used with some of these functions,
since clang needs to see __pass_object_size in order to pass an accurate
result for __builtin_object_size(s, 1) at a callsite. Since those
functions don't have __pass_object_size on their params, clang can't do
that. This makes LLVM lower the __builtin_object_size calls, which means
we get the same result as __builtin_object_size(s, 0).

We have to provide all of the overloads in Bionic, since enable_if is
only used to disambiguate overloads with (otherwise) the same type. In
other words:

// overload 1
char *strchr(const char *, int s) __attribute__((enable_if(1, "")));
// overload 2
char *strchr(char *, int s);

void foo() {
  char cs[1] = {};
  strchr(static_cast<const char *>(cs), '\0'); // calls overload #1.
  strchr(cs, '\0'); // calls overload #2.
}

Bug: 34747525
Test: m checkbuild on bullhead internal master + AOSP. vts -m
BionicUnitTests passes on both. Surprisingly, the only code that this
seems to break is contained in Bionic.

Change-Id: Ie406f42fb3d1c5bf940dc857889876fc39b57c90

diff --git a/libc/bionic/fortify.cpp b/libc/bionic/fortify.cpp
index cf2d1c2..cfbfbc5 100644 (file)
--- a/libc/bionic/fortify.cpp
+++ b/libc/bionic/fortify.cpp
@@ -133,7 +133,7 @@ extern char* __getcwd_chk(char* buf, size_t len, size_t actual_size) {
 
 void* __memchr_chk(const void* s, int c, size_t n, size_t actual_size) {
   __check_buffer_access("memchr", "read from", n, actual_size);
-  return memchr(s, c, n);
+  return const_cast<void*>(memchr(s, c, n));
 }
 
 // Runtime implementation of __builtin____memmove_chk (used directly by compiler, not in headers).

diff --git a/libc/bionic/memmem.cpp b/libc/bionic/memmem.cpp
index 61d681f..019e772 100644 (file)
--- a/libc/bionic/memmem.cpp
+++ b/libc/bionic/memmem.cpp
@@ -35,7 +35,7 @@ void* memmem(const void* void_haystack, size_t n, const void* void_needle, size_
   if (n < m) return nullptr;
 
   if (m == 0) return const_cast<void*>(void_haystack);
-  if (m == 1) return memchr(haystack, needle[0], n);
+  if (m == 1) return const_cast<void*>(memchr(haystack, needle[0], n));
 
   // This uses the "Not So Naive" algorithm, a very simple but usually effective algorithm.
   // http://www-igm.univ-mlv.fr/~lecroq/string/

diff --git a/libc/bionic/ndk_cruft.cpp b/libc/bionic/ndk_cruft.cpp
index c042f9f..29565a2 100644 (file)
--- a/libc/bionic/ndk_cruft.cpp
+++ b/libc/bionic/ndk_cruft.cpp
@@ -322,7 +322,7 @@ int ftime(struct timeb* tb) {
 
 // This was removed from POSIX 2008.
 char* index(const char* str, int ch) {
-  return strchr(str, ch);
+  return const_cast<char*>(strchr(str, ch));
 }
 
 // This was removed from BSD.

diff --git a/libc/include/string.h b/libc/include/string.h
index 0cbd61c..2027231 100644 (file)
--- a/libc/include/string.h
+++ b/libc/include/string.h
@@ -366,7 +366,6 @@ char* strchr(const char* const _Nonnull s __pass_object_size, int c)
         return __builtin_strchr(s, c);
     }
 
-    // return __builtin_strchr(s, c);
     return __strchr_chk(s, c, bos);
 }
 
@@ -606,6 +605,92 @@ char* strrchr(const char* _Nonnull s, int c) {
 #endif /* defined(__clang__) */
 #endif /* defined(__BIONIC_FORTIFY) */
 
+/* Const-correct overloads. Placed after FORTIFY so we call those functions, if possible. */
+#if defined(__cplusplus) && defined(__clang__)
+/*
+ * Use two enable_ifs so these overloads don't conflict with + are preferred over libcxx's. This can
+ * be reduced to 1 after libcxx recognizes that we have const-correct overloads.
+ */
+#define __prefer_this_overload __enable_if(true, "preferred overload") __enable_if(true, "")
+extern "C++" {
+inline __always_inline
+void* __bionic_memchr(const void* const _Nonnull s __pass_object_size, int c, size_t n) {
+    return memchr(s, c, n);
+}
+
+inline __always_inline
+const void* memchr(const void* const _Nonnull s __pass_object_size, int c, size_t n)
+        __prefer_this_overload {
+    return __bionic_memchr(s, c, n);
+}
+
+inline __always_inline
+void* memchr(void* const _Nonnull s __pass_object_size, int c, size_t n) __prefer_this_overload {
+    return __bionic_memchr(s, c, n);
+}
+
+inline __always_inline
+char* __bionic_strchr(const char* const _Nonnull s __pass_object_size, int c) {
+    return strchr(s, c);
+}
+
+inline __always_inline
+const char* strchr(const char* const _Nonnull s __pass_object_size, int c)
+        __prefer_this_overload {
+    return __bionic_strchr(s, c);
+}
+
+inline __always_inline
+char* strchr(char* const _Nonnull s __pass_object_size, int c)
+        __prefer_this_overload {
+    return __bionic_strchr(s, c);
+}
+
+inline __always_inline
+char* __bionic_strrchr(const char* const _Nonnull s __pass_object_size, int c) {
+    return strrchr(s, c);
+}
+
+inline __always_inline
+const char* strrchr(const char* const _Nonnull s __pass_object_size, int c) __prefer_this_overload {
+    return __bionic_strrchr(s, c);
+}
+
+inline __always_inline
+char* strrchr(char* const _Nonnull s __pass_object_size, int c) __prefer_this_overload {
+    return __bionic_strrchr(s, c);
+}
+
+/* Functions with no FORTIFY counterpart. */
+inline __always_inline
+char* __bionic_strstr(const char* _Nonnull h, const char* _Nonnull n) { return strstr(h, n); }
+
+inline __always_inline
+const char* strstr(const char* _Nonnull h, const char* _Nonnull n) __prefer_this_overload {
+    return __bionic_strstr(h, n);
+}
+
+inline __always_inline
+char* strstr(char* _Nonnull h, const char* _Nonnull n) __prefer_this_overload {
+    return __bionic_strstr(h, n);
+}
+
+inline __always_inline
+char* __bionic_strpbrk(const char* _Nonnull h, const char* _Nonnull n) { return strpbrk(h, n); }
+
+inline __always_inline
+char* strpbrk(char* _Nonnull h, const char* _Nonnull n) __prefer_this_overload {
+    return __bionic_strpbrk(h, n);
+}
+
+inline __always_inline
+const char* strpbrk(const char* _Nonnull h, const char* _Nonnull n) __prefer_this_overload {
+    return __bionic_strpbrk(h, n);
+}
+}
+#undef __prefer_this_overload
+#endif
+
 #if defined(__clang__)
 #pragma clang diagnostic pop
 #endif

diff --git a/libc/include/sys/cdefs.h b/libc/include/sys/cdefs.h
index dab252d..aa93c78 100644 (file)
--- a/libc/include/sys/cdefs.h
+++ b/libc/include/sys/cdefs.h
@@ -297,9 +297,12 @@
 /* __BIONIC_FORTIFY_NONSTATIC_INLINE is pointless in GCC's FORTIFY */
 #    define __BIONIC_FORTIFY_INLINE extern __inline__ __always_inline __attribute__((gnu_inline)) __attribute__((__artificial__))
 #  endif
-#  define __pass_object_size __pass_object_size_n(__bos_level)
-#  define __pass_object_size0 __pass_object_size_n(0)
+#else
+/* Further increase sharing for some inline functions */
+#  define __pass_object_size_n(n)
 #endif
+#define __pass_object_size __pass_object_size_n(__bos_level)
+#define __pass_object_size0 __pass_object_size_n(0)
 
 /* Used to support clangisms with FORTIFY. This isn't in the FORTIFY section
  * because these change how symbols are emitted. The linker must be kept happy.

diff --git a/tests/fortify_test.cpp b/tests/fortify_test.cpp
index 86b282c..c21c9da 100644 (file)
--- a/tests/fortify_test.cpp
+++ b/tests/fortify_test.cpp
@@ -231,35 +231,41 @@ TEST_F(DEATHTEST, strcpy3_fortified2) {
 }
 #endif
 
-#ifndef __clang__
-// This test is disabled in clang because clang doesn't properly detect
-// this buffer overflow. TODO: Fix clang.
 TEST_F(DEATHTEST, strchr_fortified2) {
 #if defined(__BIONIC__)
   foo myfoo;
   memcpy(myfoo.a, "0123456789", sizeof(myfoo.a));
   myfoo.b[0] = '\0';
   ASSERT_FORTIFY(printf("%s", strchr(myfoo.a, 'a')));
+  ASSERT_FORTIFY(printf("%s", strchr(static_cast<const char*>(myfoo.a), 'a')));
 #else // __BIONIC__
   GTEST_LOG_(INFO) << "This test does nothing.\n";
 #endif // __BIONIC__
 }
-#endif
 
-#ifndef __clang__
-// This test is disabled in clang because clang doesn't properly detect
-// this buffer overflow. TODO: Fix clang.
 TEST_F(DEATHTEST, strrchr_fortified2) {
 #if defined(__BIONIC__)
   foo myfoo;
   memcpy(myfoo.a, "0123456789", 10);
   memcpy(myfoo.b, "01234", 6);
   ASSERT_FORTIFY(printf("%s", strrchr(myfoo.a, 'a')));
+  ASSERT_FORTIFY(printf("%s", strrchr(static_cast<const char*>(myfoo.a), 'a')));
+#else // __BIONIC__
+  GTEST_LOG_(INFO) << "This test does nothing.\n";
+#endif // __BIONIC__
+}
+
+TEST_F(DEATHTEST, memchr_fortified2) {
+#if defined(__BIONIC__)
+  foo myfoo;
+  volatile int asize = sizeof(myfoo.a) + 1;
+  memcpy(myfoo.a, "0123456789", sizeof(myfoo.a));
+  ASSERT_FORTIFY(printf("%s", memchr(myfoo.a, 'a', asize)));
+  ASSERT_FORTIFY(printf("%s", memchr(static_cast<const void*>(myfoo.a), 'a', asize)));
 #else // __BIONIC__
   GTEST_LOG_(INFO) << "This test does nothing.\n";
 #endif // __BIONIC__
 }
-#endif
 
 #ifndef __clang__
 // This test is disabled in clang because clang doesn't properly detect