From 8f0ba4b30ee5ea87a507fdae4e82d2c67382afcd Mon Sep 17 00:00:00 2001 From: Peter Kasting Date: Thu, 25 Jul 2024 09:39:54 -0700 Subject: [PATCH 01/11] [jumbo] Add begin()/end() to Slice. This allows this type to meet the requirements of e.g. std::ranges::range, which is necessary for it to work with the std::span range constructor, or the "non-legacy" constructor for Chromium's base::span. Bug: none --- include/leveldb/slice.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/leveldb/slice.h b/include/leveldb/slice.h index 37cb82178d..e97223a743 100644 --- a/include/leveldb/slice.h +++ b/include/leveldb/slice.h @@ -51,6 +51,9 @@ class LEVELDB_EXPORT Slice { // Return true iff the length of the referenced data is zero bool empty() const { return size_ == 0; } + const char* begin() const { return data(); } + const char* end() const { return data() + size(); } + // Return the ith byte in the referenced data. // REQUIRES: n < size() char operator[](size_t n) const { From f7e87426aae0b9d80d0be1131df002d3bc78261f Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 2 Jan 2025 14:02:29 -0500 Subject: [PATCH 02/11] Fix invalid pointer arithmetic in Hash (#1222) It is UB to exceed the bounds of the buffer when doing pointer arithemetic. That means the following is not a valid bounds check: if (start + 4 <= limit) Because if we were at the end of the buffer, we wouldn't be allowed to add 4 anyway. Instead, this must be written as: if (limit - start >= 4) Basic forms of this issue are flagged by UBSan. If building with -fsanitize=undefined, the following test trips an error: [ RUN ] HASH.SignedUnsignedIssue .../leveldb/util/hash.cc:30:15: runtime error: applying non-zero offset 4 to null pointer SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/local/google/home/davidben/leveldb/util/hash.cc:30:15 in [ OK ] HASH.SignedUnsignedIssue (1 ms) --- util/hash.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/hash.cc b/util/hash.cc index 8122fa834d..fa252c72e1 100644 --- a/util/hash.cc +++ b/util/hash.cc @@ -27,7 +27,7 @@ uint32_t Hash(const char* data, size_t n, uint32_t seed) { uint32_t h = seed ^ (n * m); // Pick up four bytes at a time - while (data + 4 <= limit) { + while (limit - data >= 4) { uint32_t w = DecodeFixed32(data); data += 4; h += w; From 2d33d0a8bbaf0638a438cf01874f6404eac7a761 Mon Sep 17 00:00:00 2001 From: leveldb Team Date: Wed, 8 Jan 2025 18:54:39 +0000 Subject: [PATCH 03/11] Fix C++23 compilation errors in leveldb Remove usages of std::aligned_storage, which is deprecated. More details about the replacement in https://crbug.com/388068052. PiperOrigin-RevId: 713346733 --- include/leveldb/slice.h | 3 --- util/env_posix.cc | 9 ++++++--- util/env_windows.cc | 9 ++++++--- util/hash.cc | 2 +- util/no_destructor.h | 10 +++++++--- 5 files changed, 20 insertions(+), 13 deletions(-) diff --git a/include/leveldb/slice.h b/include/leveldb/slice.h index e97223a743..37cb82178d 100644 --- a/include/leveldb/slice.h +++ b/include/leveldb/slice.h @@ -51,9 +51,6 @@ class LEVELDB_EXPORT Slice { // Return true iff the length of the referenced data is zero bool empty() const { return size_ == 0; } - const char* begin() const { return data(); } - const char* end() const { return data() + size(); } - // Return the ith byte in the referenced data. // REQUIRES: n < size() char operator[](size_t n) const { diff --git a/util/env_posix.cc b/util/env_posix.cc index ffd06c4031..57c19ec70b 100644 --- a/util/env_posix.cc +++ b/util/env_posix.cc @@ -874,7 +874,11 @@ class SingletonEnv { #endif // !defined(NDEBUG) static_assert(sizeof(env_storage_) >= sizeof(EnvType), "env_storage_ will not fit the Env"); - static_assert(alignof(decltype(env_storage_)) >= alignof(EnvType), + static_assert(std::is_standard_layout_v>); + static_assert( + offsetof(SingletonEnv, env_storage_) % alignof(EnvType) == 0, + "env_storage_ does not meet the Env's alignment needs"); + static_assert(alignof(SingletonEnv) % alignof(EnvType) == 0, "env_storage_ does not meet the Env's alignment needs"); new (&env_storage_) EnvType(); } @@ -892,8 +896,7 @@ class SingletonEnv { } private: - typename std::aligned_storage::type - env_storage_; + alignas(EnvType) char env_storage_[sizeof(EnvType)]; #if !defined(NDEBUG) static std::atomic env_initialized_; #endif // !defined(NDEBUG) diff --git a/util/env_windows.cc b/util/env_windows.cc index 1c74b02a7f..e0a19cc5eb 100644 --- a/util/env_windows.cc +++ b/util/env_windows.cc @@ -769,7 +769,11 @@ class SingletonEnv { #endif // !defined(NDEBUG) static_assert(sizeof(env_storage_) >= sizeof(EnvType), "env_storage_ will not fit the Env"); - static_assert(alignof(decltype(env_storage_)) >= alignof(EnvType), + static_assert(std::is_standard_layout_v>); + static_assert( + offsetof(SingletonEnv, env_storage_) % alignof(EnvType) == 0, + "env_storage_ does not meet the Env's alignment needs"); + static_assert(alignof(SingletonEnv) % alignof(EnvType) == 0, "env_storage_ does not meet the Env's alignment needs"); new (&env_storage_) EnvType(); } @@ -787,8 +791,7 @@ class SingletonEnv { } private: - typename std::aligned_storage::type - env_storage_; + alignas(EnvType) char env_storage_[sizeof(EnvType)]; #if !defined(NDEBUG) static std::atomic env_initialized_; #endif // !defined(NDEBUG) diff --git a/util/hash.cc b/util/hash.cc index fa252c72e1..8122fa834d 100644 --- a/util/hash.cc +++ b/util/hash.cc @@ -27,7 +27,7 @@ uint32_t Hash(const char* data, size_t n, uint32_t seed) { uint32_t h = seed ^ (n * m); // Pick up four bytes at a time - while (limit - data >= 4) { + while (data + 4 <= limit) { uint32_t w = DecodeFixed32(data); data += 4; h += w; diff --git a/util/no_destructor.h b/util/no_destructor.h index a0d3b8703d..08ce6a40a9 100644 --- a/util/no_destructor.h +++ b/util/no_destructor.h @@ -5,6 +5,7 @@ #ifndef STORAGE_LEVELDB_UTIL_NO_DESTRUCTOR_H_ #define STORAGE_LEVELDB_UTIL_NO_DESTRUCTOR_H_ +#include #include #include @@ -20,8 +21,12 @@ class NoDestructor { explicit NoDestructor(ConstructorArgTypes&&... constructor_args) { static_assert(sizeof(instance_storage_) >= sizeof(InstanceType), "instance_storage_ is not large enough to hold the instance"); + static_assert(std::is_standard_layout_v>); static_assert( - alignof(decltype(instance_storage_)) >= alignof(InstanceType), + offsetof(NoDestructor, instance_storage_) % alignof(InstanceType) == 0, + "instance_storage_ does not meet the instance's alignment requirement"); + static_assert( + alignof(NoDestructor) % alignof(InstanceType) == 0, "instance_storage_ does not meet the instance's alignment requirement"); new (&instance_storage_) InstanceType(std::forward(constructor_args)...); @@ -37,8 +42,7 @@ class NoDestructor { } private: - typename std::aligned_storage::type instance_storage_; + alignas(InstanceType) char instance_storage_[sizeof(InstanceType)]; }; } // namespace leveldb From 68d51b7d1d47b5a4301df08b59b9a299717434b9 Mon Sep 17 00:00:00 2001 From: leveldb Team Date: Tue, 21 Jan 2025 16:19:03 +0000 Subject: [PATCH 04/11] Fix speculatively some "placement new" issues in leveldb cl/713346733 changed the type of some variables to pointers, but didn't adjust the placement new statements. From pkasting@: "I suspect your code is wrong and will crash. An array is a pointer, so taking its address also gives a pointer, which is why it compiles; but the value of that pointer is different. You're no longer providing the address of the storage, but rather the address of the array pointer." PiperOrigin-RevId: 717926210 --- util/env_posix.cc | 2 +- util/env_windows.cc | 2 +- util/no_destructor.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/util/env_posix.cc b/util/env_posix.cc index 57c19ec70b..c2490322e5 100644 --- a/util/env_posix.cc +++ b/util/env_posix.cc @@ -880,7 +880,7 @@ class SingletonEnv { "env_storage_ does not meet the Env's alignment needs"); static_assert(alignof(SingletonEnv) % alignof(EnvType) == 0, "env_storage_ does not meet the Env's alignment needs"); - new (&env_storage_) EnvType(); + new (env_storage_) EnvType(); } ~SingletonEnv() = default; diff --git a/util/env_windows.cc b/util/env_windows.cc index e0a19cc5eb..ae5149a505 100644 --- a/util/env_windows.cc +++ b/util/env_windows.cc @@ -775,7 +775,7 @@ class SingletonEnv { "env_storage_ does not meet the Env's alignment needs"); static_assert(alignof(SingletonEnv) % alignof(EnvType) == 0, "env_storage_ does not meet the Env's alignment needs"); - new (&env_storage_) EnvType(); + new (env_storage_) EnvType(); } ~SingletonEnv() = default; diff --git a/util/no_destructor.h b/util/no_destructor.h index 08ce6a40a9..c28a107313 100644 --- a/util/no_destructor.h +++ b/util/no_destructor.h @@ -28,7 +28,7 @@ class NoDestructor { static_assert( alignof(NoDestructor) % alignof(InstanceType) == 0, "instance_storage_ does not meet the instance's alignment requirement"); - new (&instance_storage_) + new (instance_storage_) InstanceType(std::forward(constructor_args)...); } From 055626617dbb134fed17a0c7915954abecf0cd28 Mon Sep 17 00:00:00 2001 From: Victor Hugo Vianna Silva Date: Wed, 29 Jan 2025 17:09:35 +0000 Subject: [PATCH 05/11] Reland changes accidentally reverted in 302786e These changes 1) 2cc36eb - "[jumbo] Add begin()/end() to Slice." 2) 578eeb7 - "Fix invalid pointer arithmetic in Hash (#1222)" were committed in the public repository but never got imported to the internal Google repository. Later, cl/713346733 landed in the internal repo. When tooling published the internal change as 302786e ("Fix C++23 compilation errors in leveldb"), it accidentally reverted commits (1) and (2). This change re-commits a bundled version of (1) and (2) in the public repo. This will then be imported to the private repo, leaving the 2 in sync. --- include/leveldb/slice.h | 3 +++ util/hash.cc | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/include/leveldb/slice.h b/include/leveldb/slice.h index 37cb82178d..e97223a743 100644 --- a/include/leveldb/slice.h +++ b/include/leveldb/slice.h @@ -51,6 +51,9 @@ class LEVELDB_EXPORT Slice { // Return true iff the length of the referenced data is zero bool empty() const { return size_ == 0; } + const char* begin() const { return data(); } + const char* end() const { return data() + size(); } + // Return the ith byte in the referenced data. // REQUIRES: n < size() char operator[](size_t n) const { diff --git a/util/hash.cc b/util/hash.cc index 8122fa834d..fa252c72e1 100644 --- a/util/hash.cc +++ b/util/hash.cc @@ -27,7 +27,7 @@ uint32_t Hash(const char* data, size_t n, uint32_t seed) { uint32_t h = seed ^ (n * m); // Pick up four bytes at a time - while (data + 4 <= limit) { + while (limit - data >= 4) { uint32_t w = DecodeFixed32(data); data += 4; h += w; From 503e9840159b331c36383a2d6fb48a3c60cd56f9 Mon Sep 17 00:00:00 2001 From: leveldb Team Date: Sat, 7 Feb 2026 11:29:03 +0000 Subject: [PATCH 06/11] Automated Code Change PiperOrigin-RevId: 866841934 --- db/version_set.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/db/version_set.cc b/db/version_set.cc index 4e37bf90e9..b84e7da3d5 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -815,7 +815,7 @@ Status VersionSet::LogAndApply(VersionEdit* edit, port::Mutex* mu) { // Unlock during expensive MANIFEST log write { - mu->Unlock(); + mu->unlock(); // Write new record to MANIFEST log if (s.ok()) { @@ -836,7 +836,7 @@ Status VersionSet::LogAndApply(VersionEdit* edit, port::Mutex* mu) { s = SetCurrentFile(env_, dbname_, manifest_file_number_); } - mu->Lock(); + mu->lock(); } // Install the new version From 37df282f1b9f8a831e6031a649c19c99736460f6 Mon Sep 17 00:00:00 2001 From: leveldb Team Date: Sat, 7 Feb 2026 11:55:50 +0000 Subject: [PATCH 07/11] Internal change. PiperOrigin-RevId: 866848697 --- db/version_set.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/db/version_set.cc b/db/version_set.cc index b84e7da3d5..4e37bf90e9 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -815,7 +815,7 @@ Status VersionSet::LogAndApply(VersionEdit* edit, port::Mutex* mu) { // Unlock during expensive MANIFEST log write { - mu->unlock(); + mu->Unlock(); // Write new record to MANIFEST log if (s.ok()) { @@ -836,7 +836,7 @@ Status VersionSet::LogAndApply(VersionEdit* edit, port::Mutex* mu) { s = SetCurrentFile(env_, dbname_, manifest_file_number_); } - mu->lock(); + mu->Lock(); } // Install the new version From 5f71871eeffefb9bb597794a81fa93557625e039 Mon Sep 17 00:00:00 2001 From: leveldb Team Date: Fri, 6 Mar 2026 21:29:47 +0000 Subject: [PATCH 08/11] Fix TEST_CompactMemTable deadlock This CL ensures that CompactRange doesn't get stuck on waiting on background_work_finished_signal_ during shutdown. While background_work_finished_signal_ may be in fact signaled, CompactMemTable() will never run, not giving opportunity for workers blocked in TEST_CompactMemTable to exit the loop. We suspect this contributes to b/370844779; while the hang was mitigated by limiting concurrency to 1 worker (which avoids filling the thread pool with blocked workers), this issue can still happen and will block 1 worker during shutdown. PiperOrigin-RevId: 879794057 --- db/db_impl.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index f96d245583..ce4ef8276a 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -646,7 +646,8 @@ Status DBImpl::TEST_CompactMemTable() { if (s.ok()) { // Wait until the compaction completes MutexLock l(&mutex_); - while (imm_ != nullptr && bg_error_.ok()) { + while (imm_ != nullptr && bg_error_.ok() && + !shutting_down_.load(std::memory_order_acquire)) { background_work_finished_signal_.Wait(); } if (imm_ != nullptr) { From eb301058645c9cfdf619823cf69a325bf686eeda Mon Sep 17 00:00:00 2001 From: Victor Costan Date: Wed, 11 Mar 2026 01:30:32 +0000 Subject: [PATCH 09/11] Effectively, this change * bumps the minimum CMake version to 3.22 * bumps the minimum C++ version to C++17 This aligns with the Google C++ support policy at https://github.com/google/oss-policies-info/blob/main/foundational-cxx-support-matrix.md PiperOrigin-RevId: 881717521 --- CMakeLists.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index fda9e01bbb..ce8f5878d7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2,7 +2,7 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. See the AUTHORS file for names of contributors. -cmake_minimum_required(VERSION 3.9) +cmake_minimum_required(VERSION 3.22) # Keep the version below in sync with the one in db.h project(leveldb VERSION 1.23.0 LANGUAGES C CXX) @@ -16,8 +16,8 @@ endif(NOT CMAKE_C_STANDARD) # C++ standard can be overridden when this is used as a sub-project. if(NOT CMAKE_CXX_STANDARD) - # This project requires C++11. - set(CMAKE_CXX_STANDARD 11) + # This project requires C++17. + set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_EXTENSIONS OFF) endif(NOT CMAKE_CXX_STANDARD) From 544a217c95fce78413c25f4f3293ecb32cc9d190 Mon Sep 17 00:00:00 2001 From: Victor Costan Date: Fri, 6 Mar 2026 17:49:26 -0800 Subject: [PATCH 10/11] Bump third_party/ dependencies. --- third_party/benchmark | 2 +- third_party/googletest | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/third_party/benchmark b/third_party/benchmark index f7547e29cc..1a54956777 160000 --- a/third_party/benchmark +++ b/third_party/benchmark @@ -1 +1 @@ -Subproject commit f7547e29ccaed7b64ef4f7495ecfff1c9f6f3d03 +Subproject commit 1a54956777ba672764db09a51960056ea042af7e diff --git a/third_party/googletest b/third_party/googletest index 662fe38e44..a35bc7693c 160000 --- a/third_party/googletest +++ b/third_party/googletest @@ -1 +1 @@ -Subproject commit 662fe38e44900c007eccb65a5d2ea19df7bd520e +Subproject commit a35bc7693c117a048152beeb34f6aac354b9423f From 2033033aca38e5f48bdb0a843e4e047e5c08f348 Mon Sep 17 00:00:00 2001 From: Evan Stade Date: Thu, 19 Mar 2026 23:06:27 +0000 Subject: [PATCH 11/11] Handle GetVarint32Ptr failure more gracefully In particular, don't create a Slice with a null pointer and uninitialized (potentially non-zero) length, which might be the cause of this Chrome crash: https://crbug.com/493304613 --- db/memtable.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/db/memtable.cc b/db/memtable.cc index 4f09340e0e..33a99288c8 100644 --- a/db/memtable.cc +++ b/db/memtable.cc @@ -12,7 +12,7 @@ namespace leveldb { static Slice GetLengthPrefixedSlice(const char* data) { - uint32_t len; + uint32_t len = 0u; const char* p = data; p = GetVarint32Ptr(p, p + 5, &len); // +5: we assume "p" is not corrupted return Slice(p, len); @@ -114,8 +114,11 @@ bool MemTable::Get(const LookupKey& key, std::string* value, Status* s) { // sequence number since the Seek() call above should have skipped // all entries with overly large sequence numbers. const char* entry = iter.key(); - uint32_t key_length; + uint32_t key_length = 0u; const char* key_ptr = GetVarint32Ptr(entry, entry + 5, &key_length); + if (!key_ptr) { + return false; + } if (comparator_.comparator.user_comparator()->Compare( Slice(key_ptr, key_length - 8), key.user_key()) == 0) { // Correct user key