Skip to content

Commit a2ab7cc

Browse files
mralephCommit Bot
authored and
Commit Bot
committed
[vm] Avoid UB in FinalizeHash(hash)
FinalizeHash(hash) was trying to avoid UB in expression 1 << 32 by casting 1 to uintptr_t. This type however is not wide enough on 32-bit platforms. Instead just use explicit comparison hashbits < kBitsPerInt32 to avoid overflow in left shift. This bug went unnoticed for a while because it the only place where we call FinalizeHash(hash) is in the snapshot profile writer code and it only triggers when gen_snapshot is a 32-bit binary - which is only true on Windows, as Mac and Linux seem to use simarm_x64 configuration instead. This UB was explicitly affecting the code behavior because C++ compiler would either optimize out or change behavior of any code that consumed value produced by FinalizeHash(hash). Fixes flutter/flutter#97764 TEST=vm/cc/DirectChainedHashMap Change-Id: I39f2b09e7516c875b765e5a065d1c1331f89fa33 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/250741 Commit-Queue: Slava Egorov <[email protected]> Reviewed-by: Daco Harkes <[email protected]>
1 parent 1c7a7d0 commit a2ab7cc

File tree

2 files changed

+6
-5
lines changed

2 files changed

+6
-5
lines changed

runtime/vm/hash.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,9 @@ inline uint32_t FinalizeHash(uint32_t hash, intptr_t hashbits = kBitsPerInt32) {
2020
hash += hash << 3;
2121
hash ^= hash >> 11; // Logical shift, unsigned hash.
2222
hash += hash << 15;
23-
// FinalizeHash gets called with values for hashbits that are bigger than 31
24-
// (like kBitsPerWord - 1). Therefore we are careful to use a type
25-
// (uintptr_t) big enough to avoid undefined behavior with the left shift.
26-
hash &= (static_cast<uintptr_t>(1) << hashbits) - 1;
23+
if (hashbits < kBitsPerInt32) {
24+
hash &= (static_cast<uint32_t>(1) << hashbits) - 1;
25+
}
2726
return (hash == 0) ? 1 : hash;
2827
}
2928

runtime/vm/hash_map_test.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ namespace dart {
1111
class TestValue {
1212
public:
1313
explicit TestValue(intptr_t x) : x_(x) {}
14-
uword Hash() const { return x_ & 1; }
14+
// FinalizeHash is used here to provide coverage for FinalizeHash(...)
15+
// function.
16+
uword Hash() const { return FinalizeHash(static_cast<uint32_t>(x_) & 1); }
1517
bool Equals(const TestValue& other) { return x_ == other.x_; }
1618

1719
private:

0 commit comments

Comments
 (0)