https://github.com/JustinStitt updated https://github.com/llvm/llvm-project/pull/86618
>From 1df2f520f6a8ab0e45b80f7a1d680d34f8ab37c9 Mon Sep 17 00:00:00 2001 From: Justin Stitt <justinst...@google.com> Date: Tue, 5 Mar 2024 03:14:49 +0000 Subject: [PATCH] implement wraps attribute Signed-off-by: Justin Stitt <justinst...@google.com> --- clang/docs/ReleaseNotes.rst | 10 +++ clang/include/clang/AST/Expr.h | 3 + clang/include/clang/AST/Type.h | 2 + clang/include/clang/Basic/Attr.td | 7 ++ clang/include/clang/Basic/AttrDocs.td | 69 +++++++++++++++++++ clang/include/clang/Basic/DiagnosticGroups.td | 6 ++ .../clang/Basic/DiagnosticSemaKinds.td | 7 ++ clang/lib/AST/Expr.cpp | 9 +++ clang/lib/AST/ExprConstant.cpp | 4 +- clang/lib/AST/Type.cpp | 4 ++ clang/lib/AST/TypePrinter.cpp | 3 + clang/lib/CodeGen/CGExprScalar.cpp | 47 +++++++++---- clang/lib/Sema/Sema.cpp | 3 + clang/lib/Sema/SemaChecking.cpp | 33 ++++++++- clang/lib/Sema/SemaDeclAttr.cpp | 8 ++- clang/lib/Sema/SemaType.cpp | 15 ++++ clang/test/CodeGen/integer-overflow.c | 66 ++++++++++++++++++ clang/test/CodeGen/unsigned-overflow.c | 63 ++++++++++++++--- ...a-attribute-supported-attributes-list.test | 1 + clang/test/Sema/attr-wraps.c | 43 ++++++++++++ 20 files changed, 377 insertions(+), 26 deletions(-) create mode 100644 clang/test/Sema/attr-wraps.c diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index ba4637d98197f..a1fc18523936f 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -407,6 +407,16 @@ Attribute Changes in Clang (`OpenCL-C to Vulkan SPIR-V compiler <https://github.com/google/clspv>`_) to identify functions coming from libclc (`OpenCL-C builtin library <https://libclc.llvm.org>`_). +- Introduced ``__attribute((wraps))__`` which can be added to type or variable + declarations. Using an attributed type or variable in an arithmetic + expression will define the overflow behavior for that expression as having + two's complement wrap-around. These expressions cannot trigger integer + overflow warnings or sanitizer warnings. They also cannot be optimized away + by some eager UB optimizations. + + This attribute is only valid for C, as there are built-in language + alternatives for other languages. + Improvements to Clang's diagnostics ----------------------------------- - Clang now applies syntax highlighting to the code snippets it diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index f2bf667636dc9..48968cbbbaf7e 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -4061,6 +4061,9 @@ class BinaryOperator : public Expr { return getFPFeaturesInEffect(LO).getAllowFEnvAccess(); } + /// Does one of the subexpressions have the wraps attribute? + bool hasWrappingOperand(const ASTContext &Ctx) const; + protected: BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs, Opcode opc, QualType ResTy, ExprValueKind VK, ExprObjectKind OK, diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index da3834f19ca04..641f943205891 100644 --- a/clang/include/clang/AST/Type.h +++ b/clang/include/clang/AST/Type.h @@ -1441,6 +1441,8 @@ class QualType { return getQualifiers().hasStrongOrWeakObjCLifetime(); } + bool hasWrapsAttr() const; + // true when Type is objc's weak and weak is enabled but ARC isn't. bool isNonWeakInMRRWithObjCWeak(const ASTContext &Context) const; diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 7008bea483c87..922dcba29a8de 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -4595,3 +4595,10 @@ def ClspvLibclcBuiltin: InheritableAttr { let Documentation = [ClspvLibclcBuiltinDoc]; let SimpleHandler = 1; } + +def Wraps : DeclOrTypeAttr { + let Spellings = [Clang<"wraps">]; + let Subjects = SubjectList<[Var, TypedefName, Field]>; + let Documentation = [WrapsDocs]; + let LangOpts = [COnly]; +} diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 54197d588eb45..dc050e616d63a 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -8106,3 +8106,72 @@ Attribute used by `clspv`_ (OpenCL-C to Vulkan SPIR-V compiler) to identify func .. _`libclc`: https://libclc.llvm.org }]; } + +def WrapsDocs : Documentation { + let Category = DocCatField; + let Content = [{ +This attribute can be used with type or variable declarations to denote that +arithmetic containing these marked components have defined overflow behavior. +Specifically, the behavior is defined as being consistent with two's complement +wrap-around. For the purposes of sanitizers or warnings that concern themselves +with the definedness of integer arithmetic, they will cease to instrument or +warn about arithmetic that directly involves a "wrapping" component. + +For example, ``-fsanitize=signed-integer-overflow`` or ``-Winteger-overflow`` +will not warn about suspicious overflowing arithmetic -- assuming correct usage +of the wraps attribute. + +This example shows some basic usage of ``__attribute__((wraps))`` on a type +definition when building with ``-fsanitize=signed-integer-overflow`` + +.. code-block:: c + + typedef int __attribute__((wraps)) wrapping_int; + + void foo() { + wrapping_int a = INT_MAX; + ++a; // no sanitizer warning + } + + int main() { foo(); } + +In the following example, we use ``__attribute__((wraps))`` on a variable to +disable overflow instrumentation for arithmetic expressions it appears in. We +do so with a popular overflow-checking pattern which we might not want to trip +sanitizers (like ``-fsanitize=unsigned-integer-overflow``). + +.. code-block:: c + + void foo(int offset) { + unsigned int A __attribute__((wraps)) = UINT_MAX; + + // to check for overflow using this pattern, we may perform a real overflow + // thus triggering sanitizers to step in. Since A is "wrapping", we can be + // sure there are no sanitizer warnings. + if (A + offset < A) { + // handle overflow manually + // ... + return; + } + + // now, handle non-overflow case ... + } + +The above example demonstrates some of the power and elegance this attribute +provides. We can use code patterns we are already familiar with (like ``if (x + +y < x)``) while gaining control over the overflow behavior on a case-by-case +basis. + +When combined with ``-fwrapv``, this attribute can still be applied as normal +but has no function apart from annotating types and variables for readers. This +is because ``-fwrapv`` defines all arithmetic as being "wrapping", rending this +attribute's efforts redundant. + +When using this attribute without ``-fwrapv`` and without any sanitizers, it +still has an impact on the definedness of arithmetic expressions containing +wrapping components. Since the behavior of said expressions is now technically +defined, the compiler will forgo some eager optimizations that are used on +expressions containing UB. +}]; +} + diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 4cb4f3d999f7a..6dd880dd8f717 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1530,3 +1530,9 @@ def BitIntExtension : DiagGroup<"bit-int-extension">; // Warnings about misuse of ExtractAPI options. def ExtractAPIMisuse : DiagGroup<"extractapi-misuse">; + +// Warnings regarding the usage of __attribute__((wraps)) on non-integer types. +def UselessWrapsAttr : DiagGroup<"useless-wraps-attribute">; + +// Warnings about the wraps attribute getting implicitly discarded +def ImpDiscardedWrapsAttr : DiagGroup<"implicitly-discarded-wraps-attribute">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index e3b4186f1b06f..8530f449b654d 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6561,6 +6561,13 @@ def err_counted_by_attr_refer_to_union : Error< def note_flexible_array_counted_by_attr_field : Note< "field %0 declared here">; +def warn_wraps_attr_var_decl_type_not_integer : Warning< + "using attribute 'wraps' with non-integer type '%0' has no function">, + InGroup<UselessWrapsAttr>; +def warn_wraps_attr_maybe_lost : Warning< + "'wraps' attribute may be implicitly discarded when converted to %0">, + InGroup<ImpDiscardedWrapsAttr>; + let CategoryName = "ARC Semantic Issue" in { // ARC-mode diagnostics. diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index ac0b1b38f0162..2fb82ed0e1306 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -2237,6 +2237,11 @@ bool BinaryOperator::isNullPointerArithmeticExtension(ASTContext &Ctx, return true; } +bool BinaryOperator::hasWrappingOperand(const ASTContext &Ctx) const { + return getLHS()->getType().hasWrapsAttr() || + getRHS()->getType().hasWrapsAttr(); +} + SourceLocExpr::SourceLocExpr(const ASTContext &Ctx, SourceLocIdentKind Kind, QualType ResultTy, SourceLocation BLoc, SourceLocation RParenLoc, @@ -4756,6 +4761,8 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs, if (hasStoredFPFeatures()) setStoredFPFeatures(FPFeatures); setDependence(computeDependence(this)); + if (hasWrappingOperand(Ctx)) + setType(Ctx.getAttributedType(attr::Wraps, getType(), getType())); } BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs, @@ -4773,6 +4780,8 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs, if (hasStoredFPFeatures()) setStoredFPFeatures(FPFeatures); setDependence(computeDependence(this)); + if (hasWrappingOperand(Ctx)) + setType(Ctx.getAttributedType(attr::Wraps, getType(), getType())); } BinaryOperator *BinaryOperator::CreateEmpty(const ASTContext &C, diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index f1aa19e4409e1..795f6c82bc069 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -2779,7 +2779,7 @@ static bool CheckedIntArithmetic(EvalInfo &Info, const Expr *E, APSInt Value(Op(LHS.extend(BitWidth), RHS.extend(BitWidth)), false); Result = Value.trunc(LHS.getBitWidth()); if (Result.extend(BitWidth) != Value) { - if (Info.checkingForUndefinedBehavior()) + if (Info.checkingForUndefinedBehavior() && !E->getType().hasWrapsAttr()) Info.Ctx.getDiagnostics().Report(E->getExprLoc(), diag::warn_integer_constant_overflow) << toString(Result, 10, Result.isSigned(), /*formatAsCLiteral=*/false, @@ -14117,7 +14117,7 @@ bool IntExprEvaluator::VisitUnaryOperator(const UnaryOperator *E) { if (!Result.isInt()) return Error(E); const APSInt &Value = Result.getInt(); if (Value.isSigned() && Value.isMinSignedValue() && E->canOverflow()) { - if (Info.checkingForUndefinedBehavior()) + if (Info.checkingForUndefinedBehavior() && !E->getType().hasWrapsAttr()) Info.Ctx.getDiagnostics().Report(E->getExprLoc(), diag::warn_integer_constant_overflow) << toString(Value, 10, Value.isSigned(), /*formatAsCLiteral=*/false, diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp index e31741cd44240..737723f2bfcfa 100644 --- a/clang/lib/AST/Type.cpp +++ b/clang/lib/AST/Type.cpp @@ -2820,6 +2820,10 @@ bool QualType::isTriviallyEqualityComparableType( CanonicalType, /*CheckIfTriviallyCopyable=*/false); } +bool QualType::hasWrapsAttr() const { + return !isNull() && getTypePtr()->hasAttr(attr::Wraps); +} + bool QualType::isNonWeakInMRRWithObjCWeak(const ASTContext &Context) const { return !Context.getLangOpts().ObjCAutoRefCount && Context.getLangOpts().ObjCWeak && diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp index 87f0a8728d850..6e10aa1c7bb44 100644 --- a/clang/lib/AST/TypePrinter.cpp +++ b/clang/lib/AST/TypePrinter.cpp @@ -1962,6 +1962,9 @@ void TypePrinter::printAttributedAfter(const AttributedType *T, case attr::AArch64SVEPcs: OS << "aarch64_sve_pcs"; break; case attr::AMDGPUKernelCall: OS << "amdgpu_kernel"; break; case attr::IntelOclBicc: OS << "inteloclbicc"; break; + case attr::Wraps: + OS << "wraps"; + break; case attr::PreserveMost: OS << "preserve_most"; break; diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 1b144c178ce96..a62904a0d7d49 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -156,6 +156,10 @@ struct BinOpInfo { } return false; } + + /// Does the BinaryOperator have the wraps attribute? + /// If so, we can elide overflow sanitizer checks. + bool hasWrappingOperand() const { return E->getType().hasWrapsAttr(); } }; static bool MustVisitNullValue(const Expr *E) { @@ -735,7 +739,8 @@ class ScalarExprEmitter // Binary Operators. Value *EmitMul(const BinOpInfo &Ops) { - if (Ops.Ty->isSignedIntegerOrEnumerationType()) { + if (Ops.Ty->isSignedIntegerOrEnumerationType() && + !Ops.hasWrappingOperand()) { switch (CGF.getLangOpts().getSignedOverflowBehavior()) { case LangOptions::SOB_Defined: if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) @@ -771,7 +776,8 @@ class ScalarExprEmitter if (Ops.Ty->isUnsignedIntegerType() && CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) && - !CanElideOverflowCheck(CGF.getContext(), Ops)) + !CanElideOverflowCheck(CGF.getContext(), Ops) && + !Ops.hasWrappingOperand()) return EmitOverflowCheckedBinOp(Ops); if (Ops.LHS->getType()->isFPOrFPVectorTy()) { @@ -1103,7 +1109,7 @@ void ScalarExprEmitter::EmitIntegerTruncationCheck(Value *Src, QualType SrcType, // If the comparison result is 'i1 false', then the truncation was lossy. // Do we care about this type of truncation? - if (!CGF.SanOpts.has(Check.second.second)) + if (!CGF.SanOpts.has(Check.second.second) || DstType.hasWrapsAttr()) return; llvm::Constant *StaticArgs[] = { @@ -1336,6 +1342,14 @@ void CodeGenFunction::EmitBitfieldConversionCheck(Value *Src, QualType SrcType, bool SrcSigned = SrcType->isSignedIntegerOrEnumerationType(); bool DstSigned = DstType->isSignedIntegerOrEnumerationType(); + bool SrcWraps = SrcType.hasWrapsAttr(); + bool DstWraps = DstType.hasWrapsAttr(); + + // The wraps attribute should silence any sanitizer warnings + // regarding truncation or overflow + if (SrcWraps || DstWraps) + return; + CodeGenFunction::SanitizerScope SanScope(this); std::pair<ScalarExprEmitter::ImplicitConversionCheckKind, @@ -2844,6 +2858,9 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV, } else if (type->isIntegerType()) { QualType promotedType; bool canPerformLossyDemotionCheck = false; + BinOpInfo Ops = createBinOpInfoFromIncDec( + E, value, isInc, E->getFPFeaturesInEffect(CGF.getLangOpts())); + if (CGF.getContext().isPromotableIntegerType(type)) { promotedType = CGF.getContext().getPromotedIntegerType(type); assert(promotedType != type && "Shouldn't promote to the same type."); @@ -2900,10 +2917,12 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV, // Note that signed integer inc/dec with width less than int can't // overflow because of promotion rules; we're just eliding a few steps // here. - } else if (E->canOverflow() && type->isSignedIntegerOrEnumerationType()) { + } else if (E->canOverflow() && type->isSignedIntegerOrEnumerationType() && + !Ops.hasWrappingOperand()) { value = EmitIncDecConsiderOverflowBehavior(E, value, isInc); } else if (E->canOverflow() && type->isUnsignedIntegerType() && - CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow)) { + CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) && + !Ops.hasWrappingOperand()) { value = EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec( E, value, isInc, E->getFPFeaturesInEffect(CGF.getLangOpts()))); } else { @@ -3692,7 +3711,8 @@ Value *ScalarExprEmitter::EmitDiv(const BinOpInfo &Ops) { if ((CGF.SanOpts.has(SanitizerKind::IntegerDivideByZero) || CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) && Ops.Ty->isIntegerType() && - (Ops.mayHaveIntegerDivisionByZero() || Ops.mayHaveIntegerOverflow())) { + (Ops.mayHaveIntegerDivisionByZero() || Ops.mayHaveIntegerOverflow()) && + !Ops.hasWrappingOperand()) { llvm::Value *Zero = llvm::Constant::getNullValue(ConvertType(Ops.Ty)); EmitUndefinedBehaviorIntegerDivAndRemCheck(Ops, Zero, true); } else if (CGF.SanOpts.has(SanitizerKind::FloatDivideByZero) && @@ -3741,7 +3761,8 @@ Value *ScalarExprEmitter::EmitRem(const BinOpInfo &Ops) { if ((CGF.SanOpts.has(SanitizerKind::IntegerDivideByZero) || CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) && Ops.Ty->isIntegerType() && - (Ops.mayHaveIntegerDivisionByZero() || Ops.mayHaveIntegerOverflow())) { + (Ops.mayHaveIntegerDivisionByZero() || Ops.mayHaveIntegerOverflow()) && + !Ops.hasWrappingOperand()) { CodeGenFunction::SanitizerScope SanScope(&CGF); llvm::Value *Zero = llvm::Constant::getNullValue(ConvertType(Ops.Ty)); EmitUndefinedBehaviorIntegerDivAndRemCheck(Ops, Zero, false); @@ -4106,7 +4127,7 @@ Value *ScalarExprEmitter::EmitAdd(const BinOpInfo &op) { op.RHS->getType()->isPointerTy()) return emitPointerArithmetic(CGF, op, CodeGenFunction::NotSubtraction); - if (op.Ty->isSignedIntegerOrEnumerationType()) { + if (op.Ty->isSignedIntegerOrEnumerationType() && !op.hasWrappingOperand()) { switch (CGF.getLangOpts().getSignedOverflowBehavior()) { case LangOptions::SOB_Defined: if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) @@ -4139,7 +4160,7 @@ Value *ScalarExprEmitter::EmitAdd(const BinOpInfo &op) { if (op.Ty->isUnsignedIntegerType() && CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) && - !CanElideOverflowCheck(CGF.getContext(), op)) + !CanElideOverflowCheck(CGF.getContext(), op) && !op.hasWrappingOperand()) return EmitOverflowCheckedBinOp(op); if (op.LHS->getType()->isFPOrFPVectorTy()) { @@ -4262,7 +4283,7 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) { Value *ScalarExprEmitter::EmitSub(const BinOpInfo &op) { // The LHS is always a pointer if either side is. if (!op.LHS->getType()->isPointerTy()) { - if (op.Ty->isSignedIntegerOrEnumerationType()) { + if (op.Ty->isSignedIntegerOrEnumerationType() && !op.hasWrappingOperand()) { switch (CGF.getLangOpts().getSignedOverflowBehavior()) { case LangOptions::SOB_Defined: if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) @@ -4295,7 +4316,8 @@ Value *ScalarExprEmitter::EmitSub(const BinOpInfo &op) { if (op.Ty->isUnsignedIntegerType() && CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) && - !CanElideOverflowCheck(CGF.getContext(), op)) + !CanElideOverflowCheck(CGF.getContext(), op) && + !op.hasWrappingOperand()) return EmitOverflowCheckedBinOp(op); if (op.LHS->getType()->isFPOrFPVectorTy()) { @@ -4415,7 +4437,8 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) { bool SanitizeSignedBase = CGF.SanOpts.has(SanitizerKind::ShiftBase) && Ops.Ty->hasSignedIntegerRepresentation() && !CGF.getLangOpts().isSignedOverflowDefined() && - !CGF.getLangOpts().CPlusPlus20; + !CGF.getLangOpts().CPlusPlus20 && + !Ops.hasWrappingOperand(); bool SanitizeUnsignedBase = CGF.SanOpts.has(SanitizerKind::UnsignedShiftBase) && Ops.Ty->hasUnsignedIntegerRepresentation(); diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index f847c49920cf3..3c6edd1d6c59b 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -675,6 +675,9 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty, QualType ExprTy = Context.getCanonicalType(E->getType()); QualType TypeTy = Context.getCanonicalType(Ty); + if (E->getType().getTypePtr()->isIntegerType() && E->getType().hasWrapsAttr()) + Ty = Context.getAttributedType(attr::Wraps, Ty, Ty); + if (ExprTy == TypeTy) return E; diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index f2dc8e9dd0050..c45759fab385f 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -15631,7 +15631,8 @@ static void AnalyzeAssignment(Sema &S, BinaryOperator *E) { // We want to recurse on the RHS as normal unless we're assigning to // a bitfield. if (FieldDecl *Bitfield = E->getLHS()->getSourceBitField()) { - if (AnalyzeBitFieldAssignment(S, Bitfield, E->getRHS(), + if (!E->hasWrappingOperand(S.Context) && + AnalyzeBitFieldAssignment(S, Bitfield, E->getRHS(), E->getOperatorLoc())) { // Recurse, ignoring any implicit conversions on the RHS. return AnalyzeImplicitConversions(S, E->getRHS()->IgnoreParenImpCasts(), @@ -15871,11 +15872,37 @@ static bool IsImplicitBoolFloatConversion(Sema &S, Expr *Ex, bool ToBool) { FloatCandidateBT && (FloatCandidateBT->isFloatingPoint())); } +/// Check to see if the wraps attribute may have been lost through a function +/// call. For cases where we are unsure, assume not. +static bool IsWrapsAttrLost(Sema &S, const CallExpr *TheCall, + const FunctionDecl *FD, unsigned i) { + // We may not have a FunctionDecl if this CallExpr is associated virtual, + // templated or overloaded functions. + if (!FD) + return false; + + if (i >= TheCall->getNumArgs() || i >= FD->getNumParams()) + return false; + + const QualType ProvidedArgTy = TheCall->getArg(i)->getType(); + const QualType PrototypedArgTy = FD->getParamDecl(i)->getType(); + + return ProvidedArgTy.hasWrapsAttr() && !PrototypedArgTy.hasWrapsAttr(); +} + static void CheckImplicitArgumentConversions(Sema &S, CallExpr *TheCall, SourceLocation CC) { unsigned NumArgs = TheCall->getNumArgs(); + const FunctionDecl *FD = TheCall->getDirectCallee(); + for (unsigned i = 0; i < NumArgs; ++i) { Expr *CurrA = TheCall->getArg(i); + + if (IsWrapsAttrLost(S, TheCall, FD, i)) + S.Diag(CurrA->getSourceRange().getBegin(), + diag::warn_wraps_attr_maybe_lost) + << FD->getParamDecl(i)->getType(); + if (!IsImplicitBoolFloatConversion(S, CurrA, true)) continue; @@ -16474,7 +16501,7 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T, S.Context, E, S.isConstantEvaluatedContext(), /*Approximate=*/true); IntRange TargetRange = IntRange::forTargetOfCanonicalType(S.Context, Target); - if (LikelySourceRange.Width > TargetRange.Width) { + if (LikelySourceRange.Width > TargetRange.Width && !T.hasWrapsAttr()) { // If the source is a constant, use a default-on diagnostic. // TODO: this should happen for bitfield stores, too. Expr::EvalResult Result; @@ -16522,7 +16549,7 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T, if (TargetRange.Width == LikelySourceRange.Width && !TargetRange.NonNegative && LikelySourceRange.NonNegative && - Source->isSignedIntegerType()) { + Source->isSignedIntegerType() && !T.hasWrapsAttr()) { // Warn when doing a signed to signed conversion, warn if the positive // source value is exactly the width of the target type, which will // cause a negative value to be stored. diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index ca5938083917f..865c982426246 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -4481,6 +4481,10 @@ void Sema::AddAlignValueAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E) { D->addAttr(::new (Context) AlignValueAttr(Context, CI, E)); } +static void handleWrapsAttr(Sema &S, Decl *D, const ParsedAttr &AL) { + D->addAttr(::new (S.Context) WrapsAttr(S.Context, AL)); +} + static void handleAlignedAttr(Sema &S, Decl *D, const ParsedAttr &AL) { if (AL.hasParsedType()) { const ParsedType &TypeArg = AL.getTypeArg(); @@ -9778,10 +9782,12 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, case ParsedAttr::AT_AvailableOnlyInDefaultEvalMethod: handleAvailableOnlyInDefaultEvalMethod(S, D, AL); break; - case ParsedAttr::AT_CountedBy: handleCountedByAttrField(S, D, AL); break; + case ParsedAttr::AT_Wraps: + handleWrapsAttr(S, D, AL); + break; // Microsoft attributes: case ParsedAttr::AT_LayoutVersion: diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index c19c8cc34dd3b..38b75039039a7 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -6513,6 +6513,18 @@ static void HandleBTFTypeTagAttribute(QualType &Type, const ParsedAttr &Attr, ::new (Ctx) BTFTypeTagAttr(Ctx, Attr, BTFTypeTag), Type); } +static void handleWrapsAttr(QualType &Type, const ParsedAttr &Attr, + TypeProcessingState &State) { + Sema &S = State.getSema(); + ASTContext &Ctx = S.Context; + + if (!Type->isIntegerType()) + S.Diag(Attr.getLoc(), diag::warn_wraps_attr_var_decl_type_not_integer) + << Type.getAsString(); + + Type = State.getAttributedType(::new (Ctx) WrapsAttr(Ctx, Attr), Type, Type); +} + /// HandleAddressSpaceTypeAttribute - Process an address_space attribute on the /// specified type. The attribute contains 1 argument, the id of the address /// space for the type. @@ -8503,6 +8515,9 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type, HandleBTFTypeTagAttribute(type, attr, state); attr.setUsedAsTypeAttr(); break; + case ParsedAttr::AT_Wraps: + handleWrapsAttr(type, attr, state); + break; case ParsedAttr::AT_MayAlias: // FIXME: This attribute needs to actually be handled, but if we ignore diff --git a/clang/test/CodeGen/integer-overflow.c b/clang/test/CodeGen/integer-overflow.c index 461b026d39615..cb6cb201b55f9 100644 --- a/clang/test/CodeGen/integer-overflow.c +++ b/clang/test/CodeGen/integer-overflow.c @@ -105,3 +105,69 @@ void test1(void) { // TRAPV: call ptr @llvm.frameaddress.p0(i32 0) // CATCH_UB: call ptr @llvm.frameaddress.p0(i32 0) } + +// Tests for integer overflow using __attribute__((wraps)) +typedef int __attribute__((wraps)) wrapping_int; + +void test2(void) { + // DEFAULT-LABEL: define{{.*}} void @test2 + // WRAPV-LABEL: define{{.*}} void @test2 + // TRAPV-LABEL: define{{.*}} void @test2 + extern volatile wrapping_int a, b, c; + + // Basically, all cases should match the WRAPV case since this attribute + // effectively enables wrapv for expressions containing wrapping types. + + // DEFAULT,WRAPV,TRAPV,CATCH_UB,TRAPV_HANDLER: add i32 + a = b + c; + + // DEFAULT,WRAPV,TRAPV,CATCH_UB,TRAPV_HANDLER: sub i32 + a = b - c; + + // DEFAULT,WRAPV,TRAPV,CATCH_UB,TRAPV_HANDLER: mul i32 + a = b * c; + + // DEFAULT,WRAPV,TRAPV,CATCH_UB,TRAPV_HANDLER: sub i32 0, + a = -b; + + // DEFAULT,WRAPV,TRAPV,CATCH_UB,TRAPV_HANDLER: add i32 {{.*}}, 1 + ++b; + + // DEFAULT,WRAPV,TRAPV,CATCH_UB,TRAPV_HANDLER: add i32 {{.*}}, -1 + --b; + + // Less trivial cases + extern volatile wrapping_int u, v; + extern volatile int w; + + // DEFAULT,WRAPV,TRAPV,CATCH_UB,TRAPV_HANDLER: add i32 + if (u + v < u) {} + + // DEFAULT,WRAPV,TRAPV,CATCH_UB,TRAPV_HANDLER: add i32 + for (;u + v < u;) {} + + // this (w+1) should have instrumentation + // DEFAULT,WRAPV,TRAPV,CATCH_UB,TRAPV_HANDLER: call {{.*}} @llvm.sadd.with.overflow.i32 + u = (w+1) + v; + + // no parts of this expression should have instrumentation + // DEFAULT,WRAPV,TRAPV,CATCH_UB,TRAPV_HANDLER: add i32 {{.*}}, 1 + u = (v+1) + w; + + // downcast off the wraps attribute + // DEFAULT,WRAPV,TRAPV,CATCH_UB,TRAPV_HANDLER: call { i32, i1 } @llvm.sadd.with.overflow.i32 + u = (int) u + (int) v; + + // DEFAULT,WRAPV,TRAPV,CATCH_UB,TRAPV_HANDLER: call { i32, i1 } @llvm.sadd.with.overflow.i32 + u = (int) u + w; + + // persist wraps attribute through implicit integer promotion + // DEFAULT,WRAPV,TRAPV,CATCH_UB,TRAPV_HANDLER: store i8 127, ptr [[D1:%.*]] + // DEFAULT,WRAPV,TRAPV,CATCH_UB,TRAPV_HANDLER: mul{{.*}} = mul i32 + // would look like mul{{.*}} = mul nsw i32 + // if overflow behavior was not defined. + char __attribute__((wraps)) d; + d = 127; + w = d*d*d*d*d; // d is promoted to int, then calculation is made. + // wraps prevents instrumentation even through promotion +} diff --git a/clang/test/CodeGen/unsigned-overflow.c b/clang/test/CodeGen/unsigned-overflow.c index 6c2f0c1efc145..471a06e5fa63f 100644 --- a/clang/test/CodeGen/unsigned-overflow.c +++ b/clang/test/CodeGen/unsigned-overflow.c @@ -5,6 +5,11 @@ unsigned long li, lj, lk; unsigned int ii, ij, ik; +// The wraps attribute disables sanitizer instrumentation for arithmetic +// expressions containing these types. +unsigned long __attribute__((wraps)) li_w, lj_w, lk_w; +unsigned int __attribute__((wraps)) ii_w, ij_w, ik_w; + extern void opaquelong(unsigned long); extern void opaqueint(unsigned int); @@ -18,6 +23,11 @@ void testlongadd(void) { // CHECK-NEXT: [[T5:%.*]] = extractvalue { i64, i1 } [[T3]], 1 // CHECK: call void @__ubsan_handle_add_overflow li = lj + lk; + + // CHECK: [[T6:%.*]] = load i64, ptr @lj_w + // CHECK-NEXT: [[T7:%.*]] = load i64, ptr @lk_w + // CHECK-NEXT: add i64 [[T6]], [[T7]] + li_w = lj_w + lk_w; } // CHECK-LABEL: define{{.*}} void @testlongsub() @@ -30,6 +40,11 @@ void testlongsub(void) { // CHECK-NEXT: [[T5:%.*]] = extractvalue { i64, i1 } [[T3]], 1 // CHECK: call void @__ubsan_handle_sub_overflow li = lj - lk; + + // CHECK: [[T6:%.*]] = load i64, ptr @lj_w + // CHECK-NEXT: [[T7:%.*]] = load i64, ptr @lk_w + // CHECK-NEXT: sub i64 [[T6]], [[T7]] + li_w = lj_w - lk_w; } // CHECK-LABEL: define{{.*}} void @testlongmul() @@ -42,28 +57,39 @@ void testlongmul(void) { // CHECK-NEXT: [[T5:%.*]] = extractvalue { i64, i1 } [[T3]], 1 // CHECK: call void @__ubsan_handle_mul_overflow li = lj * lk; + + // CHECK: [[T6:%.*]] = load i64, ptr @lj_w + // CHECK-NEXT: [[T7:%.*]] = load i64, ptr @lk_w + // CHECK-NEXT: mul i64 [[T6]], [[T7]] + li_w = lj_w * lk_w; } // CHECK-LABEL: define{{.*}} void @testlongpostinc() void testlongpostinc(void) { - opaquelong(li++); - // CHECK: [[T1:%.*]] = load i64, ptr @li // CHECK-NEXT: [[T2:%.*]] = call { i64, i1 } @llvm.uadd.with.overflow.i64(i64 [[T1]], i64 1) // CHECK-NEXT: [[T3:%.*]] = extractvalue { i64, i1 } [[T2]], 0 // CHECK-NEXT: [[T4:%.*]] = extractvalue { i64, i1 } [[T2]], 1 // CHECK: call void @__ubsan_handle_add_overflow + opaquelong(li++); + + // CHECK: [[T5:%.*]] = load i64, ptr @li_w + // CHECK-NEXT: add i64 [[T5]], 1 + opaquelong(li_w++); } // CHECK-LABEL: define{{.*}} void @testlongpreinc() void testlongpreinc(void) { - opaquelong(++li); - // CHECK: [[T1:%.*]] = load i64, ptr @li // CHECK-NEXT: [[T2:%.*]] = call { i64, i1 } @llvm.uadd.with.overflow.i64(i64 [[T1]], i64 1) // CHECK-NEXT: [[T3:%.*]] = extractvalue { i64, i1 } [[T2]], 0 // CHECK-NEXT: [[T4:%.*]] = extractvalue { i64, i1 } [[T2]], 1 // CHECK: call void @__ubsan_handle_add_overflow + opaquelong(++li); + + // CHECK: [[T5:%.*]] = load i64, ptr @li_w + // CHECK-NEXT: add i64 [[T5]], 1 + opaquelong(++li_w); } // CHECK-LABEL: define{{.*}} void @testintadd() @@ -76,6 +102,11 @@ void testintadd(void) { // CHECK-NEXT: [[T5:%.*]] = extractvalue { i32, i1 } [[T3]], 1 // CHECK: call void @__ubsan_handle_add_overflow ii = ij + ik; + + // CHECK: [[T6:%.*]] = load i32, ptr @ij_w + // CHECK-NEXT: [[T7:%.*]] = load i32, ptr @ik_w + // CHECK-NEXT: add i32 [[T6]], [[T7]] + ii_w = ij_w + ik_w; } // CHECK-LABEL: define{{.*}} void @testintsub() @@ -88,6 +119,11 @@ void testintsub(void) { // CHECK-NEXT: [[T5:%.*]] = extractvalue { i32, i1 } [[T3]], 1 // CHECK: call void @__ubsan_handle_sub_overflow ii = ij - ik; + + // CHECK: [[T6:%.*]] = load i32, ptr @ij_w + // CHECK-NEXT: [[T7:%.*]] = load i32, ptr @ik_w + // CHECK-NEXT: sub i32 [[T6]], [[T7]] + ii_w = ij_w - ik_w; } // CHECK-LABEL: define{{.*}} void @testintmul() @@ -100,26 +136,37 @@ void testintmul(void) { // CHECK-NEXT: [[T5:%.*]] = extractvalue { i32, i1 } [[T3]], 1 // CHECK: call void @__ubsan_handle_mul_overflow ii = ij * ik; + + // CHECK: [[T6:%.*]] = load i32, ptr @ij_w + // CHECK-NEXT: [[T7:%.*]] = load i32, ptr @ik_w + // CHECK-NEXT: mul i32 [[T6]], [[T7]] + ii_w = ij_w * ik_w; } // CHECK-LABEL: define{{.*}} void @testintpostinc() void testintpostinc(void) { - opaqueint(ii++); - // CHECK: [[T1:%.*]] = load i32, ptr @ii // CHECK-NEXT: [[T2:%.*]] = call { i32, i1 } @llvm.uadd.with.overflow.i32(i32 [[T1]], i32 1) // CHECK-NEXT: [[T3:%.*]] = extractvalue { i32, i1 } [[T2]], 0 // CHECK-NEXT: [[T4:%.*]] = extractvalue { i32, i1 } [[T2]], 1 // CHECK: call void @__ubsan_handle_add_overflow + opaqueint(ii++); + + // CHECK: [[T5:%.*]] = load i32, ptr @ii_w + // CHECK-NEXT: add i32 [[T5]], 1 + opaqueint(ii_w++); } // CHECK-LABEL: define{{.*}} void @testintpreinc() void testintpreinc(void) { - opaqueint(++ii); - // CHECK: [[T1:%.*]] = load i32, ptr @ii // CHECK-NEXT: [[T2:%.*]] = call { i32, i1 } @llvm.uadd.with.overflow.i32(i32 [[T1]], i32 1) // CHECK-NEXT: [[T3:%.*]] = extractvalue { i32, i1 } [[T2]], 0 // CHECK-NEXT: [[T4:%.*]] = extractvalue { i32, i1 } [[T2]], 1 // CHECK: call void @__ubsan_handle_add_overflow + opaqueint(++ii); + + // CHECK: [[T5:%.*]] = load i32, ptr @ii_w + // CHECK-NEXT: add i32 [[T5]], 1 + opaqueint(++ii_w); } diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test index fd0e6d71baa80..3073b3fcc6638 100644 --- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test +++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test @@ -212,6 +212,7 @@ // CHECK-NEXT: WebAssemblyImportModule (SubjectMatchRule_function) // CHECK-NEXT: WebAssemblyImportName (SubjectMatchRule_function) // CHECK-NEXT: WorkGroupSizeHint (SubjectMatchRule_function) +// CHECK-NEXT: Wraps (SubjectMatchRule_variable, SubjectMatchRule_type_alias, SubjectMatchRule_field) // CHECK-NEXT: XRayInstrument (SubjectMatchRule_function, SubjectMatchRule_objc_method) // CHECK-NEXT: XRayLogArgs (SubjectMatchRule_function, SubjectMatchRule_objc_method) // CHECK-NEXT: ZeroCallUsedRegs (SubjectMatchRule_function) diff --git a/clang/test/Sema/attr-wraps.c b/clang/test/Sema/attr-wraps.c new file mode 100644 index 0000000000000..cb30200f072f2 --- /dev/null +++ b/clang/test/Sema/attr-wraps.c @@ -0,0 +1,43 @@ +// RUN: %clang_cc1 %s -verify -fsyntax-only -triple x86_64-pc-linux-gnu +typedef int __attribute__((wraps)) wrapping_int; +typedef unsigned __attribute__((wraps)) wrapping_u32; + +int implicit_truncation(void) { + const wrapping_int A = 1; + return 2147483647 + A; // no warning +} + +struct R { + wrapping_int a: 2; // test bitfield sign change -- no warning + wrapping_u32 b: 1; // test bitfield overflow/truncation -- no warning + int baseline: 2; // baseline, should warn +}; + +void bitfields_truncation(void) { + struct R r; + r.a = 2; // this value changes from 2 to -2 + ++r.a; + + r.b = 2; // changes from 2 to 0 + ++r.b; + + // expected-warning@+1 {{to bit-field changes value from}} + r.baseline = 2; +} + +extern void implicitly_discards_wraps_attribute(int discards); + +int discard_test(void) { + wrapping_int A = 1; + // expected-warning@+1 {{'wraps' attribute may be implicitly discarded}} + implicitly_discards_wraps_attribute(A); + + int B = A; // assignments don't warn right now -- probably too noisy + return A; // neither do non-wrapping return types +} + +float useless_wraps_attribute(void) { + // expected-warning@+1 {{using attribute 'wraps' with non-integer type}} + float __attribute__((wraps)) A = 3.14; + return A; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
