Skip to content

Commit 72a9eb2

Browse files
serge-sans-pailleserge-sans-paille
committed
[llvm] Fix __builtin_object_size interaction between Negative Offset and Select/Phi
When picking a SizeOffsetAPInt through combineSizeOffset, the current constant offset that has been extracted should be taken into account to perform the right decision, otherwise in the presence of nullptr, we may perform the wrong decision. Fix #111709
1 parent 6af2f22 commit 72a9eb2

File tree

3 files changed

+136
-7
lines changed

3 files changed

+136
-7
lines changed

‎llvm/include/llvm/Analysis/MemoryBuiltins.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ struct ObjectSizeOpts {
160160
/// though they can't be evaluated. Otherwise, null is always considered to
161161
/// point to a 0 byte region of memory.
162162
bool NullIsUnknownSize = false;
163+
163164
/// If set, used for more accurate evaluation
164165
AAResults *AA = nullptr;
165166
};
@@ -230,6 +231,7 @@ class ObjectSizeOffsetVisitor
230231
ObjectSizeOpts Options;
231232
unsigned IntTyBits;
232233
APInt Zero;
234+
APInt ConstantOffset;
233235
SmallDenseMap<Instruction *, SizeOffsetAPInt, 8> SeenInsts;
234236
unsigned InstructionsVisited;
235237

‎llvm/lib/Analysis/MemoryBuiltins.cpp

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,14 @@ static APInt getSizeWithOverflow(const SizeOffsetAPInt &Data) {
570570
return Size - Offset;
571571
}
572572

