Skip to content

Commit b4804bc

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 b4804bc

File tree

3 files changed

+165
-8
lines changed

3 files changed

+165
-8
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: 33 additions & 8 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,11 @@ SizeOffsetAPInt ObjectSizeOffsetVisitor::computeImpl(Value *V) {
706715
IntTyBits = DL.getIndexTypeSizeInBits(V->getType());
707716
Zero = APInt::getZero(IntTyBits);
708717

718+
APInt PrevConstantOffset = ConstantOffset;
719+
720+
ConstantOffset = ConstantOffset.sextOrTrunc(Offset.getBitWidth()) + Offset;
709721
SizeOffsetAPInt SOT = computeValue(V);
722+
ConstantOffset = PrevConstantOffset;
710723

711724
bool IndexTypeSizeChanged = InitialIntTyBits != IntTyBits;
712725
if (!IndexTypeSizeChanged && Offset.isZero())
@@ -723,8 +736,7 @@ SizeOffsetAPInt ObjectSizeOffsetVisitor::computeImpl(Value *V) {
723736
SOT.Offset = APInt();
724737
}
725738
// 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};
739+
return applyConstantOffset(SOT, Offset);
728740
}
729741

730742
SizeOffsetAPInt ObjectSizeOffsetVisitor::computeValue(Value *V) {
@@ -981,30 +993,43 @@ ObjectSizeOffsetVisitor::combineSizeOffset(SizeOffsetAPInt LHS,
981993
SizeOffsetAPInt RHS) {
982994
if (!LHS.bothKnown() || !RHS.bothKnown())
983995
return ObjectSizeOffsetVisitor::unknown();
996+
SizeOffsetAPInt AdjustedLHS = applyConstantOffset(LHS, ConstantOffset);
997+
SizeOffsetAPInt AdjustedRHS = applyConstantOffset(RHS, ConstantOffset);
984998

985999
switch (Options.EvalMode) {
9861000
case ObjectSizeOpts::Mode::Min:
987-
return (getSizeWithOverflow(LHS).slt(getSizeWithOverflow(RHS))) ? LHS : RHS;
1001+
return (getSizeWithOverflow(AdjustedLHS)
1002+
.slt(getSizeWithOverflow(AdjustedRHS)))
1003+
? LHS
1004+
: RHS;
9881005
case ObjectSizeOpts::Mode::Max:
989-
return (getSizeWithOverflow(LHS).sgt(getSizeWithOverflow(RHS))) ? LHS : RHS;
1006+
return (getSizeWithOverflow(AdjustedLHS)
1007+
.sgt(getSizeWithOverflow(AdjustedRHS)))
1008+
? LHS
1009+
: RHS;
9901010
case ObjectSizeOpts::Mode::ExactSizeFromOffset:
991-
return (getSizeWithOverflow(LHS).eq(getSizeWithOverflow(RHS)))
1011+
return (getSizeWithOverflow(AdjustedLHS)
1012+
.eq(getSizeWithOverflow(AdjustedRHS)))
9921013
? LHS
9931014
: ObjectSizeOffsetVisitor::unknown();
9941015
case ObjectSizeOpts::Mode::ExactUnderlyingSizeAndOffset:
995-
return LHS == RHS ? LHS : ObjectSizeOffsetVisitor::unknown();
1016+
return AdjustedLHS == AdjustedRHS ? LHS
1017+
: ObjectSizeOffsetVisitor::unknown();
9961018
}
9971019
llvm_unreachable("missing an eval mode");
9981020
}
9991021

10001022
SizeOffsetAPInt ObjectSizeOffsetVisitor::visitPHINode(PHINode &PN) {
10011023
if (PN.getNumIncomingValues() == 0)
10021024
return ObjectSizeOffsetVisitor::unknown();
1025+
10031026
auto IncomingValues = PN.incoming_values();
1027+
10041028
return std::accumulate(IncomingValues.begin() + 1, IncomingValues.end(),
10051029
computeImpl(*IncomingValues.begin()),
10061030
[this](SizeOffsetAPInt LHS, Value *VRHS) {
1007-
return combineSizeOffset(LHS, computeImpl(VRHS));
1031+
SizeOffsetAPInt RHS = computeImpl(VRHS);
1032+
return combineSizeOffset(LHS, RHS);
10081033
});
10091034
}
10101035

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

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,3 +117,133 @@ 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+
}
226+
227+
define i64 @chain_pick_negative_offset_with_nullptr(i32 %x) {
228+
; CHECK-LABEL: @chain_pick_negative_offset_with_nullptr(
229+
; CHECK-NEXT: entry:
230+
; CHECK-NEXT: [[ARRAY:%.*]] = alloca [4 x i32], align 4
231+
; CHECK-NEXT: [[C:%.*]] = icmp eq i32 [[X:%.*]], 0
232+
; CHECK-NEXT: [[P:%.*]] = getelementptr i8, ptr [[ARRAY]], i64 8
233+
; CHECK-NEXT: [[COND:%.*]] = select i1 [[C]], ptr [[P]], ptr null
234+
; CHECK-NEXT: [[P4:%.*]] = getelementptr i8, ptr [[COND]], i64 8
235+
; CHECK-NEXT: [[COND6:%.*]] = select i1 [[C]], ptr [[P4]], ptr null
236+
; CHECK-NEXT: [[P7:%.*]] = getelementptr i8, ptr [[COND6]], i64 -4
237+
; CHECK-NEXT: ret i64 4
238+
;
239+
entry:
240+
%array = alloca [4 x i32]
241+
%c = icmp eq i32 %x, 0
242+
%p = getelementptr i8, ptr %array, i64 8
243+
%cond = select i1 %c, ptr %p, ptr null
244+
%p4 = getelementptr i8, ptr %cond, i64 8
245+
%cond6 = select i1 %c, ptr %p4, ptr null
246+
%p7 = getelementptr i8, ptr %cond6, i64 -4
247+
%size = call i64 @llvm.objectsize.i64.p0(ptr %p7, i1 false, i1 false, i1 false)
248+
ret i64 %size
249+
}

0 commit comments

Comments
 (0)