Skip to content

Commit 1df2f52

Browse files
JustinStittJustinStitt
committed
implement wraps attribute
Signed-off-by: Justin Stitt <[email protected]>
1 parent 549fdda commit 1df2f52

20 files changed

+377
-26
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,16 @@ Attribute Changes in Clang
407407
(`OpenCL-C to Vulkan SPIR-V compiler <https://github.com/google/clspv>`_) to identify functions coming from libclc
408408
(`OpenCL-C builtin library <https://libclc.llvm.org>`_).
409409

410+
- Introduced ``__attribute((wraps))__`` which can be added to type or variable
411+
declarations. Using an attributed type or variable in an arithmetic
412+
expression will define the overflow behavior for that expression as having
413+
two's complement wrap-around. These expressions cannot trigger integer
414+
overflow warnings or sanitizer warnings. They also cannot be optimized away
415+
by some eager UB optimizations.
416+
417+
This attribute is only valid for C, as there are built-in language
418+
alternatives for other languages.
419+
410420
Improvements to Clang's diagnostics
411421
-----------------------------------
412422
- Clang now applies syntax highlighting to the code snippets it

‎clang/include/clang/AST/Expr.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4061,6 +4061,9 @@ class BinaryOperator : public Expr {
40614061
return getFPFeaturesInEffect(LO).getAllowFEnvAccess();
40624062
}
40634063

4064+
/// Does one of the subexpressions have the wraps attribute?
4065+
bool hasWrappingOperand(const ASTContext &Ctx) const;
4066+
40644067
protected:
40654068
BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs, Opcode opc,
40664069
QualType ResTy, ExprValueKind VK, ExprObjectKind OK,

‎clang/include/clang/AST/Type.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1441,6 +1441,8 @@ class QualType {
14411441
return getQualifiers().hasStrongOrWeakObjCLifetime();
14421442
}
14431443

1444+
bool hasWrapsAttr() const;
1445+
14441446
// true when Type is objc's weak and weak is enabled but ARC isn't.
14451447
bool isNonWeakInMRRWithObjCWeak(const ASTContext &Context) const;
14461448

‎clang/include/clang/Basic/Attr.td

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4595,3 +4595,10 @@ def ClspvLibclcBuiltin: InheritableAttr {
45954595
let Documentation = [ClspvLibclcBuiltinDoc];
45964596
let SimpleHandler = 1;
45974597
}
4598+
4599+
def Wraps : DeclOrTypeAttr {
4600+
let Spellings = [Clang<"wraps">];
4601+
let Subjects = SubjectList<[Var, TypedefName, Field]>;
4602+
let Documentation = [WrapsDocs];
4603+
let LangOpts = [COnly];
4604+
}

