[llvm] 203296d - [BoundsChecking] Fix merging of sizes
Arthur Eubanks via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 3 17:21:35 PDT 2022
Author: Arthur Eubanks
Date: 2022-08-03T17:21:19-07:00
New Revision: 203296d642c385da07a62098050b08493ed8b236
URL: https://github.com/llvm/llvm-project/commit/203296d642c385da07a62098050b08493ed8b236
DIFF: https://github.com/llvm/llvm-project/commit/203296d642c385da07a62098050b08493ed8b236.diff
LOG: [BoundsChecking] Fix merging of sizes
BoundsChecking uses ObjectSizeOffsetEvaluator to keep track of the
underlying size/offset of pointers in allocations. However,
ObjectSizeOffsetVisitor (something ObjectSizeOffsetEvaluator
uses to check for constant sizes/offsets)
doesn't quite treat sizes and offsets the same way as
BoundsChecking. BoundsChecking wants to know the size of the
underlying allocation and the current pointer's offset within
it, but ObjectSizeOffsetVisitor only cares about the size
from the pointer to the end of the underlying allocation.
This only comes up when merging two size/offset pairs. Add a new mode to
ObjectSizeOffsetVisitor which cares about the underlying size/offset
rather than the size from the current pointer to the end of the
allocation.
Fixes a false positive with -fsanitize=bounds.
Reviewed By: vitalybuka, asbirlea
Differential Revision: https://reviews.llvm.org/D131001
Added:
Modified:
llvm/include/llvm/Analysis/MemoryBuiltins.h
llvm/lib/Analysis/MemoryBuiltins.cpp
llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
llvm/test/Instrumentation/BoundsChecking/simple.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/Analysis/MemoryBuiltins.h b/llvm/include/llvm/Analysis/MemoryBuiltins.h
index 5cc85c4677cb2..949fe7270821a 100644
--- a/llvm/include/llvm/Analysis/MemoryBuiltins.h
+++ b/llvm/include/llvm/Analysis/MemoryBuiltins.h
@@ -134,17 +134,21 @@ Optional<StringRef> getAllocationFamily(const Value *I,
struct ObjectSizeOpts {
/// Controls how we handle conditional statements with unknown conditions.
enum class Mode : uint8_t {
- /// Fail to evaluate an unknown condition.
- Exact,
+ /// All branches must be known and have the same size, starting from the
+ /// offset, to be merged.
+ ExactSizeFromOffset,
+ /// All branches must be known and have the same underlying size and offset
+ /// to be merged.
+ ExactUnderlyingSizeAndOffset,
/// Evaluate all branches of an unknown condition. If all evaluations
/// succeed, pick the minimum size.
Min,
/// Same as Min, except we pick the maximum size of all of the branches.
- Max
+ Max,
};
/// How we want to evaluate this object's size.
- Mode EvalMode = Mode::Exact;
+ Mode EvalMode = Mode::ExactSizeFromOffset;
/// Whether to round the result up to the alignment of allocas, byval
/// arguments, and global variables.
bool RoundToAlign = false;
diff --git a/llvm/lib/Analysis/MemoryBuiltins.cpp b/llvm/lib/Analysis/MemoryBuiltins.cpp
index 413ec6dd4b429..31704c21358a5 100644
--- a/llvm/lib/Analysis/MemoryBuiltins.cpp
+++ b/llvm/lib/Analysis/MemoryBuiltins.cpp
@@ -641,7 +641,7 @@ Value *llvm::lowerObjectSizeCall(IntrinsicInst *ObjectSize,
EvalOptions.EvalMode =
MaxVal ? ObjectSizeOpts::Mode::Max : ObjectSizeOpts::Mode::Min;
else
- EvalOptions.EvalMode = ObjectSizeOpts::Mode::Exact;
+ EvalOptions.EvalMode = ObjectSizeOpts::Mode::ExactSizeFromOffset;
EvalOptions.NullIsUnknownSize =
cast<ConstantInt>(ObjectSize->getArgOperand(2))->isOne();
@@ -999,9 +999,11 @@ SizeOffsetType ObjectSizeOffsetVisitor::combineSizeOffset(SizeOffsetType LHS,
return (getSizeWithOverflow(LHS).slt(getSizeWithOverflow(RHS))) ? LHS : RHS;
case ObjectSizeOpts::Mode::Max:
return (getSizeWithOverflow(LHS).sgt(getSizeWithOverflow(RHS))) ? LHS : RHS;
- case ObjectSizeOpts::Mode::Exact:
+ case ObjectSizeOpts::Mode::ExactSizeFromOffset:
return (getSizeWithOverflow(LHS).eq(getSizeWithOverflow(RHS))) ? LHS
: unknown();
+ case ObjectSizeOpts::Mode::ExactUnderlyingSizeAndOffset:
+ return LHS == RHS && LHS.second.eq(RHS.second) ? LHS : unknown();
}
llvm_unreachable("missing an eval mode");
}
diff --git a/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp b/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
index 1eadafb4e4b4c..81ffa628c682a 100644
--- a/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
+++ b/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
@@ -146,6 +146,7 @@ static bool addBoundsChecking(Function &F, TargetLibraryInfo &TLI,
const DataLayout &DL = F.getParent()->getDataLayout();
ObjectSizeOpts EvalOpts;
EvalOpts.RoundToAlign = true;
+ EvalOpts.EvalMode = ObjectSizeOpts::Mode::ExactUnderlyingSizeAndOffset;
ObjectSizeOffsetEvaluator ObjSizeEval(DL, &TLI, F.getContext(), EvalOpts);
// check HANDLE_MEMORY_INST in include/llvm/Instruction.def for memory
diff --git a/llvm/test/Instrumentation/BoundsChecking/simple.ll b/llvm/test/Instrumentation/BoundsChecking/simple.ll
index a2af719d02516..57858618d17b3 100644
--- a/llvm/test/Instrumentation/BoundsChecking/simple.ll
+++ b/llvm/test/Instrumentation/BoundsChecking/simple.ll
@@ -330,7 +330,6 @@ alive:
}
; Check that merging sizes in a phi works.
-; FIXME: bounds-checking thinks that %alloc has an underlying size of 0.
define i8 @f14(i1 %i) {
; CHECK-LABEL: @f14(
; CHECK-NEXT: entry:
@@ -340,16 +339,18 @@ define i8 @f14(i1 %i) {
; CHECK-NEXT: [[G:%.*]] = getelementptr i8, ptr [[A]], i32 32
; CHECK-NEXT: br label [[BB2]]
; CHECK: bb2:
-; CHECK-NEXT: [[ALLOC:%.*]] = phi ptr [ null, [[ENTRY:%.*]] ], [ [[G]], [[BB1]] ]
+; CHECK-NEXT: [[TMP0:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ 32, [[BB1]] ]
+; CHECK-NEXT: [[TMP1:%.*]] = phi i64 [ 0, [[ENTRY]] ], [ 32, [[BB1]] ]
+; CHECK-NEXT: [[ALLOC:%.*]] = phi ptr [ null, [[ENTRY]] ], [ [[G]], [[BB1]] ]
; CHECK-NEXT: [[IND:%.*]] = phi i64 [ 0, [[ENTRY]] ], [ -4, [[BB1]] ]
-; CHECK-NEXT: [[TMP0:%.*]] = add i64 0, [[IND]]
+; CHECK-NEXT: [[TMP2:%.*]] = add i64 [[TMP1]], [[IND]]
; CHECK-NEXT: [[P:%.*]] = getelementptr i8, ptr [[ALLOC]], i64 [[IND]]
-; CHECK-NEXT: [[TMP1:%.*]] = sub i64 0, [[TMP0]]
-; CHECK-NEXT: [[TMP2:%.*]] = icmp ult i64 0, [[TMP0]]
-; CHECK-NEXT: [[TMP3:%.*]] = icmp ult i64 [[TMP1]], 1
-; CHECK-NEXT: [[TMP4:%.*]] = or i1 [[TMP2]], [[TMP3]]
-; CHECK-NEXT: br i1 [[TMP4]], label [[TRAP:%.*]], label [[TMP5:%.*]]
-; CHECK: 5:
+; CHECK-NEXT: [[TMP3:%.*]] = sub i64 [[TMP0]], [[TMP2]]
+; CHECK-NEXT: [[TMP4:%.*]] = icmp ult i64 [[TMP0]], [[TMP2]]
+; CHECK-NEXT: [[TMP5:%.*]] = icmp ult i64 [[TMP3]], 1
+; CHECK-NEXT: [[TMP6:%.*]] = or i1 [[TMP4]], [[TMP5]]
+; CHECK-NEXT: br i1 [[TMP6]], label [[TRAP:%.*]], label [[TMP7:%.*]]
+; CHECK: 7:
; CHECK-NEXT: [[RET:%.*]] = load i8, ptr [[P]], align 1
; CHECK-NEXT: ret i8 [[RET]]
; CHECK: trap:
More information about the llvm-commits
mailing list