Skip to content

Commit 60b5e1f

Browse files
goldvitalycopybara-github
authored andcommitted
Fix overflow of kSeedMask on 32 bits platform in generate_new_seed.
`data_ = (data_ & ~kSeedMask) | (seed & kSeedMask);` was clearing top 32 bits. Somehow that was happening only in ASAN 32 bits. On non-asan builds the compiler was "optimizing" to be correct. The bug was reported by chromium: https://g-issues.chromium.org/issues/402910364. PiperOrigin-RevId: 736994877 Change-Id: I13e9bf519b5ff091aee6fa46b567b436c3d5c6ec
1 parent 6e6dee6 commit 60b5e1f

File tree

2 files changed

+42
-19
lines changed

2 files changed

+42
-19
lines changed

absl/container/internal/raw_hash_set.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -520,8 +520,8 @@ class HashtableSize {
520520
static constexpr size_t kSizeShift = 64 - kSizeBitCount;
521521
static constexpr uint64_t kSizeOneNoMetadata = uint64_t{1} << kSizeShift;
522522
static constexpr uint64_t kMetadataMask = kSizeOneNoMetadata - 1;
523-
static constexpr size_t kSeedMask =
524-
(size_t{1} << PerTableSeed::kBitCount) - 1;
523+
static constexpr uint64_t kSeedMask =
524+
(uint64_t{1} << PerTableSeed::kBitCount) - 1;
525525
// The next bit after the seed.
526526
static constexpr uint64_t kHasInfozMask = kSeedMask + 1;
527527
uint64_t data_;

absl/container/internal/raw_hash_set_test.cc

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -915,6 +915,8 @@ constexpr size_t kNonSooSize = sizeof(HeapOrSoo) + 8;
915915
using NonSooIntTableSlotType = SizedValue<kNonSooSize>;
916916
static_assert(sizeof(NonSooIntTableSlotType) >= kNonSooSize, "too small");
917917
using NonSooIntTable = ValueTable<NonSooIntTableSlotType>;
918+
using SooInt32Table =
919+
ValueTable<int32_t, /*kTransferable=*/true, /*kSoo=*/true>;
918920
using SooIntTable = ValueTable<int64_t, /*kTransferable=*/true, /*kSoo=*/true>;
919921
using NonMemcpyableSooIntTable =
920922
ValueTable<int64_t, /*kTransferable=*/false, /*kSoo=*/true>;
@@ -1581,7 +1583,10 @@ TEST(Table, InsertOverloads) {
15811583

15821584
TYPED_TEST(SooTest, LargeTable) {
15831585
TypeParam t;
1584-
for (int64_t i = 0; i != 10000; ++i) t.emplace(i << 40);
1586+
for (int64_t i = 0; i != 10000; ++i) {
1587+
t.emplace(i << 40);
1588+
ASSERT_EQ(t.size(), i + 1);
1589+
}
15851590
for (int64_t i = 0; i != 10000; ++i)
15861591
ASSERT_EQ(i << 40, static_cast<int64_t>(*t.find(i << 40)));
15871592
}
@@ -2913,14 +2918,20 @@ TYPED_TEST(SooTest, IteratorInvalidAssertsEqualityOperatorRehash) {
29132918
template <typename T>
29142919
class RawHashSamplerTest : public testing::Test {};
29152920

2916-
using RawHashSamplerTestTypes = ::testing::Types<SooIntTable, NonSooIntTable>;
2921+
using RawHashSamplerTestTypes = ::testing::Types<
2922+
// 32 bits to make sure that table is Soo for 32 bits platform as well.
2923+
// 64 bits table is not SOO due to alignment.
2924+
SooInt32Table,
2925+
NonSooIntTable>;
29172926
TYPED_TEST_SUITE(RawHashSamplerTest, RawHashSamplerTestTypes);
29182927

29192928
TYPED_TEST(RawHashSamplerTest, Sample) {
2920-
constexpr bool soo_enabled = std::is_same<SooIntTable, TypeParam>::value;
2929+
constexpr bool soo_enabled = std::is_same<SooInt32Table, TypeParam>::value;
29212930
// Enable the feature even if the prod default is off.
29222931
SetSamplingRateTo1Percent();
29232932

2933+
ASSERT_EQ(TypeParam().capacity(), soo_enabled ? SooCapacity() : 0);
2934+
29242935
auto& sampler = GlobalHashtablezSampler();
29252936
size_t start_size = 0;
29262937

@@ -2992,7 +3003,7 @@ TYPED_TEST(RawHashSamplerTest, Sample) {
29923003
}
29933004

29943005
std::vector<const HashtablezInfo*> SampleSooMutation(
2995-
absl::FunctionRef<void(SooIntTable&)> mutate_table) {
3006+
absl::FunctionRef<void(SooInt32Table&)> mutate_table) {
29963007
// Enable the feature even if the prod default is off.
29973008
SetSamplingRateTo1Percent();
29983009

@@ -3005,7 +3016,7 @@ std::vector<const HashtablezInfo*> SampleSooMutation(
30053016
++start_size;
30063017
});
30073018

3008-
std::vector<SooIntTable> tables;
3019+
std::vector<SooInt32Table> tables;
30093020
for (int i = 0; i < 1000000; ++i) {
30103021
tables.emplace_back();
30113022
mutate_table(tables.back());
@@ -3025,16 +3036,16 @@ std::vector<const HashtablezInfo*> SampleSooMutation(
30253036
}
30263037

30273038
TEST(RawHashSamplerTest, SooTableInsertToEmpty) {
3028-
if (SooIntTable().capacity() != SooCapacity()) {
3039+
if (SooInt32Table().capacity() != SooCapacity()) {
30293040
CHECK_LT(sizeof(void*), 8) << "missing SOO coverage";
30303041
GTEST_SKIP() << "not SOO on this platform";
30313042
}
30323043
std::vector<const HashtablezInfo*> infos =
3033-
SampleSooMutation([](SooIntTable& t) { t.insert(1); });
3044+
SampleSooMutation([](SooInt32Table& t) { t.insert(1); });
30343045

30353046
for (const HashtablezInfo* info : infos) {
30363047
ASSERT_EQ(info->inline_element_size,
3037-
sizeof(typename SooIntTable::value_type));
3048+
sizeof(typename SooInt32Table::value_type));
30383049
ASSERT_EQ(info->soo_capacity, SooCapacity());
30393050
ASSERT_EQ(info->capacity, NextCapacity(SooCapacity()));
30403051
ASSERT_EQ(info->size, 1);
@@ -3046,16 +3057,16 @@ TEST(RawHashSamplerTest, SooTableInsertToEmpty) {
30463057
}
30473058

30483059
TEST(RawHashSamplerTest, SooTableReserveToEmpty) {
3049-
if (SooIntTable().capacity() != SooCapacity()) {
3060+
if (SooInt32Table().capacity() != SooCapacity()) {
30503061
CHECK_LT(sizeof(void*), 8) << "missing SOO coverage";
30513062
GTEST_SKIP() << "not SOO on this platform";
30523063
}
30533064
std::vector<const HashtablezInfo*> infos =
3054-
SampleSooMutation([](SooIntTable& t) { t.reserve(100); });
3065+
SampleSooMutation([](SooInt32Table& t) { t.reserve(100); });
30553066

30563067
for (const HashtablezInfo* info : infos) {
30573068
ASSERT_EQ(info->inline_element_size,
3058-
sizeof(typename SooIntTable::value_type));
3069+
sizeof(typename SooInt32Table::value_type));
30593070
ASSERT_EQ(info->soo_capacity, SooCapacity());
30603071
ASSERT_GE(info->capacity, 100);
30613072
ASSERT_EQ(info->size, 0);
@@ -3069,19 +3080,19 @@ TEST(RawHashSamplerTest, SooTableReserveToEmpty) {
30693080
// This tests that reserve on a full SOO table doesn't incorrectly result in new
30703081
// (over-)sampling.
30713082
TEST(RawHashSamplerTest, SooTableReserveToFullSoo) {
3072-
if (SooIntTable().capacity() != SooCapacity()) {
3083+
if (SooInt32Table().capacity() != SooCapacity()) {
30733084
CHECK_LT(sizeof(void*), 8) << "missing SOO coverage";
30743085
GTEST_SKIP() << "not SOO on this platform";
30753086
}
30763087
std::vector<const HashtablezInfo*> infos =
3077-
SampleSooMutation([](SooIntTable& t) {
3088+
SampleSooMutation([](SooInt32Table& t) {
30783089
t.insert(1);
30793090
t.reserve(100);
30803091
});
30813092

30823093
for (const HashtablezInfo* info : infos) {
30833094
ASSERT_EQ(info->inline_element_size,
3084-
sizeof(typename SooIntTable::value_type));
3095+
sizeof(typename SooInt32Table::value_type));
30853096
ASSERT_EQ(info->soo_capacity, SooCapacity());
30863097
ASSERT_GE(info->capacity, 100);
30873098
ASSERT_EQ(info->size, 1);
@@ -3095,12 +3106,12 @@ TEST(RawHashSamplerTest, SooTableReserveToFullSoo) {
30953106
// This tests that rehash(0) on a sampled table with size that fits in SOO
30963107
// doesn't incorrectly result in losing sampling.
30973108
TEST(RawHashSamplerTest, SooTableRehashShrinkWhenSizeFitsInSoo) {
3098-
if (SooIntTable().capacity() != SooCapacity()) {
3109+
if (SooInt32Table().capacity() != SooCapacity()) {
30993110
CHECK_LT(sizeof(void*), 8) << "missing SOO coverage";
31003111
GTEST_SKIP() << "not SOO on this platform";
31013112
}
31023113
std::vector<const HashtablezInfo*> infos =
3103-
SampleSooMutation([](SooIntTable& t) {
3114+
SampleSooMutation([](SooInt32Table& t) {
31043115
t.reserve(100);
31053116
t.insert(1);
31063117
EXPECT_GE(t.capacity(), 100);
@@ -3109,7 +3120,7 @@ TEST(RawHashSamplerTest, SooTableRehashShrinkWhenSizeFitsInSoo) {
31093120

31103121
for (const HashtablezInfo* info : infos) {
31113122
ASSERT_EQ(info->inline_element_size,
3112-
sizeof(typename SooIntTable::value_type));
3123+
sizeof(typename SooInt32Table::value_type));
31133124
ASSERT_EQ(info->soo_capacity, SooCapacity());
31143125
ASSERT_EQ(info->capacity, NextCapacity(SooCapacity()));
31153126
ASSERT_EQ(info->size, 1);
@@ -4049,6 +4060,18 @@ TEST(Table, MovedFromCallsFail) {
40494060
}
40504061
}
40514062

4063+
TEST(HashtableSize, GenerateNewSeedDoesntChangeSize) {
4064+
size_t size = 1;
4065+
do {
4066+
HashtableSize hs(no_seed_empty_tag_t{});
4067+
hs.increment_size(size);
4068+
EXPECT_EQ(hs.size(), size);
4069+
hs.generate_new_seed();
4070+
EXPECT_EQ(hs.size(), size);
4071+
size = size * 2 + 1;
4072+
} while (size < MaxValidSizeFor1ByteSlot());
4073+
}
4074+
40524075
TEST(Table, MaxValidSize) {
40534076
IntTable t;
40544077
EXPECT_EQ(MaxValidSize(sizeof(IntTable::value_type)), t.max_size());

0 commit comments

Comments
 (0)