Skip to content

Commit e529f28

Browse files
committed
[BoundsSafety] Optimize BS_CHK_AccessSize for the case element type is 1 byte
Previously for something like ``` T* __bidi_indexable ptr; ptr[idx]; ``` for the upper bound check (when BS_CHK_AccessSize is active) we always checked 1. `&ptr[idx] + sizeof(T)` does not overflow 2. `&ptr[idx] + sizeof(T) <= ptr.upper_bound` However, for the case `sizeof(T) == 1` we can actually just check `&ptr[idx] < ptr.upper_bound` and completely avoid performing the overflow check. This gives us a code size and runtime overhead win in this particular case. We can use Z3 to prove the two different bounds checks implementations are identical when the access size is 1 using the SMT-LIBv2 file shown below: ``` (set-logic QF_BV) (declare-const cur (_ BitVec 64)) (declare-const upper (_ BitVec 64)) (declare-const access_size (_ BitVec 64)) ;;; for inspecting model (declare-const overflows Bool) (declare-const cur_plus_access_size (_ BitVec 64)) (define-fun add_overflows ((a (_ BitVec 64)) (b (_ BitVec 64))) Bool (bvult (bvadd a b) a ) ) ;;; Access size is not zero (assert (not (= access_size #x0000000000000000)) ) ;;; `overflows` is a convenience variable for inspecting the model (assert (= overflows (add_overflows cur access_size) ) ) ;;; `cur_plus_access_size` is a convenience variable for inspecting the model (assert (= cur_plus_access_size (bvadd cur access_size) ) ) ;;; Consider the special case with access size of 1 (assert (= access_size #x0000000000000001) ) ;;;; New upper bound check semantics (define-fun new_in_bounds () Bool (and (not (add_overflows cur access_size)) (bvule (bvadd cur access_size) upper ) ) ) ;;;; Old upper bound check semantics (define-fun old_in_bounds () Bool (and (bvult cur upper ) ) ) (assert (not (= new_in_bounds old_in_bounds ) ) ) (check-sat); reports unsat (get-model) (get-value (old_in_bounds)) (get-value (new_in_bounds)) (get-value (cur_plus_access_size)) (get-value (overflows)) ``` rdar://139665133 (cherry picked from commit 84b0262) Conflicts: clang/test/BoundsSafety/CodeGen/range-check-optimizations.c
1 parent 3c84c55 commit e529f28

File tree

3 files changed

+349
-39
lines changed

3 files changed

+349
-39
lines changed

clang/lib/CodeGen/CGExpr.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,9 @@ void CodeGenFunction::EmitBoundsSafetyBoundsCheck(
126126

127127
if (Upper) {
128128
if (getLangOpts().hasNewBoundsSafetyCheck(LangOptions::BS_CHK_AccessSize) &&
129-
ElemTy->isSized()) {
130-
// For sized types take the size of a potential access into account.
131-
// For something like:
129+
ElemTy->isSized() && CGM.getDataLayout().getTypeStoreSize(ElemTy) > 1) {
130+
// For sized types larger than 1 byte take the size of a potential access
131+
// into account. For something like:
132132
//
133133
// T* ptr;
134134
//
@@ -151,15 +151,24 @@ void CodeGenFunction::EmitBoundsSafetyBoundsCheck(
151151
EmitBoundsSafetyTrapCheck(Builder.CreateICmpULE(Ptr, OnePastTheEndPtr),
152152
BNS_TRAP_PTR_GT_UPPER_BOUND, TrapCtx);
153153
} else {
154-
// Legacy path and path for unsized types (e.g. function types).
154+
// Path where the size of the access is assumed to be 1 byte. This is used
155+
// for
156+
//
157+
// * 1-byte access when `BS_CHK_AccessSize` enabled.
158+
// * unsized types (e.g. function types)
159+
// * Legacy bounds checks (i.e. `BS_CHK_AccessSize` is disabled).
160+
//
155161
// In this case for something like
156162
//
157163
// T* ptr;
158164
//
159165
// We assume that `sizeof(T)` is 1. In this case
160166
//
161-
// `ptr.ptr + sizeof(T) <= ptr.upper` simplifies to
167+
// 1. The `ptr.ptr + 1` overflow check is unnecessary and so we skip
168+
// emitting it.
169+
// 2. The `ptr.ptr + sizeof(T) <= ptr.upper` check simplifies to
162170
// `ptr.ptr < ptr.upper`.
171+
//
163172
BoundsSafetyOptRemarkScope Scope(this, BNS_OR_PTR_GE_UPPER_BOUND);
164173
llvm::Value *Check = Builder.CreateICmpULT(Ptr, Upper);
165174
EmitBoundsSafetyTrapCheck(Check, BNS_TRAP_PTR_GE_UPPER_BOUND, TrapCtx);

0 commit comments

Comments
 (0)