Skip to content

Commit

Permalink
libc: add const-correct string.h overloads
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gburgessiv committed Apr 5, 2017
1 parent 07807a1 commit bd3d208
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 14 deletions.
2 changes: 1 addition & 1 deletion libc/bionic/fortify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
2 changes: 1 addition & 1 deletion libc/bionic/memmem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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/
Expand Down
2 changes: 1 addition & 1 deletion libc/bionic/ndk_cruft.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
87 changes: 86 additions & 1 deletion libc/include/string.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions libc/include/sys/cdefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
22 changes: 14 additions & 8 deletions tests/fortify_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit bd3d208

Please sign in to comment.