573+
static SizeOffsetAPInt applyConstantOffset(SizeOffsetAPInt SOT,
574+
APInt ConstantOffset) {
575+
if (ConstantOffset.isZero())
576+
return SOT;
577+
return {SOT.Size, SOT.Offset.getBitWidth() > 1 ? SOT.Offset + ConstantOffset
578+
: SOT.Offset};
579+
}
580+
573581
/// Compute the size of the object pointed by Ptr. Returns true and the
574582
/// object size in Size if successful, and false otherwise.
575583
/// If RoundToAlign is true, then Size is rounded up to the alignment of
@@ -697,7 +705,8 @@ SizeOffsetAPInt ObjectSizeOffsetVisitor::computeImpl(Value *V) {
697705
// the index type size and if we stripped address space casts we have to
698706
// readjust the APInt as we pass it upwards in order for the APInt to match
699707
// the type the caller passed in.
700-
APInt Offset(InitialIntTyBits, 0);
708+
709+
APInt Offset = APInt{InitialIntTyBits, 0};
701710
V = V->stripAndAccumulateConstantOffsets(
702711
DL, Offset, /* AllowNonInbounds */ true, /* AllowInvariantGroup */ true);
703712

@@ -706,7 +715,9 @@ SizeOffsetAPInt ObjectSizeOffsetVisitor::computeImpl(Value *V) {
706715
IntTyBits = DL.getIndexTypeSizeInBits(V->getType());
707716
Zero = APInt::getZero(IntTyBits);
708717

718+
std::swap(Offset, ConstantOffset);
709719
SizeOffsetAPInt SOT = computeValue(V);
720+
std::swap(Offset, ConstantOffset);
710721

711722
bool IndexTypeSizeChanged = InitialIntTyBits != IntTyBits;
712723
if (!IndexTypeSizeChanged && Offset.isZero())
@@ -723,8 +734,7 @@ SizeOffsetAPInt ObjectSizeOffsetVisitor::computeImpl(Value *V) {
723734
SOT.Offset = APInt();
724735
}
725736
// If the computed offset is "unknown" we cannot add the stripped offset.
726-
return {SOT.Size,
727-
SOT.Offset.getBitWidth() > 1 ? SOT.Offset + Offset : SOT.Offset};
737+
return applyConstantOffset(SOT, Offset);
728738
}
729739

730740
SizeOffsetAPInt ObjectSizeOffsetVisitor::computeValue(Value *V) {
@@ -982,17 +992,28 @@ ObjectSizeOffsetVisitor::combineSizeOffset(SizeOffsetAPInt LHS,
982992
if (!LHS.bothKnown() || !RHS.bothKnown())
983993
return ObjectSizeOffsetVisitor::unknown();
984994

995+
SizeOffsetAPInt AdjustedLHS = applyConstantOffset(LHS, ConstantOffset);
996+
SizeOffsetAPInt AdjustedRHS = applyConstantOffset(RHS, ConstantOffset);
997+
985998
switch (Options.EvalMode) {
986999
case ObjectSizeOpts::Mode::Min:
987-
return (getSizeWithOverflow(LHS).slt(getSizeWithOverflow(RHS))) ? LHS : RHS;
1000+
return (getSizeWithOverflow(AdjustedLHS)
1001+
.slt(getSizeWithOverflow(AdjustedRHS)))
1002+
? LHS
1003+
: RHS;
9881004
case ObjectSizeOpts::Mode::Max:
989-
return (getSizeWithOverflow(LHS).sgt(getSizeWithOverflow(RHS))) ? LHS : RHS;
1005+
return (getSizeWithOverflow(AdjustedLHS)
1006+
.sgt(getSizeWithOverflow(AdjustedRHS)))
1007+
? LHS
1008+
: RHS;
9901009
case ObjectSizeOpts::Mode::ExactSizeFromOffset:
991-
return (getSizeWithOverflow(LHS).eq(getSizeWithOverflow(RHS)))
1010+
return (getSizeWithOverflow(AdjustedLHS)
1011+
.eq(getSizeWithOverflow(AdjustedRHS)))
9921012
? LHS
9931013
: ObjectSizeOffsetVisitor::unknown();
9941014
case ObjectSizeOpts::Mode::ExactUnderlyingSizeAndOffset:
995-
return LHS == RHS ? LHS : ObjectSizeOffsetVisitor::unknown();
1015+
return AdjustedLHS == AdjustedRHS ? LHS
1016+
: ObjectSizeOffsetVisitor::unknown();
9961017
}
9971018
llvm_unreachable("missing an eval mode");
9981019
}

‎llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,3 +117,109 @@ if.end:
117117
%size = call i64 @llvm.objectsize.i64.p0(ptr %p, i1 true, i1 true, i1 false)
118118
ret i64 %size
119119
}
120+
121+
define i64 @pick_negative_offset(i32 %n) {
122+
; CHECK-LABEL: @pick_negative_offset(
123+
; CHECK-NEXT: entry:
124+
; CHECK-NEXT: [[BUFFER0:%.*]] = alloca i8, i64 20, align 1
125+
; CHECK-NEXT: [[OFFSETED0:%.*]] = getelementptr i8, ptr [[BUFFER0]], i64 20
126+
; CHECK-NEXT: [[COND:%.*]] = icmp eq i32 [[N:%.*]], 0
127+
; CHECK-NEXT: br i1 [[COND]], label [[IF_ELSE:%.*]], label [[IF_END:%.*]]
128+
; CHECK: if.else:
129+
; CHECK-NEXT: [[BUFFER1:%.*]] = alloca i8, i64 20, align 1
130+
; CHECK-NEXT: [[OFFSETED1:%.*]] = getelementptr i8, ptr [[BUFFER1]], i64 20
131+
; CHECK-NEXT: br label [[IF_END]]
132+
; CHECK: if.end:
133+
; CHECK-NEXT: [[P:%.*]] = phi ptr [ [[OFFSETED1]], [[IF_ELSE]] ], [ [[OFFSETED0]], [[ENTRY:%.*]] ]
134+
; CHECK-NEXT: [[POFFSETED:%.*]] = getelementptr i8, ptr [[P]], i64 -4
135+
; CHECK-NEXT: ret i64 4
136+
;
137+
entry:
138+
%buffer0 = alloca i8, i64 20
139+
%offseted0 = getelementptr i8, ptr %buffer0, i64 20
140+
%cond = icmp eq i32 %n, 0
141+
br i1 %cond, label %if.else, label %if.end
142+
143+
if.else:
144+
%buffer1 = alloca i8, i64 20
145+
%offseted1 = getelementptr i8, ptr %buffer1, i64 20
146+
br label %if.end
147+
148+
if.end:
149+
%p = phi ptr [ %offseted1, %if.else ], [ %offseted0, %entry ]
150+
%poffseted = getelementptr i8, ptr %p, i64 -4
151+
%size = call i64 @llvm.objectsize.i64.p0(ptr %poffseted, i1 false, i1 false, i1 false)
152+
ret i64 %size
153+
}
154+
155+
define i64 @pick_negative_offset_with_nullptr(i32 %n) {
156+
; CHECK-LABEL: @pick_negative_offset_with_nullptr(
157+
; CHECK-NEXT: entry:
158+
; CHECK-NEXT: [[BUFFER0:%.*]] = alloca i8, i64 20, align 1
159+
; CHECK-NEXT: [[OFFSETED0:%.*]] = getelementptr i8, ptr [[BUFFER0]], i64 20
160+
; CHECK-NEXT: [[COND:%.*]] = icmp eq i32 [[N:%.*]], 0
161+
; CHECK-NEXT: br i1 [[COND]], label [[IF_ELSE:%.*]], label [[IF_END:%.*]]
162+
; CHECK: if.else:
163+
; CHECK-NEXT: br label [[IF_END]]
164+
; CHECK: if.end:
165+
; CHECK-NEXT: [[P0:%.*]] = phi ptr [ [[OFFSETED0]], [[ENTRY:%.*]] ], [ null, [[IF_ELSE]] ]
166+
; CHECK-NEXT: [[P1:%.*]] = phi ptr [ null, [[IF_ELSE]] ], [ [[OFFSETED0]], [[ENTRY]] ]
167+
; CHECK-NEXT: [[P0OFFSETED:%.*]] = getelementptr i8, ptr [[P0]], i64 -4
168+
; CHECK-NEXT: [[P1OFFSETED:%.*]] = getelementptr i8, ptr [[P1]], i64 -4
169+
; CHECK-NEXT: ret i64 4
170+
;
171+
entry:
172+
%buffer0 = alloca i8, i64 20
173+
%offseted0 = getelementptr i8, ptr %buffer0, i64 20
174+
%cond = icmp eq i32 %n, 0
175+
br i1 %cond, label %if.else, label %if.end
176+
177+
if.else:
178+
br label %if.end
179+
180+
if.end:
181+
%p0 = phi ptr [ %offseted0, %entry ], [ null, %if.else ]
182+
%p1 = phi ptr [ null, %if.else ], [ %offseted0, %entry ]
183+
%p0offseted = getelementptr i8, ptr %p0, i64 -4
184+
%p1offseted = getelementptr i8, ptr %p1, i64 -4
185+
%size0 = call i64 @llvm.objectsize.i64.p0(ptr %p0offseted, i1 false, i1 false, i1 false)
186+
%size1 = call i64 @llvm.objectsize.i64.p0(ptr %p1offseted, i1 false, i1 false, i1 false)
187+
%size = select i1 %cond, i64 %size0, i64 %size1
188+
ret i64 %size
189+
}
190+
191+
define i64 @pick_negative_offset_with_unsized_nullptr(i32 %n) {
192+
; CHECK-LABEL: @pick_negative_offset_with_unsized_nullptr(
193+
; CHECK-NEXT: entry:
194+
; CHECK-NEXT: [[BUFFER0:%.*]] = alloca i8, i64 20, align 1
195+
; CHECK-NEXT: [[OFFSETED0:%.*]] = getelementptr i8, ptr [[BUFFER0]], i64 20
196+
; CHECK-NEXT: [[COND:%.*]] = icmp eq i32 [[N:%.*]], 0
197+
; CHECK-NEXT: br i1 [[COND]], label [[IF_ELSE:%.*]], label [[IF_END:%.*]]
198+
; CHECK: if.else:
199+
; CHECK-NEXT: br label [[IF_END]]
200+
; CHECK: if.end:
201+
; CHECK-NEXT: [[P0:%.*]] = phi ptr [ [[OFFSETED0]], [[ENTRY:%.*]] ], [ null, [[IF_ELSE]] ]
202+
; CHECK-NEXT: [[P1:%.*]] = phi ptr [ null, [[IF_ELSE]] ], [ [[OFFSETED0]], [[ENTRY]] ]
203+
; CHECK-NEXT: [[P0OFFSETED:%.*]] = getelementptr i8, ptr [[P0]], i64 -4
204+
; CHECK-NEXT: [[P1OFFSETED:%.*]] = getelementptr i8, ptr [[P1]], i64 -4
205+
; CHECK-NEXT: ret i64 -1
206+
;
207+
entry:
208+
%buffer0 = alloca i8, i64 20
209+
%offseted0 = getelementptr i8, ptr %buffer0, i64 20
210+
%cond = icmp eq i32 %n, 0
211+
br i1 %cond, label %if.else, label %if.end
212+
213+
if.else:
214+
br label %if.end
215+
216+
if.end:
217+
%p0 = phi ptr [ %offseted0, %entry ], [ null, %if.else ]
218+
%p1 = phi ptr [ null, %if.else ], [ %offseted0, %entry ]
219+
%p0offseted = getelementptr i8, ptr %p0, i64 -4
220+
%p1offseted = getelementptr i8, ptr %p1, i64 -4
221+
%size0 = call i64 @llvm.objectsize.i64.p0(ptr %p0offseted, i1 false, i1 true, i1 false)
222+
%size1 = call i64 @llvm.objectsize.i64.p0(ptr %p1offseted, i1 false, i1 true, i1 false)
223+
%size = select i1 %cond, i64 %size0, i64 %size1
224+
ret i64 %size
225+
}

0 commit comments

Comments
 (0)