leveldb: pull upstream C++23 changes

fanquake commented at 2:01 pm on January 30, 2025: member

Cherry-picks two commits from upstream (https://github.com/google/leveldb/commit/302786e211d1f2e6fd260261f642d03a91e5922c, https://github.com/google/leveldb/commit/e829478c6a3a55d8e5c1227e2678dcc18d518609), which remove the usage of std::aligned_storage/std::aligned_union.

Note the first cherry-pick is not clean, because due to Google tooling issues, it accidently contained a revert of the prior two commits. See https://github.com/google/leveldb/pull/1249 for more details.

Also see https://issues.chromium.org/issues/388068052, although note that they reverted the roll to latest leveldb. I’m guessing due to the acidental reversion issue above.

DrahtBot commented at 2:01 pm on January 30, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31766.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	darosior, dergoegge, l0rinc

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

DrahtBot commented at 3:00 pm on January 30, 2025: contributor

🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/36420557428

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

DrahtBot added the label CI failed on Jan 30, 2025

victorvianna commented at 5:48 pm on January 30, 2025: none

FYI the upstream PR landed

fanquake force-pushed on Mar 7, 2025

fanquake renamed this:
~~[WIP] leveldb: pull upstream C++23 changes~~
leveldb: pull upstream C++23 changes
on Mar 7, 2025

fanquake commented at 5:10 pm on March 7, 2025: member

Updated now that the changes have landed upstream in https://github.com/bitcoin-core/leveldb-subtree/pull/47.

fanquake marked this as ready for review on Mar 7, 2025

DrahtBot removed the label CI failed on Mar 7, 2025

Squashed 'src/leveldb/' changes from 04b5790928..4188247086

4188247086 Merge bitcoin-core/leveldb-subtree#47: Fix C++23 compilation errors in leveldb
183e79a495 Fix speculatively some "placement new" issues in leveldb
82c31046ed Fix C++23 compilation errors in leveldb

git-subtree-dir: src/leveldb
git-subtree-split: 41882470862df219f74cdd38354007b91eb98191

a130bbd154

Update leveldb subtree to latest upstream 24fd0235e4

ci: remove -Wno-error=deprecated-declarations from ASAN

This is no-longer needed after the changes to leveldb.

c8fab35617

fanquake force-pushed on Mar 17, 2025

fanquake requested review from dergoegge on Mar 18, 2025

fanquake requested review from darosior on Mar 18, 2025

darosior approved

darosior commented at 0:27 am on March 19, 2025: member

ACK c8fab356171a0e283d5716647e3243c04810ac51 – checked it’s a clean subtree pull from https://github.com/bitcoin-core/leveldb-subtree/tree/bitcoin-fork

dergoegge approved

dergoegge commented at 2:00 pm on March 19, 2025: member

utACK c8fab356171a0e283d5716647e3243c04810ac51

l0rinc commented at 2:22 pm on March 19, 2025: contributor

I have first compared every modified file completely against the latest versions of https://github.com/google/leveldb/blob/main/util/env_posix.cc, https://github.com/google/leveldb/blob/main/util/env_windows.cc and https://github.com/google/leveldb/blob/main/util/no_destructor.h, but realized we’re too far behind.

  0diff --git a/src/leveldb/util/env_posix.cc b/src/leveldb/util/env_posix.cc
  1index 86571059ba..25399f9745 100644
  2--- a/src/leveldb/util/env_posix.cc
  3+++ b/src/leveldb/util/env_posix.cc
  4@@ -4,9 +4,10 @@
  5 
  6 #include <dirent.h>
  7 #include <fcntl.h>
  8-#include <pthread.h>
  9 #include <sys/mman.h>
 10+#ifndef __Fuchsia__
 11 #include <sys/resource.h>
 12+#endif
 13 #include <sys/stat.h>
 14 #include <sys/time.h>
 15 #include <sys/types.h>
 16@@ -42,14 +43,14 @@ namespace {
 17 // Set by EnvPosixTestHelper::SetReadOnlyMMapLimit() and MaxOpenFiles().
 18 int g_open_read_only_file_limit = -1;
 19 
 20-// Up to 4096 mmap regions for 64-bit binaries; none for 32-bit.
 21-constexpr const int kDefaultMmapLimit = (sizeof(void*) >= 8) ? 4096 : 0;
 22+// Up to 1000 mmap regions for 64-bit binaries; none for 32-bit.
 23+constexpr const int kDefaultMmapLimit = (sizeof(void*) >= 8) ? 1000 : 0;
 24 
 25 // Can be set using EnvPosixTestHelper::SetReadOnlyMMapLimit().
 26 int g_mmap_limit = kDefaultMmapLimit;
 27 
 28 // Common flags defined for all posix open operations
 29-#if HAVE_O_CLOEXEC
 30+#if defined(HAVE_O_CLOEXEC)
 31 constexpr const int kOpenBaseFlags = O_CLOEXEC;
 32 #else
 33 constexpr const int kOpenBaseFlags = 0;
 34@@ -72,7 +73,14 @@ Status PosixError(const std::string& context, int error_number) {
 35 class Limiter {
 36  public:
 37   // Limit maximum number of resources to |max_acquires|.
 38-  Limiter(int max_acquires) : acquires_allowed_(max_acquires) {}
 39+  Limiter(int max_acquires)
 40+      :
 41+#if !defined(NDEBUG)
 42+        max_acquires_(max_acquires),
 43+#endif  // !defined(NDEBUG)
 44+        acquires_allowed_(max_acquires) {
 45+    assert(max_acquires >= 0);
 46+  }
 47 
 48   Limiter(const Limiter&) = delete;
 49   Limiter operator=(const Limiter&) = delete;
 50@@ -85,15 +93,35 @@ class Limiter {
 51 
 52     if (old_acquires_allowed > 0) return true;
 53 
 54-    acquires_allowed_.fetch_add(1, std::memory_order_relaxed);
 55+    int pre_increment_acquires_allowed =
 56+        acquires_allowed_.fetch_add(1, std::memory_order_relaxed);
 57+
 58+    // Silence compiler warnings about unused arguments when NDEBUG is defined.
 59+    (void)pre_increment_acquires_allowed;
 60+    // If the check below fails, Release() was called more times than acquire.
 61+    assert(pre_increment_acquires_allowed < max_acquires_);
 62+
 63     return false;
 64   }
 65 
 66   // Release a resource acquired by a previous call to Acquire() that returned
 67   // true.
 68-  void Release() { acquires_allowed_.fetch_add(1, std::memory_order_relaxed); }
 69+  void Release() {
 70+    int old_acquires_allowed =
 71+        acquires_allowed_.fetch_add(1, std::memory_order_relaxed);
 72+
 73+    // Silence compiler warnings about unused arguments when NDEBUG is defined.
 74+    (void)old_acquires_allowed;
 75+    // If the check below fails, Release() was called more times than acquire.
 76+    assert(old_acquires_allowed < max_acquires_);
 77+  }
 78 
 79  private:
 80+#if !defined(NDEBUG)
 81+  // Catches an excessive number of Release() calls.
 82+  const int max_acquires_;
 83+#endif  // !defined(NDEBUG)
 84+
 85   // The number of available resources.
 86   //
 87   // This is a counter and is not tied to the invariants of any other class, so
 88@@ -108,7 +136,7 @@ class Limiter {
 89 class PosixSequentialFile final : public SequentialFile {
 90  public:
 91   PosixSequentialFile(std::string filename, int fd)
 92-      : fd_(fd), filename_(filename) {}
 93+      : fd_(fd), filename_(std::move(filename)) {}
 94   ~PosixSequentialFile() override { close(fd_); }
 95 
 96   Status Read(size_t n, Slice* result, char* scratch) override {
 97@@ -135,8 +163,6 @@ class PosixSequentialFile final : public SequentialFile {
 98     return Status::OK();
 99   }
100 
101-  virtual std::string GetName() const override { return filename_; }
102-
103  private:
104   const int fd_;
105   const std::string filename_;
106@@ -197,8 +223,6 @@ class PosixRandomAccessFile final : public RandomAccessFile {
107     return status;
108   }
109 
110-  virtual std::string GetName() const override { return filename_; }
111-
112  private:
113   const bool has_permanent_fd_;  // If false, the file is opened on every read.
114   const int fd_;                 // -1 if has_permanent_fd_ is false.
115@@ -218,7 +242,7 @@ class PosixMmapReadableFile final : public RandomAccessFile {
116   // over the ownership of the region.
117   //
118   // |mmap_limiter| must outlive this instance. The caller must have already
119-  // aquired the right to use one mmap region, which will be released when this
120+  // acquired the right to use one mmap region, which will be released when this
121   // instance is destroyed.
122   PosixMmapReadableFile(std::string filename, char* mmap_base, size_t length,
123                         Limiter* mmap_limiter)
124@@ -243,8 +267,6 @@ class PosixMmapReadableFile final : public RandomAccessFile {
125     return Status::OK();
126   }
127 
128-  virtual std::string GetName() const override { return filename_; }
129-
130  private:
131   char* const mmap_base_;
132   const size_t length_;
133@@ -325,7 +347,7 @@ class PosixWritableFile final : public WritableFile {
134       return status;
135     }
136 
137-    return SyncFd(fd_, filename_, false);
138+    return SyncFd(fd_, filename_);
139   }
140 
141  private:
142@@ -360,7 +382,7 @@ class PosixWritableFile final : public WritableFile {
143     if (fd < 0) {
144       status = PosixError(dirname_, errno);
145     } else {
146-      status = SyncFd(fd, dirname_, true);
147+      status = SyncFd(fd, dirname_);
148       ::close(fd);
149     }
150     return status;
151@@ -372,7 +394,7 @@ class PosixWritableFile final : public WritableFile {
152   //
153   // The path argument is only used to populate the description string in the
154   // returned Status if an error occurs.
155-  static Status SyncFd(int fd, const std::string& fd_path, bool syncing_dir) {
156+  static Status SyncFd(int fd, const std::string& fd_path) {
157 #if HAVE_FULLFSYNC
158     // On macOS and iOS, fsync() doesn't guarantee durability past power
159     // failures. fcntl(F_FULLFSYNC) is required for that purpose. Some
160@@ -392,11 +414,6 @@ class PosixWritableFile final : public WritableFile {
161     if (sync_success) {
162       return Status::OK();
163     }
164-    // Do not crash if filesystem can't fsync directories
165-    // (see [#10000](/bitcoin-bitcoin/10000/))
166-    if (syncing_dir && errno == EINVAL) {
167-      return Status::OK();
168-    }
169     return PosixError(fd_path, errno);
170   }
171 
172@@ -437,8 +454,6 @@ class PosixWritableFile final : public WritableFile {
173     return Basename(filename).starts_with("MANIFEST");
174   }
175 
176-  virtual std::string GetName() const override { return filename_; }
177-
178   // buf_[0, pos_ - 1] contains data to be written to fd_.
179   char buf_[kWritableFileBufferSize];
180   size_t pos_;
181@@ -600,7 +615,7 @@ class PosixEnv : public Env {
182     return Status::OK();
183   }
184 
185-  Status DeleteFile(const std::string& filename) override {
186+  Status RemoveFile(const std::string& filename) override {
187     if (::unlink(filename.c_str()) != 0) {
188       return PosixError(filename, errno);
189     }
190@@ -614,7 +629,7 @@ class PosixEnv : public Env {
191     return Status::OK();
192   }
193 
194-  Status DeleteDir(const std::string& dirname) override {
195+  Status RemoveDir(const std::string& dirname) override {
196     if (::rmdir(dirname.c_str()) != 0) {
197       return PosixError(dirname, errno);
198     }
199@@ -741,7 +756,7 @@ class PosixEnv : public Env {
200   // Instances are constructed on the thread calling Schedule() and used on the
201   // background thread.
202   //
203-  // This structure is thread-safe beacuse it is immutable.
204+  // This structure is thread-safe because it is immutable.
205   struct BackgroundWorkItem {
206     explicit BackgroundWorkItem(void (*function)(void* arg), void* arg)
207         : function(function), arg(arg) {}
208@@ -770,6 +785,10 @@ int MaxOpenFiles() {
209   if (g_open_read_only_file_limit >= 0) {
210     return g_open_read_only_file_limit;
211   }
212+#ifdef __Fuchsia__
213+  // Fuchsia doesn't implement getrlimit.
214+  g_open_read_only_file_limit = 50;
215+#else
216   struct ::rlimit rlim;
217   if (::getrlimit(RLIMIT_NOFILE, &rlim)) {
218     // getrlimit failed, fallback to hard-coded default.
219@@ -780,6 +799,7 @@ int MaxOpenFiles() {
220     // Allow use of 20% of available file descriptors for read-only files.
221     g_open_read_only_file_limit = rlim.rlim_cur / 5;
222   }
223+#endif
224   return g_open_read_only_file_limit;
225 }
226 
227@@ -906,4 +926,4 @@ Env* Env::Default() {
228   return env_container.env();
229 }
230 
231-}  // namespace leveldb
232+}  // namespace leveldb
233\ No newline at end of file
234diff --git a/src/leveldb/util/env_windows.cc b/src/leveldb/util/env_windows.cc
235index 0a48c3fb52..4b68efdd33 100644
236--- a/src/leveldb/util/env_windows.cc
237+++ b/src/leveldb/util/env_windows.cc
238@@ -33,10 +33,6 @@
239 #include "util/mutexlock.h"
240 #include "util/windows_logger.h"
241 
242-#if defined(DeleteFile)
243-#undef DeleteFile
244-#endif  // defined(DeleteFile)
245-
246 namespace leveldb {
247 
248 namespace {
249@@ -118,7 +114,14 @@ class ScopedHandle {
250 class Limiter {
251  public:
252   // Limit maximum number of resources to |max_acquires|.
253-  Limiter(int max_acquires) : acquires_allowed_(max_acquires) {}
254+  Limiter(int max_acquires)
255+      :
256+#if !defined(NDEBUG)
257+        max_acquires_(max_acquires),
258+#endif  // !defined(NDEBUG)
259+        acquires_allowed_(max_acquires) {
260+    assert(max_acquires >= 0);
261+  }
262 
263   Limiter(const Limiter&) = delete;
264   Limiter operator=(const Limiter&) = delete;
265@@ -137,9 +140,22 @@ class Limiter {
266 
267   // Release a resource acquired by a previous call to Acquire() that returned
268   // true.
269-  void Release() { acquires_allowed_.fetch_add(1, std::memory_order_relaxed); }
270+  void Release() {
271+    int old_acquires_allowed =
272+        acquires_allowed_.fetch_add(1, std::memory_order_relaxed);
273+
274+    // Silence compiler warnings about unused arguments when NDEBUG is defined.
275+    (void)old_acquires_allowed;
276+    // If the check below fails, Release() was called more times than acquire.
277+    assert(old_acquires_allowed < max_acquires_);
278+  }
279 
280  private:
281+#if !defined(NDEBUG)
282+  // Catches an excessive number of Release() calls.
283+  const int max_acquires_;
284+#endif  // !defined(NDEBUG)
285+
286   // The number of available resources.
287   //
288   // This is a counter and is not tied to the invariants of any other class, so
289@@ -177,8 +193,6 @@ class WindowsSequentialFile : public SequentialFile {
290     return Status::OK();
291   }
292 
293-  std::string GetName() const override { return filename_; }
294-
295  private:
296   const ScopedHandle handle_;
297   const std::string filename_;
298@@ -194,7 +208,7 @@ class WindowsRandomAccessFile : public RandomAccessFile {
299   Status Read(uint64_t offset, size_t n, Slice* result,
300               char* scratch) const override {
301     DWORD bytes_read = 0;
302-    OVERLAPPED overlapped = {};
303+    OVERLAPPED overlapped = {0};
304 
305     overlapped.OffsetHigh = static_cast<DWORD>(offset >> 32);
306     overlapped.Offset = static_cast<DWORD>(offset);
307@@ -211,8 +225,6 @@ class WindowsRandomAccessFile : public RandomAccessFile {
308     return Status::OK();
309   }
310 
311-  std::string GetName() const override { return filename_; }
312-
313  private:
314   const ScopedHandle handle_;
315   const std::string filename_;
316@@ -244,8 +256,6 @@ class WindowsMmapReadableFile : public RandomAccessFile {
317     return Status::OK();
318   }
319 
320-  std::string GetName() const override { return filename_; }
321-
322  private:
323   char* const mmap_base_;
324   const size_t length_;
325@@ -315,8 +325,6 @@ class WindowsWritableFile : public WritableFile {
326     return Status::OK();
327   }
328 
329-  std::string GetName() const override { return filename_; }
330-
331  private:
332   Status FlushBuffer() {
333     Status status = WriteUnbuffered(buf_, pos_);
334@@ -387,9 +395,8 @@ class WindowsEnv : public Env {
335     *result = nullptr;
336     DWORD desired_access = GENERIC_READ;
337     DWORD share_mode = FILE_SHARE_READ;
338-    auto wFilename = toUtf16(filename);
339-    ScopedHandle handle = ::CreateFileW(
340-        wFilename.c_str(), desired_access, share_mode,
341+    ScopedHandle handle = ::CreateFileA(
342+        filename.c_str(), desired_access, share_mode,
343         /*lpSecurityAttributes=*/nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL,
344         /*hTemplateFile=*/nullptr);
345     if (!handle.is_valid()) {
346@@ -405,9 +412,8 @@ class WindowsEnv : public Env {
347     *result = nullptr;
348     DWORD desired_access = GENERIC_READ;
349     DWORD share_mode = FILE_SHARE_READ;
350-    auto wFilename = toUtf16(filename);
351     ScopedHandle handle =
352-        ::CreateFileW(wFilename.c_str(), desired_access, share_mode,
353+        ::CreateFileA(filename.c_str(), desired_access, share_mode,
354                       /*lpSecurityAttributes=*/nullptr, OPEN_EXISTING,
355                       FILE_ATTRIBUTE_READONLY,
356                       /*hTemplateFile=*/nullptr);
357@@ -427,7 +433,7 @@ class WindowsEnv : public Env {
358     }
359 
360     ScopedHandle mapping =
361-        ::CreateFileMappingW(handle.get(),
362+        ::CreateFileMappingA(handle.get(),
363                              /*security attributes=*/nullptr, PAGE_READONLY,
364                              /*dwMaximumSizeHigh=*/0,
365                              /*dwMaximumSizeLow=*/0,
366@@ -452,9 +458,8 @@ class WindowsEnv : public Env {
367                          WritableFile** result) override {
368     DWORD desired_access = GENERIC_WRITE;
369     DWORD share_mode = 0;  // Exclusive access.
370-    auto wFilename = toUtf16(filename);
371-    ScopedHandle handle = ::CreateFileW(
372-        wFilename.c_str(), desired_access, share_mode,
373+    ScopedHandle handle = ::CreateFileA(
374+        filename.c_str(), desired_access, share_mode,
375         /*lpSecurityAttributes=*/nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL,
376         /*hTemplateFile=*/nullptr);
377     if (!handle.is_valid()) {
378@@ -470,9 +475,8 @@ class WindowsEnv : public Env {
379                            WritableFile** result) override {
380     DWORD desired_access = FILE_APPEND_DATA;
381     DWORD share_mode = 0;  // Exclusive access.
382-    auto wFilename = toUtf16(filename);
383-    ScopedHandle handle = ::CreateFileW(
384-        wFilename.c_str(), desired_access, share_mode,
385+    ScopedHandle handle = ::CreateFileA(
386+        filename.c_str(), desired_access, share_mode,
387         /*lpSecurityAttributes=*/nullptr, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL,
388         /*hTemplateFile=*/nullptr);
389     if (!handle.is_valid()) {
390@@ -485,16 +489,14 @@ class WindowsEnv : public Env {
391   }
392 
393   bool FileExists(const std::string& filename) override {
394-    auto wFilename = toUtf16(filename);
395-    return GetFileAttributesW(wFilename.c_str()) != INVALID_FILE_ATTRIBUTES;
396+    return GetFileAttributesA(filename.c_str()) != INVALID_FILE_ATTRIBUTES;
397   }
398 
399   Status GetChildren(const std::string& directory_path,
400                      std::vector<std::string>* result) override {
401     const std::string find_pattern = directory_path + "\\*";
402-    WIN32_FIND_DATAW find_data;
403-    auto wFind_pattern = toUtf16(find_pattern);
404-    HANDLE dir_handle = ::FindFirstFileW(wFind_pattern.c_str(), &find_data);
405+    WIN32_FIND_DATAA find_data;
406+    HANDLE dir_handle = ::FindFirstFileA(find_pattern.c_str(), &find_data);
407     if (dir_handle == INVALID_HANDLE_VALUE) {
408       DWORD last_error = ::GetLastError();
409       if (last_error == ERROR_FILE_NOT_FOUND) {
410@@ -506,12 +508,11 @@ class WindowsEnv : public Env {
411       char base_name[_MAX_FNAME];
412       char ext[_MAX_EXT];
413 
414-      auto find_data_filename = toUtf8(find_data.cFileName);
415-      if (!_splitpath_s(find_data_filename.c_str(), nullptr, 0, nullptr, 0,
416-                        base_name, ARRAYSIZE(base_name), ext, ARRAYSIZE(ext))) {
417+      if (!_splitpath_s(find_data.cFileName, nullptr, 0, nullptr, 0, base_name,
418+                        ARRAYSIZE(base_name), ext, ARRAYSIZE(ext))) {
419         result->emplace_back(std::string(base_name) + ext);
420       }
421-    } while (::FindNextFileW(dir_handle, &find_data));
422+    } while (::FindNextFileA(dir_handle, &find_data));
423     DWORD last_error = ::GetLastError();
424     ::FindClose(dir_handle);
425     if (last_error != ERROR_NO_MORE_FILES) {
426@@ -520,25 +521,22 @@ class WindowsEnv : public Env {
427     return Status::OK();
428   }
429 
430-  Status DeleteFile(const std::string& filename) override {
431-    auto wFilename = toUtf16(filename);
432-    if (!::DeleteFileW(wFilename.c_str())) {
433+  Status RemoveFile(const std::string& filename) override {
434+    if (!::DeleteFileA(filename.c_str())) {
435       return WindowsError(filename, ::GetLastError());
436     }
437     return Status::OK();
438   }
439 
440   Status CreateDir(const std::string& dirname) override {
441-    auto wDirname = toUtf16(dirname);
442-    if (!::CreateDirectoryW(wDirname.c_str(), nullptr)) {
443+    if (!::CreateDirectoryA(dirname.c_str(), nullptr)) {
444       return WindowsError(dirname, ::GetLastError());
445     }
446     return Status::OK();
447   }
448 
449-  Status DeleteDir(const std::string& dirname) override {
450-    auto wDirname = toUtf16(dirname);
451-    if (!::RemoveDirectoryW(wDirname.c_str())) {
452+  Status RemoveDir(const std::string& dirname) override {
453+    if (!::RemoveDirectoryA(dirname.c_str())) {
454       return WindowsError(dirname, ::GetLastError());
455     }
456     return Status::OK();
457@@ -546,8 +544,7 @@ class WindowsEnv : public Env {
458 
459   Status GetFileSize(const std::string& filename, uint64_t* size) override {
460     WIN32_FILE_ATTRIBUTE_DATA file_attributes;
461-    auto wFilename = toUtf16(filename);
462-    if (!::GetFileAttributesExW(wFilename.c_str(), GetFileExInfoStandard,
463+    if (!::GetFileAttributesExA(filename.c_str(), GetFileExInfoStandard,
464                                 &file_attributes)) {
465       return WindowsError(filename, ::GetLastError());
466     }
467@@ -561,9 +558,7 @@ class WindowsEnv : public Env {
468   Status RenameFile(const std::string& from, const std::string& to) override {
469     // Try a simple move first. It will only succeed when |to| doesn't already
470     // exist.
471-    auto wFrom = toUtf16(from);
472-    auto wTo = toUtf16(to);
473-    if (::MoveFileW(wFrom.c_str(), wTo.c_str())) {
474+    if (::MoveFileA(from.c_str(), to.c_str())) {
475       return Status::OK();
476     }
477     DWORD move_error = ::GetLastError();
478@@ -572,7 +567,7 @@ class WindowsEnv : public Env {
479     // succeed when |to| does exist. When writing to a network share, we may not
480     // be able to change the ACLs. Ignore ACL errors then
481     // (REPLACEFILE_IGNORE_MERGE_ERRORS).
482-    if (::ReplaceFileW(wTo.c_str(), wFrom.c_str(), /*lpBackupFileName=*/nullptr,
483+    if (::ReplaceFileA(to.c_str(), from.c_str(), /*lpBackupFileName=*/nullptr,
484                        REPLACEFILE_IGNORE_MERGE_ERRORS,
485                        /*lpExclude=*/nullptr, /*lpReserved=*/nullptr)) {
486       return Status::OK();
487@@ -592,9 +587,8 @@ class WindowsEnv : public Env {
488   Status LockFile(const std::string& filename, FileLock** lock) override {
489     *lock = nullptr;
490     Status result;
491-    auto wFilename = toUtf16(filename);
492-    ScopedHandle handle = ::CreateFileW(
493-        wFilename.c_str(), GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ,
494+    ScopedHandle handle = ::CreateFileA(
495+        filename.c_str(), GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ,
496         /*lpSecurityAttributes=*/nullptr, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL,
497         nullptr);
498     if (!handle.is_valid()) {
499@@ -634,11 +628,10 @@ class WindowsEnv : public Env {
500       return Status::OK();
501     }
502 
503-    wchar_t wtmp_path[MAX_PATH];
504-    if (!GetTempPathW(ARRAYSIZE(wtmp_path), wtmp_path)) {
505+    char tmp_path[MAX_PATH];
506+    if (!GetTempPathA(ARRAYSIZE(tmp_path), tmp_path)) {
507       return WindowsError("GetTempPath", ::GetLastError());
508     }
509-    std::string tmp_path = toUtf8(std::wstring(wtmp_path));
510     std::stringstream ss;
511     ss << tmp_path << "leveldbtest-" << std::this_thread::get_id();
512     *result = ss.str();
513@@ -649,8 +642,7 @@ class WindowsEnv : public Env {
514   }
515 
516   Status NewLogger(const std::string& filename, Logger** result) override {
517-    auto wFilename = toUtf16(filename);
518-    std::FILE* fp = _wfopen(wFilename.c_str(), L"w");
519+    std::FILE* fp = std::fopen(filename.c_str(), "wN");
520     if (fp == nullptr) {
521       *result = nullptr;
522       return WindowsError(filename, ::GetLastError());
523@@ -689,7 +681,7 @@ class WindowsEnv : public Env {
524   // Instances are constructed on the thread calling Schedule() and used on the
525   // background thread.
526   //
527-  // This structure is thread-safe beacuse it is immutable.
528+  // This structure is thread-safe because it is immutable.
529   struct BackgroundWorkItem {
530     explicit BackgroundWorkItem(void (*function)(void* arg), void* arg)
531         : function(function), arg(arg) {}
532@@ -706,31 +698,6 @@ class WindowsEnv : public Env {
533       GUARDED_BY(background_work_mutex_);
534 
535   Limiter mmap_limiter_;  // Thread-safe.
536-
537-  // Converts a Windows wide multi-byte UTF-16 string to a UTF-8 string.
538-  // See http://utf8everywhere.org/#windows
539-  std::string toUtf8(const std::wstring& wstr) {
540-    if (wstr.empty()) return std::string();
541-    int size_needed = WideCharToMultiByte(
542-        CP_UTF8, 0, &wstr[0], (int)wstr.size(), NULL, 0, NULL, NULL);
543-    std::string strTo(size_needed, 0);
544-    WideCharToMultiByte(CP_UTF8, 0, &wstr[0], (int)wstr.size(), &strTo[0],
545-                        size_needed, NULL, NULL);
546-    return strTo;
547-  }
548-
549-  // Converts a UTF-8 string to a Windows UTF-16 multi-byte wide character
550-  // string.
551-  // See http://utf8everywhere.org/#windows
552-  std::wstring toUtf16(const std::string& str) {
553-    if (str.empty()) return std::wstring();
554-    int size_needed =
555-        MultiByteToWideChar(CP_UTF8, 0, &str[0], (int)str.size(), NULL, 0);
556-    std::wstring strTo(size_needed, 0);
557-    MultiByteToWideChar(CP_UTF8, 0, &str[0], (int)str.size(), &strTo[0],
558-                        size_needed);
559-    return strTo;
560-  }
561 };
562 
563 // Return the maximum number of concurrent mmaps.
564@@ -849,4 +816,4 @@ Env* Env::Default() {
565   return env_container.env();
566 }
567 
568-}  // namespace leveldb
569+}  // namespace leveldb
570\ No newline at end of file
571diff --git a/src/leveldb/util/no_destructor.h b/src/leveldb/util/no_destructor.h
572index c28a107313..1836d2029c 100644
573--- a/src/leveldb/util/no_destructor.h
574+++ b/src/leveldb/util/no_destructor.h
575@@ -16,35 +16,35 @@ namespace leveldb {
576 // This is intended for use with function-level static variables.
577 template <typename InstanceType>
578 class NoDestructor {
579- public:
580-  template <typename... ConstructorArgTypes>
581-  explicit NoDestructor(ConstructorArgTypes&&... constructor_args) {
582-    static_assert(sizeof(instance_storage_) >= sizeof(InstanceType),
583-                  "instance_storage_ is not large enough to hold the instance");
584-    static_assert(std::is_standard_layout_v<NoDestructor<InstanceType>>);
585-    static_assert(
586-        offsetof(NoDestructor, instance_storage_) % alignof(InstanceType) == 0,
587-        "instance_storage_ does not meet the instance's alignment requirement");
588-    static_assert(
589-        alignof(NoDestructor<InstanceType>) % alignof(InstanceType) == 0,
590-        "instance_storage_ does not meet the instance's alignment requirement");
591-    new (instance_storage_)
592-        InstanceType(std::forward<ConstructorArgTypes>(constructor_args)...);
593-  }
594-
595-  ~NoDestructor() = default;
596-
597-  NoDestructor(const NoDestructor&) = delete;
598-  NoDestructor& operator=(const NoDestructor&) = delete;
599-
600-  InstanceType* get() {
601-    return reinterpret_cast<InstanceType*>(&instance_storage_);
602-  }
603-
604- private:
605-  alignas(InstanceType) char instance_storage_[sizeof(InstanceType)];
606+public:
607+    template <typename... ConstructorArgTypes>
608+    explicit NoDestructor(ConstructorArgTypes&&... constructor_args) {
609+        static_assert(sizeof(instance_storage_) >= sizeof(InstanceType),
610+                      "instance_storage_ is not large enough to hold the instance");
611+        static_assert(std::is_standard_layout_v<NoDestructor<InstanceType>>);
612+        static_assert(
613+            offsetof(NoDestructor, instance_storage_) % alignof(InstanceType) == 0,
614+            "instance_storage_ does not meet the instance's alignment requirement");
615+        static_assert(
616+            alignof(NoDestructor<InstanceType>) % alignof(InstanceType) == 0,
617+            "instance_storage_ does not meet the instance's alignment requirement");
618+        new (instance_storage_)
619+            InstanceType(std::forward<ConstructorArgTypes>(constructor_args)...);
620+    }
621+
622+    ~NoDestructor() = default;
623+
624+    NoDestructor(const NoDestructor&) = delete;
625+    NoDestructor& operator=(const NoDestructor&) = delete;
626+
627+    InstanceType* get() {
628+        return reinterpret_cast<InstanceType*>(&instance_storage_);
629+    }
630+
631+private:
632+    alignas(InstanceType) char instance_storage_[sizeof(InstanceType)];
633 };
634 
635 }  // namespace leveldb
636 
637-#endif  // STORAGE_LEVELDB_UTIL_NO_DESTRUCTOR_H_
638+#endif  // STORAGE_LEVELDB_UTIL_NO_DESTRUCTOR_H_
639\ No newline at end of file

After that I have compared the modified codeparts only against latest and they match exactly.

Build failure is known and unrelated. Build and tests are passing locally for a rebased version against latest master.

ACK c8fab356171a0e283d5716647e3243c04810ac51

edit: running a full reindex-chainstate for a rebased version to validate the change

fanquake merged this on Mar 20, 2025

fanquake closed this on Mar 20, 2025

fanquake deleted the branch on Mar 20, 2025

l0rinc commented at 9:13 am on March 20, 2025: contributor

reindex-chainstate finished successfully

 0COMMITS="5e934ebc82ca239cd98eed1689f7cdc2c672a9ae"; \
 1STOP_HEIGHT=888000; DBCACHE=5000; \
 2C_COMPILER=gcc; CXX_COMPILER=g++; \
 3git fetch --all -q && (for c in $COMMITS; do git log -1 --oneline $c || exit 1; done) && \
 4hyperfine \
 5  --export-json "/mnt/my_storage/rdx-${COMMITS// /-}-${STOP_HEIGHT}-${DBCACHE}-${C_COMPILER}.json" \
 6  --runs 1 \
 7  --parameter-list COMMIT ${COMMITS// /,} \
 8  --prepare "killall bitcoind || true; rm -f /mnt/my_storage/BitcoinData/debug.log; git checkout {COMMIT}; git clean -fxd; git reset --hard; cmake -B build -DCMAKE_BUILD_TYPE=Release -DENABLE_WALLET=OFF -DCMAKE_C_COMPILER=$C_COMPILER -DCMAKE_CXX_COMPILER=$CXX_COMPILER && cmake --build build -j$(nproc) --target bitcoind && ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=$STOP_HEIGHT -dbcache=10000 -printtoconsole=0 || true" \
 9  --cleanup "cp /mnt/my_storage/BitcoinData/debug.log /mnt/my_storage/logs/debug-{COMMIT}-$(date +%s).log || true" \
10  "COMPILER=$C_COMPILER COMMIT={COMMIT} ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=$STOP_HEIGHT -dbcache=$DBCACHE -blocksonly -reindex-chainstate -connect=0 -printtoconsole=0"
11
125e934ebc82 (l0rinc/detached179) ci: remove -Wno-error=deprecated-declarations from ASAN
13Benchmark 1: COMPILER=gcc COMMIT=5e934ebc82ca239cd98eed1689f7cdc2c672a9ae ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=888000 -dbcache=5000 -blocksonly -reindex-chainstate -connect=0 -printtoconsole=0
14  Time (abs ≡):        24720.986 s               [User: 22604.063 s, System: 1067.193 s]

leveldb: pull upstream C++23 changes #31766

Code Coverage & Benchmarks

Reviews

Conflicts