‎clang/include/clang/Basic/AttrDocs.td

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8106,3 +8106,72 @@ Attribute used by `clspv`_ (OpenCL-C to Vulkan SPIR-V compiler) to identify func
81068106
.. _`libclc`: https://libclc.llvm.org
81078107
}];
81088108
}
8109+
8110+
def WrapsDocs : Documentation {
8111+
let Category = DocCatField;
8112+
let Content = [{
8113+
This attribute can be used with type or variable declarations to denote that
8114+
arithmetic containing these marked components have defined overflow behavior.
8115+
Specifically, the behavior is defined as being consistent with two's complement
8116+
wrap-around. For the purposes of sanitizers or warnings that concern themselves
8117+
with the definedness of integer arithmetic, they will cease to instrument or
8118+
warn about arithmetic that directly involves a "wrapping" component.
8119+
8120+
For example, ``-fsanitize=signed-integer-overflow`` or ``-Winteger-overflow``
8121+
will not warn about suspicious overflowing arithmetic -- assuming correct usage
8122+
of the wraps attribute.
8123+
8124+
This example shows some basic usage of ``__attribute__((wraps))`` on a type
8125+
definition when building with ``-fsanitize=signed-integer-overflow``
8126+
8127+
.. code-block:: c
8128+
8129+
typedef int __attribute__((wraps)) wrapping_int;
8130+
8131+
void foo() {
8132+
wrapping_int a = INT_MAX;
8133+
++a; // no sanitizer warning
8134+
}
8135+
8136+
int main() { foo(); }
8137+
8138+
In the following example, we use ``__attribute__((wraps))`` on a variable to
8139+
disable overflow instrumentation for arithmetic expressions it appears in. We
8140+
do so with a popular overflow-checking pattern which we might not want to trip
8141+
sanitizers (like ``-fsanitize=unsigned-integer-overflow``).
8142+
8143+
.. code-block:: c
8144+
8145+
void foo(int offset) {
8146+
unsigned int A __attribute__((wraps)) = UINT_MAX;
8147+
8148+
// to check for overflow using this pattern, we may perform a real overflow
8149+
// thus triggering sanitizers to step in. Since A is "wrapping", we can be
8150+
// sure there are no sanitizer warnings.
8151+
if (A + offset < A) {
8152+
// handle overflow manually
8153+
// ...
8154+
return;
8155+
}
8156+
8157+
// now, handle non-overflow case ...
8158+
}
8159+
8160+
The above example demonstrates some of the power and elegance this attribute
8161+
provides. We can use code patterns we are already familiar with (like ``if (x +
8162+
y < x)``) while gaining control over the overflow behavior on a case-by-case
8163+
basis.
8164+
8165+
When combined with ``-fwrapv``, this attribute can still be applied as normal
8166+
but has no function apart from annotating types and variables for readers. This
8167+
is because ``-fwrapv`` defines all arithmetic as being "wrapping", rending this
8168+
attribute's efforts redundant.
8169+
8170+
When using this attribute without ``-fwrapv`` and without any sanitizers, it
8171+
still has an impact on the definedness of arithmetic expressions containing
8172+
wrapping components. Since the behavior of said expressions is now technically
8173+
defined, the compiler will forgo some eager optimizations that are used on
8174+
expressions containing UB.
8175+
}];
8176+
}
8177+

‎clang/include/clang/Basic/DiagnosticGroups.td

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1530,3 +1530,9 @@ def BitIntExtension : DiagGroup<"bit-int-extension">;
15301530

15311531
// Warnings about misuse of ExtractAPI options.
15321532
def ExtractAPIMisuse : DiagGroup<"extractapi-misuse">;
1533+
1534+
// Warnings regarding the usage of __attribute__((wraps)) on non-integer types.
1535+
def UselessWrapsAttr : DiagGroup<"useless-wraps-attribute">;
1536+
1537+
// Warnings about the wraps attribute getting implicitly discarded
1538+
def ImpDiscardedWrapsAttr : DiagGroup<"implicitly-discarded-wraps-attribute">;

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6561,6 +6561,13 @@ def err_counted_by_attr_refer_to_union : Error<
65616561
def note_flexible_array_counted_by_attr_field : Note<
65626562
"field %0 declared here">;
65636563

6564+
def warn_wraps_attr_var_decl_type_not_integer : Warning<
6565+
"using attribute 'wraps' with non-integer type '%0' has no function">,
6566+
InGroup<UselessWrapsAttr>;
6567+
def warn_wraps_attr_maybe_lost : Warning<
6568+
"'wraps' attribute may be implicitly discarded when converted to %0">,
6569+
InGroup<ImpDiscardedWrapsAttr>;
6570+
65646571
let CategoryName = "ARC Semantic Issue" in {
65656572

65666573
// ARC-mode diagnostics.

‎clang/lib/AST/Expr.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2237,6 +2237,11 @@ bool BinaryOperator::isNullPointerArithmeticExtension(ASTContext &Ctx,
22372237
return true;
22382238
}
22392239

2240+
bool BinaryOperator::hasWrappingOperand(const ASTContext &Ctx) const {
2241+
return getLHS()->getType().hasWrapsAttr() ||
2242+
getRHS()->getType().hasWrapsAttr();
2243+
}
2244+
22402245
SourceLocExpr::SourceLocExpr(const ASTContext &Ctx, SourceLocIdentKind Kind,
22412246
QualType ResultTy, SourceLocation BLoc,
22422247
SourceLocation RParenLoc,
@@ -4756,6 +4761,8 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
47564761
if (hasStoredFPFeatures())
47574762
setStoredFPFeatures(FPFeatures);
47584763
setDependence(computeDependence(this));
4764+
if (hasWrappingOperand(Ctx))
4765+
setType(Ctx.getAttributedType(attr::Wraps, getType(), getType()));
47594766
}
47604767

47614768
BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
@@ -4773,6 +4780,8 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
47734780
if (hasStoredFPFeatures())
47744781
setStoredFPFeatures(FPFeatures);
47754782
setDependence(computeDependence(this));
4783+
if (hasWrappingOperand(Ctx))
4784+
setType(Ctx.getAttributedType(attr::Wraps, getType(), getType()));
47764785
}
47774786

