[libc++] Enable assertions on all builds and add a handler for non-Windows release builds
This is mostly a reland of commit 84d5f89427e270d1830c359f9df082ff48709317
with some changes to resolve issues that resulted in the original commit
being rolled back.
The main difference in this commit is that on Windows, we enable
assertions for libc++, but we do not use our custom handler for nondebug
builds as Windows doesn't support weak symbols (crbug.com/1353463),
which are required for the libc++ assertion handler override mechanism.
Additional changes to get this to work
* Define _LIBCPP_AVAILABILITY_CUSTOM_VERBOSE_ABORT_PROVIDED.
* Disable LibcppHardeningTest.Assertions on Android (crbug.com/1353549).
* Comment fixes.
Cq-Include-Trybots: chromium/try:chromeos-amd64-generic-cfi-thin-lto-rel
Cq-Include-Trybots: chromium/try:dawn-win10-x86-deps-rel
Cq-Include-Trybots: chromium/try:linux-chromeos-dbg
Cq-Include-Trybots: chromium/try:linux_chromium_cfi_rel_ng
Cq-Include-Trybots: chromium/try:linux_chromium_chromeos_msan_rel_ng
Cq-Include-Trybots: chromium/try:mac11-arm64-rel,mac_chromium_asan_rel_ng
Cq-Include-Trybots: chromium/try:win-asan,win7-rel
Cq-Include-Trybots: chromium/try:android-official,fuchsia-official
Cq-Include-Trybots: chromium/try:mac-official,linux-official
Cq-Include-Trybots: chromium/try:win-official,win32-official
Cq-Include-Trybots: chromium/try:linux-swangle-try-x64,win-swangle-try-x86
Cq-Include-Trybots: chrome/try:linux-chromeos-chrome
Cq-Include-Trybots: chrome/try:win-chrome,win64-chrome,linux-chrome,mac-chrome
Cq-Include-Trybots: chrome/try:linux-pgo,mac-pgo,win32-pgo,win64-pgo
Binary-Size: Size increase due to added assertion checks in libc++
Fuchsia-Binary-Size: Size increase due to added assertion checks in libc++
Bug: 1335422,1353463,1353549
Change-Id: Ia282c7cc27b170c288a3c2d1f508e11be4d356ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3833545
Commit-Queue: Alan Zhao <ayzhao@google.com>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1040015}
diff --git a/base/BUILD.gn b/base/BUILD.gn
index 471bfc82..bad2090a 100644
--- a/base/BUILD.gn
+++ b/base/BUILD.gn
@@ -217,7 +217,6 @@
"base64.h",
"base64url.cc",
"base64url.h",
- "base_export.h",
"base_switches.h",
"big_endian.cc",
"big_endian.h",
@@ -370,7 +369,6 @@
"hash/hash.h",
"hash/legacy_hash.cc",
"hash/legacy_hash.h",
- "immediate_crash.h",
"json/json_common.h",
"json/json_file_value_serializer.cc",
"json/json_file_value_serializer.h",
@@ -1542,6 +1540,15 @@
"//third_party/abseil-cpp:absl",
]
+ # Windows cannot use the nodebug assertion handler because it doesn't support
+ # weak symbols, which are required to override the default libc++
+ # implementation.
+ # TODO(crbug.com/1335422): Make the non-debug assertion handler work on
+ # Windows.
+ if (use_custom_libcxx && !is_debug && !is_win) {
+ public_deps += [ ":nodebug_assertion" ]
+ }
+
# Needed for <atomic> if using newer C++ library than sysroot, except if
# building inside the cros_sdk environment - use host_toolchain as a
# more robust check for this.
@@ -2664,8 +2671,10 @@
# base_static.
static_library("base_static") {
sources = [
+ "base_export.h",
"base_switches.cc",
"base_switches.h",
+ "immediate_crash.h",
]
deps = [ "//build:chromeos_buildflags" ]
@@ -2691,6 +2700,17 @@
}
}
+if (use_custom_libcxx && !is_debug && !is_win) {
+ # nodebug_assertion.cc has to be in its own source_set instead of being
+ # included as a source in //base as otherwise its symbols won't be linked in
+ # if they end up in an archive.
+ source_set("nodebug_assertion") {
+ defines = [ "BASE_IMPLEMENTATION" ]
+ sources = [ "nodebug_assertion.cc" ]
+ deps = [ ":base_static" ]
+ }
+}
+
component("i18n") {
output_name = "base_i18n"
sources = [
@@ -3409,6 +3429,10 @@
"vlog_unittest.cc",
]
+ if (use_custom_libcxx) {
+ sources += [ "libcpp_hardening_test.cc" ]
+ }
+
# TODO(crbug.com/1304253): iOS test() targets don't support mixing Rust code
# yet.
if (!is_ios) {
diff --git a/base/libcpp_hardening_test.cc b/base/libcpp_hardening_test.cc
new file mode 100644
index 0000000..a16daab
--- /dev/null
+++ b/base/libcpp_hardening_test.cc
@@ -0,0 +1,63 @@
+// Copyright (c) 2022 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <vector>
+
+#include "build/build_config.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+#if !_LIBCPP_ENABLE_ASSERTIONS
+#error \
+ "Define _LIBCPP_ENABLE_ASSERTIONS to 1 in \
+buildtools/third_party/libc++/__config_site"
+
+#endif
+
+using ::testing::ContainsRegex;
+using ::testing::Not;
+
+// This test checks for two things:
+//
+// 0. Assertions are enabled for libc++ and cause the process to crash when
+// invoked (in this test's case, when an out of bounds access is made in
+// std::vector.
+// 1. The correct assertion handler is linked in depending on whether or not
+// this test is built in debug mode. libc++ passes the string
+// {file}:{line} assertion {expression} failed: {message}. The default
+// libc++ handler, which we use in debug mode, prints this string to stderr,
+// while the nondebug assertion handler just crashes immediately. Therefore,
+// to check that we linked in the correct assertion handler, we check for the
+// presence or absence of the above string.
+TEST(LibcppHardeningTest, Assertions) {
+ std::vector<int> vec = {0, 1, 2};
+// Windows uses the default handler even for non-debug builds as it lacks
+// support for weak symbols, which are required to override the default
+// assertion handler.
+#if defined NDEBUG && !BUILDFLAG(IS_WIN)
+// We have to explicitly check for the GTEST_HAS_DEATH_TEST macro instead of
+// using EXPECT_DEATH_IF_SUPPORTED(...) for the following reasons:
+//
+// 0. EXPECT_DEATH(...) does not support (non-escaped) parentheses in the regex,
+// so we can't use negative look arounds (https://stackoverflow.com/a/406408)
+// to check that the error message doesn't exist.
+// 1. EXPECT_DEATH_IF_SUPPORTED(...) does not support having gmock matchers as
+// the second argument if GTEST_HAS_DEATH_TEST is false.
+//
+// We also have to prevent this test from running on Android because even though
+// death tests are supported on Android, GTest death tests don't work with
+// IMMEDIATE_CRASH() (https://crbug.com/1353549#c2).
+#if GTEST_HAS_DEATH_TEST && !GTEST_OS_LINUX_ANDROID
+ EXPECT_DEATH(vec[3], Not(ContainsRegex(".*assertion.*failed:")));
+#else
+ GTEST_UNSUPPORTED_DEATH_TEST(vec[3], "", );
+#endif // GTEST_HAS_DEATH_TEST
+#else
+ EXPECT_DEATH_IF_SUPPORTED(vec[3], ".*assertion.*failed:");
+#endif // defined NDEBUG && !BUILDFLAG(IS_WIN)
+}
+
+} // namespace
diff --git a/base/nodebug_assertion.cc b/base/nodebug_assertion.cc
new file mode 100644
index 0000000..397f038
--- /dev/null
+++ b/base/nodebug_assertion.cc
@@ -0,0 +1,18 @@
+// Copyright (c) 2022 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <__config>
+#include <__verbose_abort>
+
+#include "base/base_export.h"
+#include "base/immediate_crash.h"
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+_LIBCPP_NORETURN BASE_EXPORT void __libcpp_verbose_abort(char const* format,
+ ...) {
+ IMMEDIATE_CRASH();
+}
+
+_LIBCPP_END_NAMESPACE_STD
diff --git a/buildtools/third_party/libc++/BUILD.gn b/buildtools/third_party/libc++/BUILD.gn
index 01f5a17..b743e36 100644
--- a/buildtools/third_party/libc++/BUILD.gn
+++ b/buildtools/third_party/libc++/BUILD.gn
@@ -203,5 +203,8 @@
if (!export_libcxxabi_from_executables) {
deps = [ "//buildtools/third_party/libc++abi" ]
}
+ if (!is_debug) {
+ defines += [ "_LIBCPP_AVAILABILITY_CUSTOM_VERBOSE_ABORT_PROVIDED=1" ]
+ }
}
}
diff --git a/buildtools/third_party/libc++/__config_site b/buildtools/third_party/libc++/__config_site
index 98383cd..691c324 100644
--- a/buildtools/third_party/libc++/__config_site
+++ b/buildtools/third_party/libc++/__config_site
@@ -28,4 +28,8 @@
/* #undef _LIBCPP_HAS_NO_LOCALIZATION */
#define _LIBCPP_REMOVE_TRANSITIVE_INCLUDES
+// Enable libc++ assertions for hardening and safety
+// (https://crbug.com/1335422).
+#define _LIBCPP_ENABLE_ASSERTIONS 1
+
#endif // _LIBCPP_CONFIG_SITE
