fortify: rewrite strlen to fold to a constant
This new spelling allows Clang, when parsing C, to fold `strlen("foo")`
into a constant in many more places.
This shouldn't be much of a performance diff, since LLVM is already
fully capable of folding `@__strlen_chk` into `@strlen` or a constant as
appropriate. Mostly it matters for places where constants may be
required, or may result in materially different codegen (e.g., static
initializers, array bounds, ...)
Bug: 139143453
Test: mma
Change-Id: Ib1636402a11338f6e05a731cc5a59c919cca09e8
diff --git a/libc/include/bits/fortify/string.h b/libc/include/bits/fortify/string.h
index 4d32b04..041967b 100644
--- a/libc/include/bits/fortify/string.h
+++ b/libc/include/bits/fortify/string.h
@@ -220,8 +220,13 @@
}
#if __BIONIC_FORTIFY_RUNTIME_CHECKS_ENABLED
-__BIONIC_FORTIFY_INLINE
-size_t strlen(const char* _Nonnull const s __pass_object_size0) __overloadable {
+/*
+ * Clang, when parsing C, can fold strlen to a constant without LLVM's help.
+ * This doesn't apply to overloads of strlen, so write this differently. We
+ * can't use `__pass_object_size0` here, but that's fine: it doesn't help much
+ * on __always_inline functions.
+ */
+extern __always_inline __inline__ __attribute__((gnu_inline)) size_t strlen(const char* _Nonnull s) {
return __strlen_chk(s, __bos0(s));
}
#endif
diff --git a/tests/Android.bp b/tests/Android.bp
index deb2843..226453a 100644
--- a/tests/Android.bp
+++ b/tests/Android.bp
@@ -745,6 +745,40 @@
},
}
+cc_defaults {
+ name: "bionic_fortify_c_tests_defaults",
+ defaults: [
+ "bionic_clang_fortify_tests_w_flags",
+ "bionic_tests_defaults",
+ ],
+ cflags: [
+ "-U_FORTIFY_SOURCE",
+ // -fbuiltin is required here to counteract -fno-builtin from
+ // `bionic_tests_defaults`. With `-fno-builtin`, Clang won't
+ // const-evaluate calls to `strlen`, which is tested for here.
+ "-fbuiltin",
+ ],
+ srcs: [
+ "clang_fortify_c_only_tests.c",
+ ],
+ tidy: false,
+ shared: {
+ enabled: false,
+ },
+}
+
+cc_test_library {
+ name: "libfortify1-c-tests-clang",
+ defaults: ["bionic_fortify_c_tests_defaults"],
+ cflags: ["-D_FORTIFY_SOURCE=1"],
+}
+
+cc_test_library {
+ name: "libfortify2-c-tests-clang",
+ defaults: ["bionic_fortify_c_tests_defaults"],
+ cflags: ["-D_FORTIFY_SOURCE=2"],
+}
+
// -----------------------------------------------------------------------------
// Library of all tests (excluding the dynamic linker tests).
// -----------------------------------------------------------------------------
@@ -756,8 +790,10 @@
"libBionicStandardTests",
"libBionicElfTlsTests",
"libBionicFramePointerTests",
+ "libfortify1-c-tests-clang",
"libfortify1-tests-clang",
"libfortify1-new-tests-clang",
+ "libfortify2-c-tests-clang",
"libfortify2-tests-clang",
"libfortify2-new-tests-clang",
],
diff --git a/tests/clang_fortify_c_only_tests.c b/tests/clang_fortify_c_only_tests.c
new file mode 100644
index 0000000..3bec848
--- /dev/null
+++ b/tests/clang_fortify_c_only_tests.c
@@ -0,0 +1,38 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <string.h>
+
+// This is a test specifically of bionic's FORTIFY machinery. Other stdlibs need not apply.
+#ifdef __BIONIC__
+
+// Ensure that strlen can be evaluated at compile-time. Clang doesn't support
+// this in C++, but does in C.
+_Static_assert(strlen("foo") == 3, "");
+
+#endif // __BIONIC__
diff --git a/tests/clang_fortify_tests.cpp b/tests/clang_fortify_tests.cpp
index f08fd1f..105c261 100644
--- a/tests/clang_fortify_tests.cpp
+++ b/tests/clang_fortify_tests.cpp
@@ -89,6 +89,8 @@
#include <unistd.h>
#include <wchar.h>
+#include <array>
+
#ifndef COMPILATION_TESTS
#include <android-base/silent_death_test.h>
#include <gtest/gtest.h>
@@ -133,6 +135,25 @@
const static int kBogusFD = -1;
+FORTIFY_TEST(strlen) {
+ auto run_strlen_with_contents = [&](std::array<char, 3> contents) {
+ // A lot of cruft is necessary to make this test DTRT. LLVM and Clang love to fold/optimize
+ // strlen calls, and that's the opposite of what we want to happen.
+
+ // Loop to convince LLVM that `contents` can never be known (since `xor volatile_value` can flip
+ // any bit in each elem of `contents`).
+ volatile char always_zero = 0;
+ for (char& c : contents) {
+ c ^= always_zero;
+ }
+ // Store in a volatile, so the strlen itself cannot be optimized out.
+ volatile size_t _strlen_result = strlen(&contents.front());
+ };
+
+ EXPECT_NO_DEATH(run_strlen_with_contents({'f', 'o', '\0'}));
+ EXPECT_FORTIFY_DEATH(run_strlen_with_contents({'f', 'o', 'o'}));
+}
+
FORTIFY_TEST(string) {
char small_buffer[8] = {};