47784787
BinaryOperator *BinaryOperator::CreateEmpty(const ASTContext &C,

‎clang/lib/AST/ExprConstant.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2779,7 +2779,7 @@ static bool CheckedIntArithmetic(EvalInfo &Info, const Expr *E,
27792779
APSInt Value(Op(LHS.extend(BitWidth), RHS.extend(BitWidth)), false);
27802780
Result = Value.trunc(LHS.getBitWidth());
27812781
if (Result.extend(BitWidth) != Value) {
2782-
if (Info.checkingForUndefinedBehavior())
2782+
if (Info.checkingForUndefinedBehavior() && !E->getType().hasWrapsAttr())
27832783
Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
27842784
diag::warn_integer_constant_overflow)
27852785
<< toString(Result, 10, Result.isSigned(), /*formatAsCLiteral=*/false,
@@ -14117,7 +14117,7 @@ bool IntExprEvaluator::VisitUnaryOperator(const UnaryOperator *E) {
1411714117
if (!Result.isInt()) return Error(E);
1411814118
const APSInt &Value = Result.getInt();
1411914119
if (Value.isSigned() && Value.isMinSignedValue() && E->canOverflow()) {
14120-
if (Info.checkingForUndefinedBehavior())
14120+
if (Info.checkingForUndefinedBehavior() && !E->getType().hasWrapsAttr())
1412114121
Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
1412214122
diag::warn_integer_constant_overflow)
1412314123
<< toString(Value, 10, Value.isSigned(), /*formatAsCLiteral=*/false,

‎clang/lib/AST/Type.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2820,6 +2820,10 @@ bool QualType::isTriviallyEqualityComparableType(
28202820
CanonicalType, /*CheckIfTriviallyCopyable=*/false);
28212821
}
28222822

2823+
bool QualType::hasWrapsAttr() const {
2824+
return !isNull() && getTypePtr()->hasAttr(attr::Wraps);
2825+
}
2826+
28232827
bool QualType::isNonWeakInMRRWithObjCWeak(const ASTContext &Context) const {
28242828
return !Context.getLangOpts().ObjCAutoRefCount &&
28252829
Context.getLangOpts().ObjCWeak &&

‎clang/lib/AST/TypePrinter.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1962,6 +1962,9 @@ void TypePrinter::printAttributedAfter(const AttributedType *T,
19621962
case attr::AArch64SVEPcs: OS << "aarch64_sve_pcs"; break;
19631963
case attr::AMDGPUKernelCall: OS << "amdgpu_kernel"; break;
19641964
case attr::IntelOclBicc: OS << "inteloclbicc"; break;
1965+
case attr::Wraps:
1966+
OS << "wraps";
1967+
break;
19651968
case attr::PreserveMost:
19661969
OS << "preserve_most";
19671970
break;

‎clang/lib/CodeGen/CGExprScalar.cpp

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,10 @@ struct BinOpInfo {
156156
}
157157
return false;
158158
}
159+
160+
/// Does the BinaryOperator have the wraps attribute?
161+
/// If so, we can elide overflow sanitizer checks.
162+
bool hasWrappingOperand() const { return E->getType().hasWrapsAttr(); }
159163
};
160164

161165
static bool MustVisitNullValue(const Expr *E) {
@@ -735,7 +739,8 @@ class ScalarExprEmitter
735739

736740
// Binary Operators.
737741
Value *EmitMul(const BinOpInfo &Ops) {
738-
if (Ops.Ty->isSignedIntegerOrEnumerationType()) {
742+
if (Ops.Ty->isSignedIntegerOrEnumerationType() &&
743+
!Ops.hasWrappingOperand()) {
739744
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
740745
case LangOptions::SOB_Defined:
741746
if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
@@ -771,7 +776,8 @@ class ScalarExprEmitter
771776

772777
if (Ops.Ty->isUnsignedIntegerType() &&
773778
CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) &&
774-
!CanElideOverflowCheck(CGF.getContext(), Ops))
779+
!CanElideOverflowCheck(CGF.getContext(), Ops) &&
780+
!Ops.hasWrappingOperand())
775781
return EmitOverflowCheckedBinOp(Ops);
776782

777783
if (Ops.LHS->getType()->isFPOrFPVectorTy()) {
@@ -1103,7 +1109,7 @@ void ScalarExprEmitter::EmitIntegerTruncationCheck(Value *Src, QualType SrcType,
11031109
// If the comparison result is 'i1 false', then the truncation was lossy.
11041110

11051111
// Do we care about this type of truncation?
1106-
if (!CGF.SanOpts.has(Check.second.second))
1112+
if (!CGF.SanOpts.has(Check.second.second) || DstType.hasWrapsAttr())
11071113
return;
11081114

11091115
llvm::Constant *StaticArgs[] = {
@@ -1336,6 +1342,14 @@ void CodeGenFunction::EmitBitfieldConversionCheck(Value *Src, QualType SrcType,
13361342
bool SrcSigned = SrcType->isSignedIntegerOrEnumerationType();
13371343
bool DstSigned = DstType->isSignedIntegerOrEnumerationType();
13381344

1345+
bool SrcWraps = SrcType.hasWrapsAttr();
1346+
bool DstWraps = DstType.hasWrapsAttr();
1347+
1348+
// The wraps attribute should silence any sanitizer warnings
1349+
// regarding truncation or overflow
1350+
if (SrcWraps || DstWraps)
1351+
return;
1352+
13391353
CodeGenFunction::SanitizerScope SanScope(this);
13401354

13411355
std::pair<ScalarExprEmitter::ImplicitConversionCheckKind,
@@ -2844,6 +2858,9 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV,
28442858
} else if (type->isIntegerType()) {
28452859
QualType promotedType;
28462860
bool canPerformLossyDemotionCheck = false;
2861+
BinOpInfo Ops = createBinOpInfoFromIncDec(
2862+
E, value, isInc, E->getFPFeaturesInEffect(CGF.getLangOpts()));
2863+
28472864
if (CGF.getContext().isPromotableIntegerType(type)) {
28482865
promotedType = CGF.getContext().getPromotedIntegerType(type);
28492866
assert(promotedType != type && "Shouldn't promote to the same type.");
@@ -2900,10 +2917,12 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV,
29002917
// Note that signed integer inc/dec with width less than int can't
29012918
// overflow because of promotion rules; we're just eliding a few steps
29022919
// here.
2903-
} else if (E->canOverflow() && type->isSignedIntegerOrEnumerationType()) {
2920+
} else if (E->canOverflow() && type->isSignedIntegerOrEnumerationType() &&
2921+
!Ops.hasWrappingOperand()) {
29042922
value = EmitIncDecConsiderOverflowBehavior(E, value, isInc);
29052923
} else if (E->canOverflow() && type->isUnsignedIntegerType() &&
2906-
CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow)) {
2924+
CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) &&
2925+
!Ops.hasWrappingOperand()) {
29072926
value = EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(
29082927
E, value, isInc, E->getFPFeaturesInEffect(CGF.getLangOpts())));
29092928
} else {
@@ -3692,7 +3711,8 @@ Value *ScalarExprEmitter::EmitDiv(const BinOpInfo &Ops) {
36923711
if ((CGF.SanOpts.has(SanitizerKind::IntegerDivideByZero) ||
36933712
CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) &&
36943713
Ops.Ty->isIntegerType() &&
3695-
(Ops.mayHaveIntegerDivisionByZero() || Ops.mayHaveIntegerOverflow())) {
3714+
(Ops.mayHaveIntegerDivisionByZero() || Ops.mayHaveIntegerOverflow()) &&
3715+
!Ops.hasWrappingOperand()) {
36963716
llvm::Value *Zero = llvm::Constant::getNullValue(ConvertType(Ops.Ty));
36973717
EmitUndefinedBehaviorIntegerDivAndRemCheck(Ops, Zero, true);
36983718
} else if (CGF.SanOpts.has(SanitizerKind::FloatDivideByZero) &&
@@ -3741,7 +3761,8 @@ Value *ScalarExprEmitter::EmitRem(const BinOpInfo &Ops) {
37413761
if ((CGF.SanOpts.has(SanitizerKind::IntegerDivideByZero) ||
37423762
CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) &&
37433763
Ops.Ty->isIntegerType() &&
3744-
(Ops.mayHaveIntegerDivisionByZero() || Ops.mayHaveIntegerOverflow())) {
3764+
(Ops.mayHaveIntegerDivisionByZero() || Ops.mayHaveIntegerOverflow()) &&
3765+
!Ops.hasWrappingOperand()) {
37453766
CodeGenFunction::SanitizerScope SanScope(&CGF);
37463767
llvm::Value *Zero = llvm::Constant::getNullValue(ConvertType(Ops.Ty));
37473768
EmitUndefinedBehaviorIntegerDivAndRemCheck(Ops, Zero, false);
@@ -4106,7 +4127,7 @@ Value *ScalarExprEmitter::EmitAdd(const BinOpInfo &op) {
41064127
op.RHS->getType()->isPointerTy())
41074128
return emitPointerArithmetic(CGF, op, CodeGenFunction::NotSubtraction);
41084129

4109-
if (op.Ty->isSignedIntegerOrEnumerationType()) {
4130+
if (op.Ty->isSignedIntegerOrEnumerationType() && !op.hasWrappingOperand()) {
41104131
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
41114132
case LangOptions::SOB_Defined:
41124133
if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
@@ -4139,7 +4160,7 @@ Value *ScalarExprEmitter::EmitAdd(const BinOpInfo &op) {
41394160

41404161
if (op.Ty->isUnsignedIntegerType() &&
41414162
CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) &&
4142-
!CanElideOverflowCheck(CGF.getContext(), op))
4163+
!CanElideOverflowCheck(CGF.getContext(), op) && !op.hasWrappingOperand())
41434164
return EmitOverflowCheckedBinOp(op);
41444165

41454166
if (op.LHS->getType()->isFPOrFPVectorTy()) {
@@ -4262,7 +4283,7 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) {
42624283
Value *ScalarExprEmitter::EmitSub(const BinOpInfo &op) {
42634284
// The LHS is always a pointer if either side is.
42644285
if (!op.LHS->getType()->isPointerTy()) {
4265-
if (op.Ty->isSignedIntegerOrEnumerationType()) {
4286+
if (op.Ty->isSignedIntegerOrEnumerationType() && !op.hasWrappingOperand()) {
42664287
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
42674288
case LangOptions::SOB_Defined:
42684289
if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
@@ -4295,7 +4316,8 @@ Value *ScalarExprEmitter::EmitSub(const BinOpInfo &op) {
42954316

42964317
if (op.Ty->isUnsignedIntegerType() &&
42974318
CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) &&
4298-
!CanElideOverflowCheck(CGF.getContext(), op))
4319+
!CanElideOverflowCheck(CGF.getContext(), op) &&
4320+
!op.hasWrappingOperand())
42994321
return EmitOverflowCheckedBinOp(op);
43004322

43014323
if (op.LHS->getType()->isFPOrFPVectorTy()) {
@@ -4415,7 +4437,8 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) {
44154437
bool SanitizeSignedBase = CGF.SanOpts.has(SanitizerKind::ShiftBase) &&
44164438
Ops.Ty->hasSignedIntegerRepresentation() &&
44174439
!CGF.getLangOpts().isSignedOverflowDefined() &&
4418-
!CGF.getLangOpts().CPlusPlus20;
4440+
!CGF.getLangOpts().CPlusPlus20 &&
4441+
!Ops.hasWrappingOperand();
44194442
bool SanitizeUnsignedBase =
44204443
CGF.SanOpts.has(SanitizerKind::UnsignedShiftBase) &&
44214444
Ops.Ty->hasUnsignedIntegerRepresentation();

‎clang/lib/Sema/Sema.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,9 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty,
675675
QualType ExprTy = Context.getCanonicalType(E->getType());
676676
QualType TypeTy = Context.getCanonicalType(Ty);
677677

678+
if (E->getType().getTypePtr()->isIntegerType() && E->getType().hasWrapsAttr())
679+
Ty = Context.getAttributedType(attr::Wraps, Ty, Ty);
680+
678681
if (ExprTy == TypeTy)
679682
return E;
680683

0 commit comments

Comments
 (0)