From 9ff849165a4b6c33efc2e1b10b71400ce2257fed Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Tue, 3 Dec 2024 17:00:34 -0500 Subject: [PATCH] Require C++17 Update-Note: BoringSSL now requires a C++17 toolchain. This aligns with Google's Foundational C++ Support Policy as it has now been 10 years since C++14 was published. See https://opensource.google/documentation/policies/cplusplus-support Bug: 42290600 Change-Id: I54b914ccdde2b61a76a4ebfbe67c1fb28f7ba6ac Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74467 Auto-Submit: David Benjamin Reviewed-by: Adam Langley Commit-Queue: Adam Langley --- .bcr/presubmit.yml | 2 +- BUILDING.md | 2 +- CMakeLists.txt | 10 +- crypto/fipsmodule/mldsa/mldsa.cc.inc | 147 ++++++++++----------------- crypto/rand_extra/urandom_test.cc | 28 +---- crypto/test/file_util.cc | 6 +- include/openssl/span.h | 17 +--- ssl/internal.h | 67 ++---------- 8 files changed, 74 insertions(+), 205 deletions(-) diff --git a/.bcr/presubmit.yml b/.bcr/presubmit.yml index 4c03ba75e1..32813baf0d 100644 --- a/.bcr/presubmit.yml +++ b/.bcr/presubmit.yml @@ -21,7 +21,7 @@ tasks: bazel: ${{ bazel }} build_targets: *build_targets build_flags: &macos_workaround - - '--cxxopt=-std=c++14' + - '--cxxopt=-std=c++17' - '--sandbox_block_path=/usr/local' bcr_test_module: module_path: util/bazel-example diff --git a/BUILDING.md b/BUILDING.md index 8b6203b28d..e7e7f0e8ad 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -25,7 +25,7 @@ most recent stable version of each tool. by CMake, it may be configured explicitly by setting `CMAKE_ASM_NASM_COMPILER`. - * Compilers for C11 and C++14, or later, are required. On Windows, MSVC from + * Compilers for C11 and C++17, or later, are required. On Windows, MSVC from Visual Studio 2022 or later with Windows 10 SDK 2104 or later are supported, but using the latest versions is recommended. Recent versions of GCC (6.1+) and Clang should work on non-Windows platforms, and maybe on diff --git a/CMakeLists.txt b/CMakeLists.txt index 38d63dbc83..5d96aad5bb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -97,7 +97,7 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Emscripten") set(EMSCRIPTEN 1) endif() -set(CMAKE_CXX_STANDARD 14) +set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_C_STANDARD 11) set(CMAKE_C_STANDARD_REQUIRED ON) @@ -689,13 +689,9 @@ add_executable(pki_test ${PKI_TEST_SOURCES}) target_link_libraries(pki_test test_support_lib boringssl_gtest pki crypto) add_dependencies(all_tests pki_test) -# The PKI library requires C++17, but we haven't required this everywhere yet. -# TODO(crbug.com/42290600): Remove the C++17 bits. set_target_properties( pki pki_test PROPERTIES - CXX_STANDARD 17 - CXX_STANDARD_REQUIRED YES COMPILE_FLAGS "${PKI_CXX_FLAGS}") add_executable(bssl ${BSSL_SOURCES}) @@ -718,10 +714,6 @@ if(FUZZ) set_target_properties( Fuzzer PROPERTIES COMPILE_FLAGS "-Wno-shadow -Wno-format-nonliteral -Wno-missing-prototypes -fsanitize-coverage=0" - # libFuzzer requires C++17, but we haven't required this everywhere yet. - # TODO(crbug.com/42290600): Remove this. - CXX_STANDARD 17 - CXX_STANDARD_REQUIRED TRUE ) endif() diff --git a/crypto/fipsmodule/mldsa/mldsa.cc.inc b/crypto/fipsmodule/mldsa/mldsa.cc.inc index e9166b5af7..2495a3eb2b 100644 --- a/crypto/fipsmodule/mldsa/mldsa.cc.inc +++ b/crypto/fipsmodule/mldsa/mldsa.cc.inc @@ -51,126 +51,85 @@ constexpr uint32_t kInverseDegreeMontgomery = 41978; // Constants that vary depending on ML-DSA size. // // These are implemented as templates which take the K parameter to distinguish -// the ML-DSA sizes. (At the time of writing, `if constexpr` was not available.) -// -// TODO(crbug.com/42290600): Switch this to `if constexpr` when C++17 is -// available. +// the ML-DSA sizes. template -constexpr size_t public_key_bytes(); - -template <> -constexpr size_t public_key_bytes<6>() { - return BCM_MLDSA65_PUBLIC_KEY_BYTES; -} - -template <> -constexpr size_t public_key_bytes<8>() { - return BCM_MLDSA87_PUBLIC_KEY_BYTES; +constexpr size_t public_key_bytes() { + if constexpr (K == 6) { + return BCM_MLDSA65_PUBLIC_KEY_BYTES; + } else if constexpr (K == 8) { + return BCM_MLDSA87_PUBLIC_KEY_BYTES; + } } template -constexpr size_t signature_bytes(); - -template <> -constexpr size_t signature_bytes<6>() { - return BCM_MLDSA65_SIGNATURE_BYTES; -} - -template <> -constexpr size_t signature_bytes<8>() { - return BCM_MLDSA87_SIGNATURE_BYTES; +constexpr size_t signature_bytes() { + if constexpr (K == 6) { + return BCM_MLDSA65_SIGNATURE_BYTES; + } else if constexpr (K == 8) { + return BCM_MLDSA87_SIGNATURE_BYTES; + } } template -constexpr int tau(); - -template <> -constexpr int tau<6>() { - return 49; -} - -template <> -constexpr int tau<8>() { - return 60; +constexpr int tau() { + if constexpr (K == 6) { + return 49; + } else if constexpr (K == 8) { + return 60; + } } template -constexpr int lambda_bytes(); - -template <> -constexpr int lambda_bytes<6>() { - return 192 / 8; -} - -template <> -constexpr int lambda_bytes<8>() { - return 256 / 8; +constexpr int lambda_bytes() { + if constexpr (K == 6) { + return 192 / 8; + } else if constexpr (K == 8) { + return 256 / 8; + } } template -constexpr int gamma1(); - -template <> -constexpr int gamma1<6>() { - return 1 << 19; -} - -template <> -constexpr int gamma1<8>() { - return 1 << 19; +constexpr int gamma1() { + if constexpr (K == 6 || K == 8) { + return 1 << 19; + } } template -constexpr int beta(); - -template <> -constexpr int beta<6>() { - return 196; -} - -template <> -constexpr int beta<8>() { - return 120; +constexpr int beta() { + if constexpr (K == 6) { + return 196; + } else if constexpr (K == 8) { + return 120; + } } template -constexpr int omega(); - -template <> -constexpr int omega<6>() { - return 55; -} - -template <> -constexpr int omega<8>() { - return 75; +constexpr int omega() { + if constexpr (K == 6) { + return 55; + } else if constexpr (K == 8) { + return 75; + } } template -constexpr int eta(); - -template <> -constexpr int eta<6>() { - return 4; -} - -template <> -constexpr int eta<8>() { - return 2; +constexpr int eta() { + if constexpr (K == 6) { + return 4; + } else if constexpr (K == 8) { + return 2; + } } template -constexpr int plus_minus_eta_bitlen(); - -template <> -constexpr int plus_minus_eta_bitlen<6>() { - return 4; -} - -template <> -constexpr int plus_minus_eta_bitlen<8>() { - return 3; +constexpr int plus_minus_eta_bitlen() { + if constexpr (K == 6) { + return 4; + } else if constexpr (K == 8) { + return 3; + } } // Fundamental types. diff --git a/crypto/rand_extra/urandom_test.cc b/crypto/rand_extra/urandom_test.cc index fa05f4c175..dcf9cc5603 100644 --- a/crypto/rand_extra/urandom_test.cc +++ b/crypto/rand_extra/urandom_test.cc @@ -15,6 +15,8 @@ #include #include +#include + #include #include #include @@ -305,30 +307,6 @@ static bool regs_break_syscall(int child_pid, const struct regs *orig_regs) { #endif -// SyscallResult is like std::optional. -// TODO: use std::optional when we can use C++17. -class SyscallResult { - public: - SyscallResult &operator=(int value) { - has_value_ = true; - value_ = value; - return *this; - } - - int value() const { - if (!has_value_) { - abort(); - } - return value_; - } - - bool has_value() const { return has_value_; } - - private: - bool has_value_ = false; - int value_ = 0; -}; - // memcpy_to_remote copies |n| bytes from |in_src| in the local address space, // to |dest| in the address space of |child_pid|. static void memcpy_to_remote(int child_pid, uint64_t dest, const void *in_src, @@ -471,7 +449,7 @@ static void GetTrace(std::vector *out_trace, unsigned flags, // force_result is unset to indicate that the system call should run // normally. Otherwise it's, e.g. -EINVAL, to indicate that the system call // should not run and that the given value should be injected on return. - SyscallResult force_result; + std::optional force_result; switch (regs.syscall) { case __NR_getrandom: diff --git a/crypto/test/file_util.cc b/crypto/test/file_util.cc index f086ed006f..0fcdb3fdf6 100644 --- a/crypto/test/file_util.cc +++ b/crypto/test/file_util.cc @@ -123,8 +123,7 @@ bool TemporaryFile::Init(bssl::Span content) { path_ = path; #else std::string path = temp_dir + "bssl_tmp_file.XXXXXX"; - // TODO(davidben): Use |path.data()| when we require C++17. - int fd = mkstemp(&path[0]); + int fd = mkstemp(path.data()); if (fd < 0) { perror("Could not create temporary file"); return false; @@ -208,8 +207,7 @@ bool TemporaryDirectory::Init() { } #else path += "bssl_tmp_dir.XXXXXX"; - // TODO(davidben): Use |path.data()| when we require C++17. - if (mkdtemp(&path[0]) == nullptr) { + if (mkdtemp(path.data()) == nullptr) { perror("Could not make temporary directory"); return false; } diff --git a/include/openssl/span.h b/include/openssl/span.h index 9e5ff7cfd9..46c5c6c641 100644 --- a/include/openssl/span.h +++ b/include/openssl/span.h @@ -24,11 +24,8 @@ extern "C++" { #include #include -#include - -#if __cplusplus >= 201703L #include -#endif +#include #if defined(__has_include) #if __has_include() @@ -74,12 +71,10 @@ class SpanBase { // Heuristically test whether C is a container type that can be converted into // a Span by checking for data() and size() member functions. -// -// TODO(davidben): Require C++17 support for std::is_convertible_v, etc. template using EnableIfContainer = std::enable_if_t< - std::is_convertible().data()), T *>::value && - std::is_integral().size())>::value>; + std::is_convertible_v().data()), T *> && + std::is_integral_v().size())>>; } // namespace internal @@ -210,7 +205,6 @@ class Span : private internal::SpanBase { template const size_t Span::npos; -#if __cplusplus >= 201703L template Span(T *, size_t) -> Span; template @@ -220,10 +214,7 @@ template < typename T = std::remove_pointer_t().data())>, typename = internal::EnableIfContainer> Span(C &) -> Span; -#endif -// C++17 callers can instead rely on CTAD and the deduction guides defined -// above. template constexpr Span MakeSpan(T *ptr, size_t size) { return Span(ptr, size); @@ -255,7 +246,6 @@ constexpr Span MakeConstSpan(T (&array)[size]) { return array; } -#if __cplusplus >= 201703L inline Span StringAsBytes(std::string_view s) { return MakeConstSpan(reinterpret_cast(s.data()), s.size()); } @@ -263,7 +253,6 @@ inline Span StringAsBytes(std::string_view s) { inline std::string_view BytesAsStringView(bssl::Span b) { return std::string_view(reinterpret_cast(b.data()), b.size()); } -#endif BSSL_NAMESPACE_END diff --git a/ssl/internal.h b/ssl/internal.h index 7dc522d41e..7c91643b69 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -188,53 +188,6 @@ struct SSL_X509_METHOD; // C++ utilities. -// Fill-ins for various functions in C++17. -// TODO(crbug.com/42290600): Replace these with the standard ones when we -// require C++17. - -template -ForwardIt cxx17_uninitialized_default_construct_n(ForwardIt first, size_t n) { - using T = typename std::iterator_traits::value_type; - while (n > 0) { - new (std::addressof(*first)) T; - first++; - n--; - } - return first; -} - -template -ForwardIt cxx17_uninitialized_value_construct_n(ForwardIt first, size_t n) { - using T = typename std::iterator_traits::value_type; - while (n > 0) { - new (std::addressof(*first)) T(); - first++; - n--; - } - return first; -} - -template -InputIt cxx17_uninitialized_move(InputIt first, InputIt last, OutputIt out) { - using OutputT = typename std::iterator_traits::value_type; - for (; first != last; ++first) { - new (std::addressof(*out)) OutputT(std::move(*first)); - ++out; - } - return out; -} - -template -ForwardIt cxx17_destroy_n(ForwardIt first, size_t n) { - using T = typename std::iterator_traits::value_type; - while (n > 0) { - first->~T(); - first++; - n--; - } - return first; -} - // New behaves like |new| but uses |OPENSSL_malloc| for memory allocation. It // returns nullptr on allocation error. It only implements single-object // allocation and not new T[n]. @@ -318,7 +271,7 @@ class Array { // Reset releases the current contents of the array and takes ownership of the // raw pointer supplied by the caller. void Reset(T *new_data, size_t new_size) { - cxx17_destroy_n(data_, size_); + std::destroy_n(data_, size_); OPENSSL_free(data_); data_ = new_data; size_ = new_size; @@ -341,7 +294,7 @@ class Array { if (!InitUninitialized(new_size)) { return false; } - cxx17_uninitialized_value_construct_n(data_, size_); + std::uninitialized_value_construct_n(data_, size_); return true; } @@ -352,7 +305,7 @@ class Array { if (!InitUninitialized(new_size)) { return false; } - cxx17_uninitialized_default_construct_n(data_, size_); + std::uninitialized_default_construct_n(data_, size_); return true; } @@ -372,7 +325,7 @@ class Array { if (new_size > size_) { abort(); } - cxx17_destroy_n(data_ + new_size, size_ - new_size); + std::destroy_n(data_ + new_size, size_ - new_size); size_ = new_size; } @@ -440,7 +393,7 @@ class Vector { const T *end() const { return data_ + size_; } void clear() { - cxx17_destroy_n(data_, size_); + std::destroy_n(data_, size_); OPENSSL_free(data_); data_ = nullptr; size_ = 0; @@ -499,7 +452,7 @@ class Vector { return false; } size_t new_size = size_; - cxx17_uninitialized_move(begin(), end(), new_data); + std::uninitialized_move(begin(), end(), new_data); clear(); data_ = new_data; size_ = new_size; @@ -543,7 +496,7 @@ class InplaceVector { } InplaceVector &operator=(InplaceVector &&other) { clear(); - cxx17_uninitialized_move(other.begin(), other.end(), data()); + std::uninitialized_move(other.begin(), other.end(), data()); size_ = other.size(); return *this; } @@ -575,7 +528,7 @@ class InplaceVector { // default-constructible. void Shrink(size_t new_size) { BSSL_CHECK(new_size <= size_); - cxx17_destroy_n(data() + new_size, size_ - new_size); + std::destroy_n(data() + new_size, size_ - new_size); size_ = static_cast>(new_size); } @@ -590,7 +543,7 @@ class InplaceVector { if (new_size > capacity()) { return false; } - cxx17_uninitialized_value_construct_n(data() + size_, new_size - size_); + std::uninitialized_value_construct_n(data() + size_, new_size - size_); size_ = static_cast>(new_size); return true; } @@ -606,7 +559,7 @@ class InplaceVector { if (new_size > capacity()) { return false; } - cxx17_uninitialized_default_construct_n(data() + size_, new_size - size_); + std::uninitialized_default_construct_n(data() + size_, new_size - size_); size_ = static_cast>(new_size); return true; }