Skip to content

Commit 6ff66f6

Browse files
kungakunga
authored
Avoid Undefined behavior: reference binding to misaligned address (#5638)
1 parent e9bce00 commit 6ff66f6

14 files changed

+176
-143
lines changed

‎ydb/core/tablet_flat/flat_fwd_cache.h

+7-7
Original file line numberDiff line numberDiff line change
@@ -301,9 +301,9 @@ namespace NFwd {
301301

302302
auto& meta = Part->IndexPages.GetBTree(groupId);
303303
Levels.resize(meta.LevelCount + 1);
304-
Levels[0].Queue.push_back({meta.PageId, meta.DataSize});
304+
Levels[0].Queue.push_back({meta.GetPageId(), meta.GetDataSize()});
305305
if (meta.LevelCount) {
306-
IndexPageLocator.Add(meta.PageId, GroupId, 0);
306+
IndexPageLocator.Add(meta.GetPageId(), GroupId, 0);
307307
}
308308
}
309309

@@ -371,7 +371,7 @@ namespace NFwd {
371371
if (levelId + 2 < Levels.size()) { // next level is index
372372
NPage::TBtreeIndexNode node(page.Data);
373373
for (auto pos : xrange(node.GetChildrenCount())) {
374-
IndexPageLocator.Add(node.GetShortChild(pos).PageId, GroupId, levelId + 1);
374+
IndexPageLocator.Add(node.GetShortChild(pos).GetPageId(), GroupId, levelId + 1);
375375
}
376376
}
377377

@@ -433,12 +433,12 @@ namespace NFwd {
433433
NPage::TBtreeIndexNode node(page.Data);
434434
for (auto pos : xrange(node.GetChildrenCount())) {
435435
auto& child = node.GetShortChild(pos);
436-
if (child.RowCount <= BeginRowId) {
436+
if (child.GetRowCount() <= BeginRowId) {
437437
continue;
438438
}
439-
Y_ABORT_UNLESS(!Levels[levelId + 1].Queue || Levels[levelId + 1].Queue.back().PageId < child.PageId);
440-
Levels[levelId + 1].Queue.push_back({child.PageId, child.DataSize});
441-
if (child.RowCount >= EndRowId) {
439+
Y_ABORT_UNLESS(!Levels[levelId + 1].Queue || Levels[levelId + 1].Queue.back().PageId < child.GetPageId());
440+
Levels[levelId + 1].Queue.push_back({child.GetPageId(), child.GetDataSize()});
441+
if (child.GetRowCount() >= EndRowId) {
442442
break;
443443
}
444444
}

‎ydb/core/tablet_flat/flat_page_btree_index.h

+55-23
Original file line numberDiff line numberDiff line change
@@ -84,48 +84,80 @@ namespace NKikimr::NTable::NPage {
8484
static_assert(sizeof(TIsNullBitmap) == 1, "Invalid TBtreeIndexNode TIsNullBitmap size");
8585

8686
struct TShortChild {
87-
TPageId PageId;
88-
TRowId RowCount;
89-
ui64 DataSize;
87+
TPageId PageId_;
88+
TRowId RowCount_;
89+
ui64 DataSize_;
90+
91+
inline TPageId GetPageId() const noexcept {
92+
return PageId_;
93+
}
94+
95+
inline TRowId GetRowCount() const noexcept {
96+
return RowCount_;
97+
}
98+
99+
inline ui64 GetDataSize() const noexcept {
100+
return DataSize_;
101+
}
90102

91103
auto operator<=>(const TShortChild&) const = default;
92104
} Y_PACKED;
93105

94106
static_assert(sizeof(TShortChild) == 20, "Invalid TBtreeIndexNode TShortChild size");
95107

96108
struct TChild {
97-
TPageId PageId;
98-
TRowId RowCount;
99-
ui64 DataSize;
100-
ui64 GroupDataSize;
101-
TRowId ErasedRowCount;
109+
TPageId PageId_;
110+
TRowId RowCount_;
111+
ui64 DataSize_;
112+
ui64 GroupDataSize_;
113+
TRowId ErasedRowCount_;
102114

103115
auto operator<=>(const TChild&) const = default;
104116

105-
TRowId GetNonErasedRowCount() const noexcept {
106-
return RowCount - ErasedRowCount;
117+
inline TPageId GetPageId() const noexcept {
118+
return PageId_;
119+
}
120+
121+
inline TRowId GetRowCount() const noexcept {
122+
return RowCount_;
123+
}
124+
125+
inline ui64 GetDataSize() const noexcept {
126+
return DataSize_;
127+
}
128+
129+
inline ui64 GetGroupDataSize() const noexcept {
130+
return GroupDataSize_;
131+
}
132+
133+
inline TRowId GetErasedRowCount() const noexcept {
134+
return ErasedRowCount_;
135+
}
136+
137+
inline TRowId GetNonErasedRowCount() const noexcept {
138+
return RowCount_ - ErasedRowCount_;
107139
}
108140

109-
ui64 GetTotalDataSize() const noexcept {
110-
return DataSize + GroupDataSize;
141+
inline ui64 GetTotalDataSize() const noexcept {
142+
return DataSize_ + GroupDataSize_;
111143
}
112144

113145
TString ToString() const noexcept {
114146
TStringBuilder result;
115-
result << "PageId: " << PageId << " RowCount: " << RowCount << " DataSize: " << DataSize;
116-
if (GroupDataSize) {
117-
result << " GroupDataSize: " << GroupDataSize;
147+
result << "PageId: " << GetPageId() << " RowCount: " << GetRowCount() << " DataSize: " << GetDataSize();
148+
if (GetGroupDataSize()) {
149+
result << " GroupDataSize: " << GetGroupDataSize();
118150
}
119-
result << " ErasedRowCount: " << ErasedRowCount;
151+
result << " ErasedRowCount: " << GetErasedRowCount();
120152
return result;
121153
}
122154
} Y_PACKED;
123155

124156
static_assert(sizeof(TChild) == 36, "Invalid TBtreeIndexNode TChild size");
125157

126-
static_assert(offsetof(TChild, PageId) == offsetof(TShortChild, PageId));
127-
static_assert(offsetof(TChild, RowCount) == offsetof(TShortChild, RowCount));
128-
static_assert(offsetof(TChild, DataSize) == offsetof(TShortChild, DataSize));
158+
static_assert(offsetof(TChild, PageId_) == offsetof(TShortChild, PageId_));
159+
static_assert(offsetof(TChild, RowCount_) == offsetof(TShortChild, RowCount_));
160+
static_assert(offsetof(TChild, DataSize_) == offsetof(TShortChild, DataSize_));
129161

130162
#pragma pack(pop)
131163

@@ -357,19 +389,19 @@ namespace NKikimr::NTable::NPage {
357389

358390
auto range = xrange(0u, childrenCount);
359391
const auto cmp = [this](TRowId rowId, TPos pos) {
360-
return rowId < GetShortChild(pos).RowCount;
392+
return rowId < GetShortChild(pos).GetRowCount();
361393
};
362394

363395
TRecIdx result;
364396
if (!on) {
365397
// Will do a full binary search on full range
366-
} else if (GetShortChild(*on).RowCount <= rowId) {
398+
} else if (GetShortChild(*on).GetRowCount() <= rowId) {
367399
// Try a short linear search first
368400
result = *on;
369401
for (int linear = 0; linear < 4; ++linear) {
370402
result++;
371403
Y_ABORT_UNLESS(result < childrenCount, "Should always seek some child");
372-
if (GetShortChild(result).RowCount > rowId) {
404+
if (GetShortChild(result).GetRowCount() > rowId) {
373405
return result;
374406
}
375407
}
@@ -383,7 +415,7 @@ namespace NKikimr::NTable::NPage {
383415
if (result == 0) {
384416
return 0;
385417
}
386-
if (GetShortChild(result - 1).RowCount <= rowId) {
418+
if (GetShortChild(result - 1).GetRowCount() <= rowId) {
387419
return result;
388420
}
389421
result--;

‎ydb/core/tablet_flat/flat_page_btree_index_writer.h

+10-10
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ namespace NKikimr::NTable::NPage {
5050
}
5151

5252
void AddChild(TChild child) {
53-
Y_ABORT_UNLESS(child.ErasedRowCount == 0 || !IsShortChildFormat(), "Short format can't have ErasedRowCount");
53+
Y_ABORT_UNLESS(child.GetErasedRowCount() == 0 || !IsShortChildFormat(), "Short format can't have ErasedRowCount");
5454
Children.push_back(child);
5555
}
5656

@@ -249,9 +249,9 @@ namespace NKikimr::NTable::NPage {
249249
void PlaceChild(const TChild& child) noexcept
250250
{
251251
if (IsShortChildFormat()) {
252-
Y_DEBUG_ABORT_UNLESS(child.GroupDataSize == 0);
253-
Y_DEBUG_ABORT_UNLESS(child.ErasedRowCount == 0);
254-
Place<TShortChild>() = TShortChild{child.PageId, child.RowCount, child.DataSize};
252+
Y_DEBUG_ABORT_UNLESS(child.GetGroupDataSize() == 0);
253+
Y_DEBUG_ABORT_UNLESS(child.GetErasedRowCount() == 0);
254+
Place<TShortChild>() = TShortChild{child.GetPageId(), child.GetRowCount(), child.GetDataSize()};
255255
} else {
256256
Place<TChild>() = child;
257257
}
@@ -377,15 +377,15 @@ namespace NKikimr::NTable::NPage {
377377
}
378378

379379
void AddShortChild(TShortChild child) {
380-
AddChild(TChild{child.PageId, child.RowCount, child.DataSize, 0, 0});
380+
AddChild(TChild{child.GetPageId(), child.GetRowCount(), child.GetDataSize(), 0, 0});
381381
}
382382

383383
void AddChild(TChild child) {
384384
// aggregate in order to perform search by row id from any leaf node
385-
child.RowCount = (ChildRowCount += child.RowCount);
386-
child.DataSize = (ChildDataSize += child.DataSize);
387-
child.GroupDataSize = (ChildGroupDataSize += child.GroupDataSize);
388-
child.ErasedRowCount = (ChildErasedRowCount += child.ErasedRowCount);
385+
child.RowCount_ = (ChildRowCount += child.GetRowCount());
386+
child.DataSize_ = (ChildDataSize += child.GetDataSize());
387+
child.GroupDataSize_ = (ChildGroupDataSize += child.GetGroupDataSize());
388+
child.ErasedRowCount_ = (ChildErasedRowCount += child.GetErasedRowCount());
389389

390390
Levels[0].PushChild(child);
391391
}
@@ -478,7 +478,7 @@ namespace NKikimr::NTable::NPage {
478478
Levels.emplace_back();
479479
Y_ABORT_UNLESS(Levels.size() < Max<ui32>(), "Levels size is out of bounds");
480480
}
481-
lastChild.PageId = pageId;
481+
lastChild.PageId_ = pageId;
482482
Levels[levelIndex + 1].PushChild(lastChild);
483483
if (!last) {
484484
Levels[levelIndex + 1].PushKey(Levels[levelIndex].PopKey());

0 commit comments

Comments
 (0)