Enable crashpad for arm, and cxx17, toolchains (#7479)

hlwarriner · web-flow · commit f4e01b08e6e8 · 2025-10-14T12:22:29.000-04:00
This PR includes a few sets of changes to enable crash reporting on our
evergreen-raspi reference platform:
* Customizations to build //base wtih cxx17, which partner toolchains
can opt into with a toolchain arg
* Customizations to build zlib with arm toolchains that are on a version
less than ARMv8
* Compiler flags added to the raspi port to disable warnings that are
not issued by more modern compilers.

With these changes it should now be fairly straightforward for most
partners to enable crashpad on their platforms.

Issue: 446904317
Issue: 406511608
diff --git a/base/allocator/partition_allocator/partition_alloc.gni b/base/allocator/partition_allocator/partition_alloc.gni
@@ -47,7 +47,15 @@ declare_args() {
   #     and doesn't wish to incur the library size increase (crbug.com/674570).
   # 2.  On NaCl (through this declaration), where PartitionAlloc doesn't
   #     build at all.
-  use_partition_alloc = !is_nacl
+  #
+  # Cobalt comment: it would be a significant change to use PartitionAlloc as
+  # the default allocator in components built by partners. We're considering
+  # that change in b/447346129 but for now set this false in partner toolchains
+  # so that PartitionAlloc is not even linked into partner built binaries.
+  use_partition_alloc =
+      !is_nacl && !(is_cobalt && ((current_toolchain == starboard_toolchain &&
+                                   starboard_toolchain != default_toolchain) ||
+                                  current_toolchain == native_target))
 }
 
 declare_args() {
diff --git a/base/allocator/partition_allocator/partition_alloc_base/cxx20_is_constant_evaluated.h b/base/allocator/partition_allocator/partition_alloc_base/cxx20_is_constant_evaluated.h
@@ -5,6 +5,8 @@
 #ifndef BASE_ALLOCATOR_PARTITION_ALLOCATOR_PARTITION_ALLOC_BASE_CXX20_IS_CONSTANT_EVALUATED_H_
 #define BASE_ALLOCATOR_PARTITION_ALLOCATOR_PARTITION_ALLOC_BASE_CXX20_IS_CONSTANT_EVALUATED_H_
 
+#include "build/build_config.h"
+
 namespace partition_alloc::internal::base {
 
 // std::is_constant_evaluated was introduced in C++20. PartitionAlloc's minimum
@@ -23,7 +25,15 @@ using std::is_constant_evaluated;
 // - https://en.cppreference.com/w/cpp/types/is_constant_evaluated
 // - https://wg21.link/meta.const.eval
 constexpr bool is_constant_evaluated() noexcept {
+#if BUILDFLAG(BUILD_BASE_WITH_CPP17)
+  // Compilers are not guaranteed to provide this builtin. Always returning
+  // false should be safe: if a calling function that uses this result to select
+  // a runtime or compile-time path were actually used in a constant-evaluated
+  // context then we should get a compile-time error.
+  return false;
+#else
   return __builtin_is_constant_evaluated();
+#endif
 }
 
 #endif
diff --git a/base/allocator/partition_allocator/partition_alloc_base/numerics/safe_conversions_impl.h b/base/allocator/partition_allocator/partition_alloc_base/numerics/safe_conversions_impl.h
@@ -10,6 +10,8 @@
 #include <limits>
 #include <type_traits>
 
+#include "build/build_config.h"
+
 #if defined(__GNUC__) || defined(__clang__)
 #define PA_BASE_NUMERICS_LIKELY(x) __builtin_expect(!!(x), 1)
 #define PA_BASE_NUMERICS_UNLIKELY(x) __builtin_expect(!!(x), 0)
@@ -85,9 +87,20 @@ constexpr typename std::make_unsigned<T>::type SafeUnsignedAbs(T value) {
              : static_cast<UnsignedT>(value);
 }
 
+#if BUILDFLAG(BUILD_BASE_WITH_CPP17)
+constexpr bool pa_is_constant_evaluated_stub() noexcept {
+  // Compilers are not guaranteed to provide this builtin. Always returning
+  // false should be safe: if a calling function that uses this result to select
+  // a runtime or compile-time path were actually used in a constant-evaluated
+  // context then we should get a compile-time error.
+  return false;
+}
+#define PA_IsConstantEvaluated() (pa_is_constant_evaluated_stub())
+#else
 // TODO(jschuh): Switch to std::is_constant_evaluated() once C++20 is supported.
 // Alternately, the usage could be restructured for "consteval if" in C++23.
 #define PA_IsConstantEvaluated() (__builtin_is_constant_evaluated())
+#endif
 
 // TODO(jschuh): Debug builds don't reliably propagate constants, so we restrict
 // some accelerated runtime paths to release builds until this can be forced
diff --git a/base/containers/flat_tree.h b/base/containers/flat_tree.h
@@ -17,6 +17,7 @@
 #include "base/functional/not_fn.h"
 #include "base/memory/raw_ptr_exclusion.h"
 #include "base/ranges/algorithm.h"
+#include "build/build_config.h"
 
 namespace base {
 
@@ -158,7 +159,12 @@ class flat_tree {
   struct value_compare {
     constexpr bool operator()(const value_type& left,
                               const value_type& right) const {
+#if BUILDFLAG(BUILD_BASE_WITH_CPP17)
+      // C++17 does not allow default initialization here.
+      GetKeyFromValue extractor{};
+#else
       GetKeyFromValue extractor;
+#endif
       return comp(extractor(left), extractor(right));
     }
 
diff --git a/base/cxx20_is_constant_evaluated.h b/base/cxx20_is_constant_evaluated.h
@@ -5,6 +5,8 @@
 #ifndef BASE_CXX20_IS_CONSTANT_EVALUATED_H_
 #define BASE_CXX20_IS_CONSTANT_EVALUATED_H_
 
+#include "build/build_config.h"
+
 namespace base {
 
 // Implementation of C++20's std::is_constant_evaluated.
@@ -13,7 +15,15 @@ namespace base {
 // - https://en.cppreference.com/w/cpp/types/is_constant_evaluated
 // - https://wg21.link/meta.const.eval
 constexpr bool is_constant_evaluated() noexcept {
+#if BUILDFLAG(BUILD_BASE_WITH_CPP17)
+  // Compilers are not guaranteed to provide this builtin. Always returning
+  // false should be safe: if a calling function that uses this result to select
+  // a runtime or compile-time path were actually used in a constant-evaluated
+  // context then we should get a compile-time error.
+  return false;
+#else
   return __builtin_is_constant_evaluated();
+#endif
 }
 
 }  // namespace base
diff --git a/base/numerics/safe_conversions_impl.h b/base/numerics/safe_conversions_impl.h
@@ -10,6 +10,8 @@
 #include <limits>
 #include <type_traits>
 
+#include "build/build_config.h"
+
 #if defined(__GNUC__) || defined(__clang__)
 #define BASE_NUMERICS_LIKELY(x) __builtin_expect(!!(x), 1)
 #define BASE_NUMERICS_UNLIKELY(x) __builtin_expect(!!(x), 0)
@@ -86,9 +88,20 @@ constexpr typename std::make_unsigned<T>::type SafeUnsignedAbs(T value) {
              : static_cast<UnsignedT>(value);
 }
 
+#if BUILDFLAG(BUILD_BASE_WITH_CPP17)
+constexpr bool is_constant_evaluated_stub() noexcept {
+  // Compilers are not guaranteed to provide this builtin. Always returning
+  // false should be safe: if a calling function that uses this result to select
+  // a runtime or compile-time path were actually used in a constant-evaluated
+  // context then we should get a compile-time error.
+  return false;
+}
+#define IsConstantEvaluated() (is_constant_evaluated_stub())
+#else
 // TODO(jschuh): Switch to std::is_constant_evaluated() once C++20 is supported.
 // Alternately, the usage could be restructured for "consteval if" in C++23.
 #define IsConstantEvaluated() (__builtin_is_constant_evaluated())
+#endif
 
 // TODO(jschuh): Debug builds don't reliably propagate constants, so we restrict
 // some accelerated runtime paths to release builds until this can be forced
diff --git a/base/task/sequence_manager/sequence_manager.cc b/base/task/sequence_manager/sequence_manager.cc
@@ -6,6 +6,8 @@
 
 #include <utility>
 
+#include "build/build_config.h"
+
 namespace base {
 namespace sequence_manager {
 
@@ -104,7 +106,11 @@ SequenceManager::PrioritySettings& SequenceManager::PrioritySettings::operator=(
 
 SequenceManager::Settings::Settings() = default;
 
+#if BUILDFLAG(BUILD_BASE_WITH_CPP17)
+SequenceManager::Settings::Settings(Settings&& move_from) = default;
+#else
 SequenceManager::Settings::Settings(Settings&& move_from) noexcept = default;
+#endif
 
 SequenceManager::Settings::~Settings() = default;
 
diff --git a/base/task/sequence_manager/sequence_manager.h b/base/task/sequence_manager/sequence_manager.h
@@ -21,6 +21,7 @@
 #include "base/task/sequenced_task_runner.h"
 #include "base/task/single_thread_task_runner.h"
 #include "base/time/default_tick_clock.h"
+#include "build/build_config.h"
 
 namespace base {
 
@@ -163,7 +164,13 @@ class BASE_EXPORT SequenceManager {
     Settings& operator=(const Settings&) = delete;
     // In the future MessagePump (which is move-only) will also be a setting,
     // so we are making Settings move-only in preparation.
+#if BUILDFLAG(BUILD_BASE_WITH_CPP17)
+    // The compiler does not agree that the default move constructor should be
+    // declared as non-throwing.
+    Settings(Settings&& move_from);
+#else
     Settings(Settings&& move_from) noexcept;
+#endif
 
     ~Settings();
 
diff --git a/base/trace_event/heap_profiler_allocation_context.cc b/base/trace_event/heap_profiler_allocation_context.cc
@@ -8,6 +8,7 @@
 #include <cstring>
 
 #include "base/hash/hash.h"
+#include "build/build_config.h"
 
 namespace base {
 namespace trace_event {
@@ -26,6 +27,10 @@ bool operator != (const StackFrame& lhs, const StackFrame& rhs) {
 
 Backtrace::Backtrace() = default;
 
+#if BUILDFLAG(BUILD_BASE_WITH_CPP17)
+Backtrace::Backtrace(const Backtrace& other) = default;
+#endif
+
 bool operator==(const Backtrace& lhs, const Backtrace& rhs) {
   if (lhs.frame_count != rhs.frame_count) return false;
   return std::equal(lhs.frames, lhs.frames + lhs.frame_count, rhs.frames);
diff --git a/base/trace_event/heap_profiler_allocation_context.h b/base/trace_event/heap_profiler_allocation_context.h
@@ -12,6 +12,7 @@
 
 #include "base/base_export.h"
 #include "base/memory/raw_ptr.h"
+#include "build/build_config.h"
 
 namespace base {
 namespace trace_event {
@@ -53,6 +54,11 @@ bool BASE_EXPORT operator != (const StackFrame& lhs, const StackFrame& rhs);
 struct BASE_EXPORT Backtrace {
   Backtrace();
 
+#if BUILDFLAG(BUILD_BASE_WITH_CPP17)
+  // The copy constructor is for some reason deleted by the compiler.
+  Backtrace(const Backtrace& other);
+#endif
+
   // If the stack is higher than what can be stored here, the top frames
   // (the ones further from main()) are stored. Depth of 12 is enough for most
   // pseudo traces (see above), but not for native traces, where we need more.
diff --git a/base/trace_event/memory_infra_background_allowlist.cc b/base/trace_event/memory_infra_background_allowlist.cc
@@ -18,17 +18,52 @@
 #include "base/android/meminfo_dump_provider.h"
 #endif
 
+#if BUILDFLAG(BUILD_BASE_WITH_CPP17)
+#include <utility>
+#endif
+
 namespace base {
 namespace trace_event {
 namespace {
 
+#if BUILDFLAG(BUILD_BASE_WITH_CPP17)
+// A helper that uses an index sequence to expand the array.
+template <size_t N, size_t... I>
+constexpr auto MakeFixedFlatSetFromStringLiteralsHelper(
+    const char* const (&literals)[N],
+    std::index_sequence<I...>) {
+  // This expands at compile time to:
+  // MakeFixedFlatSet<StringPiece>({StringPiece{literals[0]},
+  //                                StringPiece{literals[1]}, ...}).
+  return base::MakeFixedFlatSet<base::StringPiece>(
+      {base::StringPiece{literals[I]}...});
+}
+
+// A custom wrapper to make a fixed_flat_set from string literals in C++17.
+// This wrapper can be pushed up to base/containers/fixed_flat_set.h if/when
+// there are use cases outside of this file.
+template <size_t N>
+constexpr auto MakeFixedFlatSetFromStringLiterals(
+    const char* const (&literals)[N]) {
+  // Creates an index sequence from 0 to N-1 and forwards to the helper.
+  return MakeFixedFlatSetFromStringLiteralsHelper(
+      literals, std::make_index_sequence<N>{});
+}
+#endif  // BUILDFLAG(BUILD_BASE_WITH_CPP17)
+
 // The names of dump providers allowed to perform background tracing. Dump
 // providers can be added here only if the background mode dump has very
 // little processor and memory overhead.
 // TODO(ssid): Some dump providers do not create ownership edges on background
 // dump. So, the effective size will not be correct.
 constexpr auto kDumpProviderAllowlist =
+#if BUILDFLAG(BUILD_BASE_WITH_CPP17)
+    // We use a custom wrapper to sidestep the compiler's inability to perform
+    // template argument deduction as expected.
+    MakeFixedFlatSetFromStringLiterals({
+#else
     base::MakeFixedFlatSet<base::StringPiece>({
+#endif
 // clang-format off
 #if BUILDFLAG(IS_ANDROID)
         base::android::MeminfoDumpProvider::kDumpProviderName,
@@ -93,8 +128,14 @@ constexpr auto kDumpProviderAllowlist =
 
 // A list of string names that are allowed for the memory allocator dumps in
 // background mode.
-constexpr auto kAllocatorDumpNameAllowlist = base::MakeFixedFlatSet<
-    base::StringPiece>({
+constexpr auto kAllocatorDumpNameAllowlist =
+#if BUILDFLAG(BUILD_BASE_WITH_CPP17)
+    // We use a custom wrapper to sidestep the compiler's inability to perform
+    // template argument deduction as expected.
+    MakeFixedFlatSetFromStringLiterals({
+#else
+    base::MakeFixedFlatSet<base::StringPiece>({
+#endif
 // clang-format off
         // Some of the blink values vary based on compile time flags. The
         // compile time flags are not in base, so all are listed here.
diff --git a/base/values.cc b/base/values.cc
@@ -615,7 +615,13 @@ absl::optional<Value> Value::Dict::Extract(StringPiece key) {
   }
   Value v = std::move(*it->second);
   storage_.erase(it);
+#if BUILDFLAG(BUILD_BASE_WITH_CPP17)
+  // Use the move constructor to sidestep an implicit conversion issue with the
+  // copy constructor.
+  return std::move(v);
+#else
   return v;
+#endif
 }
 
 const Value* Value::Dict::FindByDottedPath(StringPiece path) const {
diff --git a/build/build_config.h b/build/build_config.h
@@ -323,6 +323,12 @@
 #define BUILDFLAG_INTERNAL_IS_NATIVE_TARGET_BUILD() (0)
 #endif
 
+#if defined(ENABLE_BUILDFLAG_BUILD_BASE_WITH_CPP17)
+#define BUILDFLAG_INTERNAL_BUILD_BASE_WITH_CPP17() (1)
+#else
+#define BUILDFLAG_INTERNAL_BUILD_BASE_WITH_CPP17() (0)
+#endif
+
 // Compiler detection. Note: clang masquerades as GCC on POSIX and as MSVC on
 // Windows.
 #if defined(__GNUC__)
diff --git a/build/config/BUILD.gn b/build/config/BUILD.gn
@@ -218,7 +218,12 @@ config("default_libs") {
       "CoreText.framework",
       "Foundation.framework",
     ]
-  } else if (is_linux && !is_cobalt_hermetic_build || is_chromeos) {
+  } else if (is_cobalt && is_native_target_build) {
+    # This build configuration is only used for the crashpad handler executable.
+    # This binary needs dynamic linking but gets undefined symbols, which result
+    # in symbol lookup errors at runtime, when also linked with pthread.
+    libs = [ "dl" ]
+  } else if ((is_linux && !is_cobalt_hermetic_build) || is_chromeos) {
     libs = [
       "dl",
       "pthread",
diff --git a/cobalt/BUILD.gn b/cobalt/BUILD.gn
@@ -49,10 +49,7 @@ group("gn_all") {
   if (is_starboard && build_updater_sandbox) {
     deps += [ "//chrome/updater/cobalt:updater_sandbox" ]
   }
-
-  # TODO: b/446904317 - add these deps for all 3P platforms once build issues
-  # are resolved.
-  if (target_os == "linux" && target_cpu == "x64") {
+  if (use_crashpad) {
     deps += [
       "//third_party/crashpad/crashpad/handler:crashpad_handler($native_target)",
       "//third_party/crashpad/crashpad/tools:crashpad_database_util($native_target)",
diff --git a/cobalt/build/configs/BUILD.gn b/cobalt/build/configs/BUILD.gn
@@ -63,6 +63,13 @@ config("default_includes") {
   }
 }
 
+declare_args() {
+  # Toolchains can set this true in order to build //base using C++17. Our
+  # version of this upstream library assumes it will be built using C++20 and
+  # some customizations are needed to instead build using C++17.
+  build_base_with_cpp17 = false
+}
+
 config("buildflag_defines") {
   defines = []
 
@@ -147,4 +154,10 @@ config("buildflag_defines") {
   if (is_native_target_build) {
     defines += [ "ENABLE_BUILDFLAG_IS_NATIVE_TARGET_BUILD" ]
   }
+
+  # Use build_base_with_cpp17 and BUILDFLAG(BUILD_BASE_WITH_CPP17) to guard
+  # customizations to //base in order to build the library using C++17.
+  if (build_base_with_cpp17) {
+    defines += [ "ENABLE_BUILDFLAG_BUILD_BASE_WITH_CPP17" ]
+  }
 }
diff --git a/starboard/build/config/base_configuration.gni b/starboard/build/config/base_configuration.gni
@@ -149,4 +149,11 @@ declare_args() {
   # all unused symbols and nplb fails to link.
   # TODO: b/297808555 Remove this flag after nplb is fixed
   sb_has_unused_symbol_issue = false
+
+  # Platforms can set this to true in order to build and use a real crashpad
+  # implementation. If false, crashpad either won't be built or a stub
+  # implementation will be built and used.
+  # TODO: b/406511608 - remove once crashpad builds and behaves as expected on
+  # all platforms.
+  use_crashpad = false
 }
diff --git a/starboard/elf_loader/BUILD.gn b/starboard/elf_loader/BUILD.gn
diff --git a/starboard/linux/x64x11/shared/platform_configuration/configuration.gni b/starboard/linux/x64x11/shared/platform_configuration/configuration.gni
diff --git a/starboard/loader_app/BUILD.gn b/starboard/loader_app/BUILD.gn
diff --git a/starboard/raspi/2/platform_configuration/configuration.gni b/starboard/raspi/2/platform_configuration/configuration.gni
diff --git a/starboard/raspi/2/toolchain/BUILD.gn b/starboard/raspi/2/toolchain/BUILD.gn
diff --git a/starboard/raspi/shared/platform_configuration/BUILD.gn b/starboard/raspi/shared/platform_configuration/BUILD.gn
diff --git a/starboard/raspi/shared/run_starboard_main.cc b/starboard/raspi/shared/run_starboard_main.cc
diff --git a/third_party/zlib/BUILD.gn b/third_party/zlib/BUILD.gn
