Skip to content

Commit 294688c

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 294688c

File tree

3 files changed

+169
-8
lines changed

3 files changed

+169
-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: 37 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,14 @@ SizeOffsetAPInt ObjectSizeOffsetVisitor::computeImpl(Value *V) {
706715
IntTyBits = DL.getIndexTypeSizeInBits(V->getType());
707716
Zero = APInt::getZero(IntTyBits);
708717

718+
APInt PrevConstantOffset = ConstantOffset;
719+
720+
if (!::CheckedZextOrTrunc(ConstantOffset, InitialIntTyBits))
721+
return ObjectSizeOffsetVisitor::unknown();
722+
723+
ConstantOffset += Offset;
709724
SizeOffsetAPInt SOT = computeValue(V);
725+
ConstantOffset = PrevConstantOffset;
710726

711727
bool IndexTypeSizeChanged = InitialIntTyBits != IntTyBits;
712728
if (!IndexTypeSizeChanged && Offset.isZero())
@@ -722,9 +738,9 @@ SizeOffsetAPInt ObjectSizeOffsetVisitor::computeImpl(Value *V) {
722738
!::CheckedZextOrTrunc(SOT.Offset, InitialIntTyBits))
723739
SOT.Offset = APInt();
724740
}
741+
725742
// 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};
743+
return applyConstantOffset(SOT, Offset);
728744
}
729745

730746
SizeOffsetAPInt ObjectSizeOffsetVisitor::computeValue(Value *V) {
@@ -981,30 +997,43 @@ ObjectSizeOffsetVisitor::combineSizeOffset(SizeOffsetAPInt LHS,
981997
SizeOffsetAPInt RHS) {
982998
if (!LHS.bothKnown() || !RHS.bothKnown())
983999
return ObjectSizeOffsetVisitor::unknown();
1000+
SizeOffsetAPInt AdjustedLHS = applyConstantOffset(LHS, ConstantOffset);
1001+
SizeOffsetAPInt AdjustedRHS = applyConstantOffset(RHS, ConstantOffset);
9841002

9851003
switch (Options.EvalMode) {
9861004
case ObjectSizeOpts::Mode::Min:
987-
return (getSizeWithOverflow(LHS).slt(getSizeWithOverflow(RHS))) ? LHS : RHS;
1005+
return (getSizeWithOverflow(AdjustedLHS)
1006+
.slt(getSizeWithOverflow(AdjustedRHS)))
1007+
? LHS
1008+
: RHS;
9881009
case ObjectSizeOpts::Mode::Max:
989-
return (getSizeWithOverflow(LHS).sgt(getSizeWithOverflow(RHS))) ? LHS : RHS;
1010+
return (getSizeWithOverflow(AdjustedLHS)
1011+
.sgt(getSizeWithOverflow(AdjustedRHS)))
1012+
? LHS
1013+
: RHS;
9901014
case ObjectSizeOpts::Mode::ExactSizeFromOffset:
991-
return (getSizeWithOverflow(LHS).eq(getSizeWithOverflow(RHS)))
1015+
return (getSizeWithOverflow(AdjustedLHS)
1016+
.eq(getSizeWithOverflow(AdjustedRHS)))
9921017
? LHS
9931018
: ObjectSizeOffsetVisitor::unknown();
9941019
case ObjectSizeOpts::Mode::ExactUnderlyingSizeAndOffset:
995-
return LHS == RHS ? LHS : ObjectSizeOffsetVisitor::unknown();
1020+
return AdjustedLHS == AdjustedRHS ? LHS
1021+
: ObjectSizeOffsetVisitor::unknown();
9961022
}
9971023
llvm_unreachable("missing an eval mode");
9981024
}
9991025

10001026
SizeOffsetAPInt ObjectSizeOffsetVisitor::visitPHINode(PHINode &PN) {
10011027
if (PN.getNumIncomingValues() == 0)
10021028
return ObjectSizeOffsetVisitor::unknown();
1029+
10031030
auto IncomingValues = PN.incoming_values();
1031+
10041032
return std::accumulate(IncomingValues.begin() + 1, IncomingValues.end(),
10051033
computeImpl(*IncomingValues.begin()),
10061034
[this](SizeOffsetAPInt LHS, Value *VRHS) {
1007-
return combineSizeOffset(LHS, computeImpl(VRHS));
1035+
SizeOffsetAPInt RHS = computeImpl(VRHS);
1036+
return combineSizeOffset(LHS, RHS);
10081037
});
10091038
}
10101039

‎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)