Repository navigation
Conversation
Use the value type for the alignment mask to prevent truncation of 64-bit values on 32-bit targets.
Use uint64_t instead of size_t for raw exception addresses, since size_t is only as wide as the host's address space. This avoids truncating 64-bit addresses when processing 64-bit minidumps on 32-bit hosts.
illwieckz
changed the title
Member
|
|
slipher
reviewed
Oct 16, 2026
| static T AlignUp(T offset, size_t alignment) { | ||
| assert(alignment > 0 && ((alignment - 1) & alignment) == 0); | ||
| return (offset + (alignment - 1)) & ~(alignment - 1); | ||
| const T mask = static_cast<T>(alignment - 1); |
Member
There was a problem hiding this comment.
This change doesn't change the semantics of the code at all. It just factors out a common subexpression without changing its type.
If there were really an issue due to passing in an alignment larger than 2^32, you'd have to change the type of the function argument. But a 32-bit process can't allocate more than 2^32 so that doesn't makes sense and can't happen.
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
2 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.
This is very minor and similar to:
Except #46 only focused on what is built in the Dæmon engine.
This makes sure 64-bit pointers and alignments are 64-bits explicitly and don't rely on luck.
While running 32-bit code in virtual machine game code can still find usefulness in possible future distributed product (for example if one day breakpad supports Wasm or LFI), running the minidump processor as 32-bit is far less needed.
What's precious in this is that not only 32-bit hosts are less forgiving on lazy size assumptions and can be useful to enforce strictness, it happens that in the special case of Breakpad this has been the only reliable way for me to track done that thread problem:
So I think I'll keep the
linux-i686platform in the CI to help catch the reintroduction of such hard-to-reproduce bugs, with the extra benefit of lazy pointer or alignment assumptions like those ones being caught.In the end, these patches are required to improve reliability in testing #48 being fixed or not, while #48 can affect every platform.
None of this broke any other platform, it has been tested in CI and outside CI on:
This is picked from: