Handle GetVarint32Ptr failure more gracefully#1306
Closed
evanstade wants to merge 11 commits intogoogle:mainfrom
Closed
Handle GetVarint32Ptr failure more gracefully#1306evanstade wants to merge 11 commits intogoogle:mainfrom
evanstade wants to merge 11 commits intogoogle:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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
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)
Remove usages of std::aligned_storage, which is deprecated. More details about the replacement in https://crbug.com/388068052. PiperOrigin-RevId: 713346733
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
These changes 1) 2cc36eb - "[jumbo] Add begin()/end() to Slice." 2) 578eeb7 - "Fix invalid pointer arithmetic in Hash (google#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.
PiperOrigin-RevId: 866841934
PiperOrigin-RevId: 866848697
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
* 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
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
evanstade
force-pushed
the
init_slice_length
branch
from
f4d984f to
2033033
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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