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.
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
Remove usages of std::aligned_storage, which is deprecated.
More details about the replacement in https://crbug.com/388068052.
PiperOrigin-RevId: 713346733
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)
As allowed by the documentation for Env::Schedule(), ChromiumEnv may
execute functions on multiple threads and guarantees no sequencing.
EnvTest.RunMany assumed that functions ran in order, is the case for the
stock PosixEnv and WindowsEnv implementations. This change updates the
test to not assume sequential execution.
There are a couple differences between ChromiumEnv and
PosixEnv/WindowsEnv which cause test failures that are fixed (or at
least patched over) in this change:
* NewSequentialFile() and NewRandomAccessFile() return Status::IOError
rather than Status::NotFound when a file is not found, due to
https://crbug.com/760362. This means a few tests need to expect a
different error result.
* GetChildren() never returns the '.' or '..' entries.
* As allowed by the documentation for Env::Schedule(), ChromiumEnv may
execute functions on multiple threads and guarantees no sequencing.
EnvTest.RunMany assumed that functions ran in order. The test has been
updated.
This gives some flexibility to embedders.
Currently, embedders have to build a binary for each test file.
After this CL, embedders can still choose to have a binary for each test
file, by linking each test file with a googletest target that includes
main() (usually "gtest_main"). Embedders can also choose to build a
single binary for almost all test files, and link with a googletest
target that includes main(). The latter is more convenient for projects
that have very few test binaries, like Chromium.
PiperOrigin-RevId: 419470798
Clang 10 includes the optimizations described in
https://bugs.llvm.org/show_bug.cgi?id=41761. This means that the
platform-independent implementations of {Decode,Encode}Fixed{32,64}()
compile to one instruction on the most recent Clang and GCC.
PiperOrigin-RevId: 306330166
leveldb::Env::DeleteFile was replaced with leveldb::Env::RemoveFile in
all tests. This allows us to remove workarounds for windows.h #defining
DeleteFile.
PiperOrigin-RevId: 289121105
The "DeleteFile" method name causes pain for Windows developers, because
<windows.h> #defines a DeleteFile macro to DeleteFileW or DeleteFileA.
Current code uses workarounds, like #undefining DeleteFile everywhere an
Env is declared, implemented, or used.
This CL removes the need for workarounds by renaming Env::DeleteFile to
Env::RemoveFile. For consistency, Env::DeleteDir is also renamed to
Env::RemoveDir. A few internal methods are also renamed for consistency.
Software that supports Windows is expected to migrate any Env
implementations and usage to Remove{File,Dir}, and never use the name
Env::Delete{File,Dir} in its code.
The renaming is done in a backwards-compatible way, at the risk of
making it slightly more difficult to build a new correct Env
implementation. The backwards compatibility is achieved using the
following hacks:
1) Env::Remove{File,Dir} methods are added, with a default
implementation that calls into Env::Delete{File,Dir}. This makes old
Env implementations compatible with code that calls into the updated
API.
2) The Env::Delete{File,Dir} methods are no longer pure virtuals.
Instead, they gain a default implementation that calls into
Env::Remove{File,Dir}. This makes updated Env implementations
compatible with code that calls into the old API.
The cost of this approach is that it's possible to write an Env without
overriding either Rename{File,Dir} or Delete{File,Dir}, without getting
a compiler warning. However, attempting to run the test suite will
immediately fail with an infinite call stack ending in
{Remove,Delete}{File,Dir}, making developers aware of the problem.
PiperOrigin-RevId: 288710907
The previous attempt of having EnvPosix use O_CLOEXEC (close-on-exec()) when opening file descriptors added tests that relied on procfs, which is Linux-specific. These tests failed on macOS. Unfortunately, the test failures were not caught due to a (since fixed) error in our Travis CI configuration.
This CL re-structures the tests to only rely on POSIX features. Since there is no POSIX-compliant way to get a file name/path out of a file descriptor, this CL breaks up the O_CLOEXEC test into multiple tests, where each Env method that creates an FD gets its own test. This is intended to make it easier to find and fix errors in Env implementations.
This CL also fixes the implementation of NewLogger() to use O_CLOEXEC on macOS. The current implementation passes "we" to fopen(), but the macOS standard C library does not implement the "e" flag yet.
PiperOrigin-RevId: 247088953
