Skip to content

Commit 6cd2f37

Browse files
rjmccallhyp
authored andcommitted
Fix a bug in OptimizedStructLayout when filling gaps before
fixed fields with highly-aligned flexible fields. The code was not considering the possibility that aligning the current offset to the alignment of a queue might push us past the end of the gap. Subtracting the offsets to figure out the maximum field size for the gap then overflowed, making us think that we had nearly unbounded space to fill. Fixes PR 51131. (cherry picked from commit 326a5a2)
1 parent 41ac94d commit 6cd2f37

File tree

2 files changed

+25
-2
lines changed

2 files changed

+25
-2
lines changed

llvm/lib/Support/OptimizedStructLayout.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,7 @@ llvm::performOptimizedStructLayout(MutableArrayRef<Field> Fields) {
350350
Optional<uint64_t> EndOffset) -> bool {
351351
assert(Queue->Head);
352352
assert(StartOffset == alignTo(LastEnd, Queue->Alignment));
353+
assert(!EndOffset || StartOffset < *EndOffset);
353354

354355
// Figure out the maximum size that a field can be, and ignore this
355356
// queue if there's nothing in it that small.
@@ -372,6 +373,7 @@ llvm::performOptimizedStructLayout(MutableArrayRef<Field> Fields) {
372373
// Helper function to find the "best" flexible-offset field according
373374
// to the criteria described above.
374375
auto tryAddBestField = [&](Optional<uint64_t> BeforeOffset) -> bool {
376+
assert(!BeforeOffset || LastEnd < *BeforeOffset);
375377
auto QueueB = FlexibleFieldsByAlignment.begin();
376378
auto QueueE = FlexibleFieldsByAlignment.end();
377379

@@ -403,9 +405,12 @@ llvm::performOptimizedStructLayout(MutableArrayRef<Field> Fields) {
403405
return false;
404406

405407
// Otherwise, scan backwards to find the most-aligned queue that
406-
// still has minimal leading padding after LastEnd.
408+
// still has minimal leading padding after LastEnd. If that
409+
// minimal padding is already at or past the end point, we're done.
407410
--FirstQueueToSearch;
408411
Offset = alignTo(LastEnd, FirstQueueToSearch->Alignment);
412+
if (BeforeOffset && Offset >= *BeforeOffset)
413+
return false;
409414
while (FirstQueueToSearch != QueueB &&
410415
Offset == alignTo(LastEnd, FirstQueueToSearch[-1].Alignment))
411416
--FirstQueueToSearch;
@@ -415,6 +420,7 @@ llvm::performOptimizedStructLayout(MutableArrayRef<Field> Fields) {
415420
// Phase 1: fill the gaps between fixed-offset fields with the best
416421
// flexible-offset field that fits.
417422
for (auto I = Fields.begin(); I != FirstFlexible; ++I) {
423+
assert(LastEnd <= I->Offset);
418424
while (LastEnd != I->Offset) {
419425
if (!tryAddBestField(I->Offset))
420426
break;

llvm/unittests/Support/OptimizedStructLayoutTest.cpp

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,4 +129,21 @@ TEST(OptimizedStructLayoutTest, GardenPath) {
129129
.flexible(2, 2, 42)
130130
.flexible(2, 2, 48)
131131
.verify(50, 4);
132-
}
132+
}
133+
134+
// PR 51131
135+
TEST(OptimizedStructLayoutTest, HighAlignment) {
136+
// Handle the case where a flexible field has such a high alignment
137+
// requirement that aligning LastEnd to it gives an offset past the
138+
// end of the gap before the next fixed-alignment field.
139+
LayoutTest()
140+
.fixed(8, 8, 0)
141+
.fixed(8, 8, 8)
142+
.fixed(64, 64, 64)
143+
.flexible(1, 1, 16)
144+
.flexible(1, 1, 17)
145+
.flexible(4, 128, 128)
146+
.flexible(1, 1, 18)
147+
.flexible(1, 1, 19)
148+
.verify(132, 128);
149+
}

0 commit comments

Comments
 (0)