[clang] [Clang] Add wraps attribute (for granular integer overflow handling) (PR #86618)
Justin Stitt via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 22 13:02:29 PDT 2024
https://github.com/JustinStitt updated https://github.com/llvm/llvm-project/pull/86618
>From 10ee32826fc2acb6bd993c88bdb7142360b6f263 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Tue, 5 Mar 2024 03:14:49 +0000
Subject: [PATCH 01/11] implement wraps attribute
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/docs/ReleaseNotes.rst | 9 +++
clang/include/clang/AST/Expr.h | 3 +
clang/include/clang/Basic/Attr.td | 6 ++
clang/include/clang/Basic/AttrDocs.td | 66 +++++++++++++++++++
.../clang/Basic/DiagnosticSemaKinds.td | 3 +
clang/include/clang/Sema/Sema.h | 4 ++
clang/lib/AST/Expr.cpp | 19 ++++++
clang/lib/AST/ExprConstant.cpp | 6 +-
clang/lib/AST/TypePrinter.cpp | 3 +
clang/lib/CodeGen/CGExprScalar.cpp | 40 +++++++++--
clang/lib/Sema/SemaDeclAttr.cpp | 12 +++-
clang/lib/Sema/SemaType.cpp | 15 +++++
clang/test/CodeGen/integer-overflow.c | 56 ++++++++++++++++
clang/test/CodeGen/unsigned-overflow.c | 63 +++++++++++++++---
clang/test/Sema/attr-wraps.c | 9 +++
15 files changed, 298 insertions(+), 16 deletions(-)
create mode 100644 clang/test/Sema/attr-wraps.c
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f96cebbde3d825..cb02af7e948989 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -282,6 +282,15 @@ Attribute Changes in Clang
This allows the ``_Nullable`` and ``_Nonnull`` family of type attributes to
apply to this class.
+- 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 ignored in C++.
+
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 2bfefeabc348be..68cd7d7c0fac3b 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -4077,6 +4077,9 @@ class BinaryOperator : public Expr {
static unsigned sizeOfTrailingObjects(bool HasFPFeatures) {
return HasFPFeatures * sizeof(FPOptionsOverride);
}
+
+ /// Do one of the subexpressions have the wraps attribute?
+ bool oneOfWraps(const ASTContext &Ctx) const;
};
/// CompoundAssignOperator - For compound assignments (e.g. +=), we keep
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index dc87a8c6f022dc..06e41fcee206c4 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4506,3 +4506,9 @@ def CodeAlign: StmtAttr {
static constexpr int MaximumAlignment = 4096;
}];
}
+
+def Wraps : DeclOrTypeAttr {
+ let Spellings = [Clang<"wraps">, CXX11<"", "wraps", 202403>];
+ let Subjects = SubjectList<[Var, TypedefName, Field]>;
+ let Documentation = [WrapsDocs];
+}
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 0ca4ea377fc36a..a2adb923e3c47c 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -8044,3 +8044,69 @@ requirement:
}
}];
}
+
+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/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1c068f6cdb4293..0d90fc2d9bcf17 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6529,6 +6529,9 @@ 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">;
+
let CategoryName = "ARC Semantic Issue" in {
// ARC-mode diagnostics.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index f311f9f3743454..8d47ac79272fa0 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3630,6 +3630,10 @@ class Sema final : public SemaBase {
void AddAnnotationAttr(Decl *D, const AttributeCommonInfo &CI,
StringRef Annot, MutableArrayRef<Expr *> Args);
+ /// AddWrapsAttr - Adds the "wraps" attribute to a particular
+ /// declaration.
+ void AddWrapsAttr(Decl *D, const AttributeCommonInfo &CI);
+
bool checkMSInheritanceAttrOnDefinition(CXXRecordDecl *RD, SourceRange Range,
bool BestCase,
MSInheritanceModel SemanticSpelling);
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 07c9f287dd0767..f8cf0559f8eac4 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -2237,6 +2237,21 @@ bool BinaryOperator::isNullPointerArithmeticExtension(ASTContext &Ctx,
return true;
}
+bool BinaryOperator::oneOfWraps(const ASTContext &Ctx) const {
+ llvm::SmallVector<Expr *, 2> Both = {getLHS(), getRHS()};
+
+ for (const Expr *oneOf : Both) {
+ if (!oneOf)
+ continue;
+ if (auto *TypePtr =
+ oneOf->IgnoreParenImpCasts()->getType().getTypePtrOrNull())
+ if (TypePtr->hasAttr(attr::Wraps)) {
+ return true;
+ }
+ }
+ return false;
+}
+
SourceLocExpr::SourceLocExpr(const ASTContext &Ctx, SourceLocIdentKind Kind,
QualType ResultTy, SourceLocation BLoc,
SourceLocation RParenLoc,
@@ -4751,6 +4766,8 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
if (hasStoredFPFeatures())
setStoredFPFeatures(FPFeatures);
setDependence(computeDependence(this));
+ if (oneOfWraps(Ctx))
+ setType(Ctx.getAttributedType(attr::Wraps, getType(), getType()));
}
BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
@@ -4768,6 +4785,8 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
if (hasStoredFPFeatures())
setStoredFPFeatures(FPFeatures);
setDependence(computeDependence(this));
+ if (oneOfWraps(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 88c8eaf6ef9b6e..51dd73bf1ee897 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2775,7 +2775,8 @@ 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().getTypePtr()->hasAttr(attr::Wraps))
Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
diag::warn_integer_constant_overflow)
<< toString(Result, 10, Result.isSigned(), /*formatAsCLiteral=*/false,
@@ -13993,7 +13994,8 @@ 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().getTypePtr()->hasAttr(attr::Wraps))
Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
diag::warn_integer_constant_overflow)
<< toString(Value, 10, Value.isSigned(), /*formatAsCLiteral=*/false,
diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index 075c8aba11fcb9..2a9346d8253d20 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 1f18e0d5ba409a..be1cae5ca8ea42 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -147,6 +147,15 @@ struct BinOpInfo {
return UnOp->getSubExpr()->getType()->isFixedPointType();
return false;
}
+
+ /// Does the BinaryOperator have the wraps attribute?
+ /// If so, we can ellide overflow sanitizer checks.
+ bool oneOfWraps() const {
+ const Type *TyPtr = E->getType().getTypePtrOrNull();
+ if (TyPtr)
+ return TyPtr->hasAttr(attr::Wraps);
+ return false;
+ }
};
static bool MustVisitNullValue(const Expr *E) {
@@ -726,6 +735,11 @@ class ScalarExprEmitter
// Binary Operators.
Value *EmitMul(const BinOpInfo &Ops) {
+ if ((Ops.Ty->isSignedIntegerOrEnumerationType() ||
+ Ops.Ty->isUnsignedIntegerType()) &&
+ Ops.oneOfWraps())
+ return Builder.CreateMul(Ops.LHS, Ops.RHS, "mul");
+
if (Ops.Ty->isSignedIntegerOrEnumerationType()) {
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
case LangOptions::SOB_Defined:
@@ -2822,6 +2836,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.");
@@ -2878,10 +2895,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.oneOfWraps()) {
value = EmitIncDecConsiderOverflowBehavior(E, value, isInc);
} else if (E->canOverflow() && type->isUnsignedIntegerType() &&
- CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow)) {
+ CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) &&
+ !Ops.oneOfWraps()) {
value = EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(
E, value, isInc, E->getFPFeaturesInEffect(CGF.getLangOpts())));
} else {
@@ -3670,7 +3689,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.oneOfWraps()) {
llvm::Value *Zero = llvm::Constant::getNullValue(ConvertType(Ops.Ty));
EmitUndefinedBehaviorIntegerDivAndRemCheck(Ops, Zero, true);
} else if (CGF.SanOpts.has(SanitizerKind::FloatDivideByZero) &&
@@ -3719,7 +3739,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.oneOfWraps()) {
CodeGenFunction::SanitizerScope SanScope(&CGF);
llvm::Value *Zero = llvm::Constant::getNullValue(ConvertType(Ops.Ty));
EmitUndefinedBehaviorIntegerDivAndRemCheck(Ops, Zero, false);
@@ -4084,6 +4105,11 @@ Value *ScalarExprEmitter::EmitAdd(const BinOpInfo &op) {
op.RHS->getType()->isPointerTy())
return emitPointerArithmetic(CGF, op, CodeGenFunction::NotSubtraction);
+ if ((op.Ty->isSignedIntegerOrEnumerationType() ||
+ op.Ty->isUnsignedIntegerType()) &&
+ op.oneOfWraps())
+ return Builder.CreateAdd(op.LHS, op.RHS, "add");
+
if (op.Ty->isSignedIntegerOrEnumerationType()) {
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
case LangOptions::SOB_Defined:
@@ -4240,6 +4266,10 @@ 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() ||
+ op.Ty->isUnsignedIntegerType()) &&
+ op.oneOfWraps())
+ return Builder.CreateSub(op.LHS, op.RHS, "sub");
if (op.Ty->isSignedIntegerOrEnumerationType()) {
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
case LangOptions::SOB_Defined:
@@ -4390,7 +4420,7 @@ 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.oneOfWraps();
bool SanitizeUnsignedBase =
CGF.SanOpts.has(SanitizerKind::UnsignedShiftBase) &&
Ops.Ty->hasUnsignedIntegerRepresentation();
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 8bce04640e748e..45dedb4cb34186 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -4413,6 +4413,14 @@ 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) {
+ S.AddWrapsAttr(D, AL);
+}
+
+void Sema::AddWrapsAttr(Decl *D, const AttributeCommonInfo &CI) {
+ D->addAttr(::new (Context) WrapsAttr(Context, CI));
+}
+
static void handleAlignedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
if (AL.hasParsedType()) {
const ParsedType &TypeArg = AL.getTypeArg();
@@ -9704,10 +9712,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 8762744396f4dd..93c34ba9b58f59 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -6954,6 +6954,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.
@@ -8945,6 +8957,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 461b026d39615b..44c42ed9efe577 100644
--- a/clang/test/CodeGen/integer-overflow.c
+++ b/clang/test/CodeGen/integer-overflow.c
@@ -105,3 +105,59 @@ 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;
+}
diff --git a/clang/test/CodeGen/unsigned-overflow.c b/clang/test/CodeGen/unsigned-overflow.c
index 6c2f0c1efc145e..471a06e5fa63ff 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/Sema/attr-wraps.c b/clang/test/Sema/attr-wraps.c
new file mode 100644
index 00000000000000..97aff317120633
--- /dev/null
+++ b/clang/test/Sema/attr-wraps.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -triple x86_64-pc-linux-gnu
+// expected-no-diagnostics
+typedef int __attribute__((wraps)) wrapping_int;
+
+void foo(void) {
+ const wrapping_int A = 1;
+ int D = 2147483647 + A;
+ (void)D;
+}
>From 6794e45c63a396c18707e10f20a12d36a775457a Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Tue, 26 Mar 2024 23:29:29 +0000
Subject: [PATCH 02/11] add wraps bypass for
implicit-(un)signed-integer-truncation sanitizers
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/lib/CodeGen/CGExprScalar.cpp | 3 ++-
clang/lib/Sema/SemaChecking.cpp | 5 +++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index be1cae5ca8ea42..f07388fdec6869 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -1108,7 +1108,8 @@ 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.getTypePtr()->hasAttr(attr::Wraps))
return;
llvm::Constant *StaticArgs[] = {
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index b84a779b7189c0..65504da36f9751 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -16439,7 +16439,8 @@ 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.getTypePtr()->hasAttr(attr::Wraps)) {
// If the source is a constant, use a default-on diagnostic.
// TODO: this should happen for bitfield stores, too.
Expr::EvalResult Result;
@@ -16487,7 +16488,7 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
if (TargetRange.Width == LikelySourceRange.Width &&
!TargetRange.NonNegative && LikelySourceRange.NonNegative &&
- Source->isSignedIntegerType()) {
+ Source->isSignedIntegerType() && !T.getTypePtr()->hasAttr(attr::Wraps)) {
// 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.
>From 0f7e8c96cd197dc273c19b071f98e41fd9185b26 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Tue, 9 Apr 2024 21:29:35 +0000
Subject: [PATCH 03/11] don't support c++
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/include/clang/Basic/Attr.td | 2 +-
clang/lib/Sema/SemaDeclAttr.cpp | 5 +++++
clang/lib/Sema/SemaType.cpp | 5 +++++
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 06e41fcee206c4..26c152e50845ce 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4508,7 +4508,7 @@ def CodeAlign: StmtAttr {
}
def Wraps : DeclOrTypeAttr {
- let Spellings = [Clang<"wraps">, CXX11<"", "wraps", 202403>];
+ let Spellings = [GNU<"wraps">];
let Subjects = SubjectList<[Var, TypedefName, Field]>;
let Documentation = [WrapsDocs];
}
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 45dedb4cb34186..15380ecc3bba6d 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -4414,6 +4414,11 @@ void Sema::AddAlignValueAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E) {
}
static void handleWrapsAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+ if (S.getLangOpts().CPlusPlus) {
+ S.Diag(AL.getLoc(), diag::warn_attribute_ignored) << AL;
+ return;
+ }
+
S.AddWrapsAttr(D, AL);
}
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 93c34ba9b58f59..52cf8d730dadd5 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -6959,6 +6959,11 @@ static void handleWrapsAttr(QualType &Type, const ParsedAttr &Attr,
Sema &S = State.getSema();
ASTContext &Ctx = S.Context;
+ // No need to warn here, that is handled by SemaDeclAttr.
+ // Simply disable applying this attribute.
+ if (S.getLangOpts().CPlusPlus)
+ return;
+
if (!Type->isIntegerType())
S.Diag(Attr.getLoc(), diag::warn_wraps_attr_var_decl_type_not_integer)
<< Type.getAsString();
>From 44d6238ceb627ee50a056f7fcb35c82fe828ad48 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Tue, 9 Apr 2024 22:57:55 +0000
Subject: [PATCH 04/11] fix AttrDocs code block syntax error
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/include/clang/Basic/AttrDocs.td | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index a2adb923e3c47c..d8a65dbeca0aaf 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -8071,6 +8071,7 @@ definition when building with ``-fsanitize=signed-integer-overflow``
}
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
@@ -8093,6 +8094,7 @@ sanitizers (like ``-fsanitize=unsigned-integer-overflow``).
// 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 +
>From 5e6d926532c2cff3288f25e9b8f872e7f2ec9b65 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Wed, 10 Apr 2024 21:49:12 +0000
Subject: [PATCH 05/11] rename oneOfWraps -> hasWrappingOperand
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/include/clang/AST/Expr.h | 2 +-
clang/lib/AST/Expr.cpp | 6 +++---
clang/lib/CodeGen/CGExprScalar.cpp | 18 +++++++++---------
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 68cd7d7c0fac3b..6d053cf4304f07 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -4079,7 +4079,7 @@ class BinaryOperator : public Expr {
}
/// Do one of the subexpressions have the wraps attribute?
- bool oneOfWraps(const ASTContext &Ctx) const;
+ bool hasWrappingOperand(const ASTContext &Ctx) const;
};
/// CompoundAssignOperator - For compound assignments (e.g. +=), we keep
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index f8cf0559f8eac4..97067df05bdc24 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -2237,7 +2237,7 @@ bool BinaryOperator::isNullPointerArithmeticExtension(ASTContext &Ctx,
return true;
}
-bool BinaryOperator::oneOfWraps(const ASTContext &Ctx) const {
+bool BinaryOperator::hasWrappingOperand(const ASTContext &Ctx) const {
llvm::SmallVector<Expr *, 2> Both = {getLHS(), getRHS()};
for (const Expr *oneOf : Both) {
@@ -4766,7 +4766,7 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
if (hasStoredFPFeatures())
setStoredFPFeatures(FPFeatures);
setDependence(computeDependence(this));
- if (oneOfWraps(Ctx))
+ if (hasWrappingOperand(Ctx))
setType(Ctx.getAttributedType(attr::Wraps, getType(), getType()));
}
@@ -4785,7 +4785,7 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
if (hasStoredFPFeatures())
setStoredFPFeatures(FPFeatures);
setDependence(computeDependence(this));
- if (oneOfWraps(Ctx))
+ if (hasWrappingOperand(Ctx))
setType(Ctx.getAttributedType(attr::Wraps, getType(), getType()));
}
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index f07388fdec6869..005affc4ffafa2 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -150,7 +150,7 @@ struct BinOpInfo {
/// Does the BinaryOperator have the wraps attribute?
/// If so, we can ellide overflow sanitizer checks.
- bool oneOfWraps() const {
+ bool hasWrappingOperand() const {
const Type *TyPtr = E->getType().getTypePtrOrNull();
if (TyPtr)
return TyPtr->hasAttr(attr::Wraps);
@@ -737,7 +737,7 @@ class ScalarExprEmitter
Value *EmitMul(const BinOpInfo &Ops) {
if ((Ops.Ty->isSignedIntegerOrEnumerationType() ||
Ops.Ty->isUnsignedIntegerType()) &&
- Ops.oneOfWraps())
+ Ops.hasWrappingOperand())
return Builder.CreateMul(Ops.LHS, Ops.RHS, "mul");
if (Ops.Ty->isSignedIntegerOrEnumerationType()) {
@@ -2897,11 +2897,11 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV,
// overflow because of promotion rules; we're just eliding a few steps
// here.
} else if (E->canOverflow() && type->isSignedIntegerOrEnumerationType() &&
- !Ops.oneOfWraps()) {
+ !Ops.hasWrappingOperand()) {
value = EmitIncDecConsiderOverflowBehavior(E, value, isInc);
} else if (E->canOverflow() && type->isUnsignedIntegerType() &&
CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) &&
- !Ops.oneOfWraps()) {
+ !Ops.hasWrappingOperand()) {
value = EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(
E, value, isInc, E->getFPFeaturesInEffect(CGF.getLangOpts())));
} else {
@@ -3691,7 +3691,7 @@ Value *ScalarExprEmitter::EmitDiv(const BinOpInfo &Ops) {
CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) &&
Ops.Ty->isIntegerType() &&
(Ops.mayHaveIntegerDivisionByZero() || Ops.mayHaveIntegerOverflow()) &&
- !Ops.oneOfWraps()) {
+ !Ops.hasWrappingOperand()) {
llvm::Value *Zero = llvm::Constant::getNullValue(ConvertType(Ops.Ty));
EmitUndefinedBehaviorIntegerDivAndRemCheck(Ops, Zero, true);
} else if (CGF.SanOpts.has(SanitizerKind::FloatDivideByZero) &&
@@ -3741,7 +3741,7 @@ Value *ScalarExprEmitter::EmitRem(const BinOpInfo &Ops) {
CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) &&
Ops.Ty->isIntegerType() &&
(Ops.mayHaveIntegerDivisionByZero() || Ops.mayHaveIntegerOverflow()) &&
- !Ops.oneOfWraps()) {
+ !Ops.hasWrappingOperand()) {
CodeGenFunction::SanitizerScope SanScope(&CGF);
llvm::Value *Zero = llvm::Constant::getNullValue(ConvertType(Ops.Ty));
EmitUndefinedBehaviorIntegerDivAndRemCheck(Ops, Zero, false);
@@ -4108,7 +4108,7 @@ Value *ScalarExprEmitter::EmitAdd(const BinOpInfo &op) {
if ((op.Ty->isSignedIntegerOrEnumerationType() ||
op.Ty->isUnsignedIntegerType()) &&
- op.oneOfWraps())
+ op.hasWrappingOperand())
return Builder.CreateAdd(op.LHS, op.RHS, "add");
if (op.Ty->isSignedIntegerOrEnumerationType()) {
@@ -4269,7 +4269,7 @@ Value *ScalarExprEmitter::EmitSub(const BinOpInfo &op) {
if (!op.LHS->getType()->isPointerTy()) {
if ((op.Ty->isSignedIntegerOrEnumerationType() ||
op.Ty->isUnsignedIntegerType()) &&
- op.oneOfWraps())
+ op.hasWrappingOperand())
return Builder.CreateSub(op.LHS, op.RHS, "sub");
if (op.Ty->isSignedIntegerOrEnumerationType()) {
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
@@ -4421,7 +4421,7 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) {
bool SanitizeSignedBase = CGF.SanOpts.has(SanitizerKind::ShiftBase) &&
Ops.Ty->hasSignedIntegerRepresentation() &&
!CGF.getLangOpts().isSignedOverflowDefined() &&
- !CGF.getLangOpts().CPlusPlus20 && !Ops.oneOfWraps();
+ !CGF.getLangOpts().CPlusPlus20 && !Ops.hasWrappingOperand();
bool SanitizeUnsignedBase =
CGF.SanOpts.has(SanitizerKind::UnsignedShiftBase) &&
Ops.Ty->hasUnsignedIntegerRepresentation();
>From 0d7566777f06b3f2058d93dd77624ab2c66f1127 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Wed, 10 Apr 2024 21:52:43 +0000
Subject: [PATCH 06/11] fixup C-only rules and checks
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/docs/ReleaseNotes.rst | 3 ++-
clang/include/clang/Basic/Attr.td | 3 ++-
clang/lib/Sema/SemaDeclAttr.cpp | 5 -----
clang/lib/Sema/SemaType.cpp | 5 -----
4 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index cb02af7e948989..9dfe18822496a6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -289,7 +289,8 @@ Attribute Changes in Clang
overflow warnings or sanitizer warnings. They also cannot be optimized away
by some eager UB optimizations.
- This attribute is ignored in C++.
+ This attribute is only valid for C, as there are built-in language
+ alternatives for other languages.
Improvements to Clang's diagnostics
-----------------------------------
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 26c152e50845ce..f23cfc3b12d2de 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4508,7 +4508,8 @@ def CodeAlign: StmtAttr {
}
def Wraps : DeclOrTypeAttr {
- let Spellings = [GNU<"wraps">];
+ let Spellings = [Clang<"wraps">];
let Subjects = SubjectList<[Var, TypedefName, Field]>;
let Documentation = [WrapsDocs];
+ let LangOpts = [COnly];
}
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 15380ecc3bba6d..45dedb4cb34186 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -4414,11 +4414,6 @@ void Sema::AddAlignValueAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E) {
}
static void handleWrapsAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
- if (S.getLangOpts().CPlusPlus) {
- S.Diag(AL.getLoc(), diag::warn_attribute_ignored) << AL;
- return;
- }
-
S.AddWrapsAttr(D, AL);
}
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 52cf8d730dadd5..93c34ba9b58f59 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -6959,11 +6959,6 @@ static void handleWrapsAttr(QualType &Type, const ParsedAttr &Attr,
Sema &S = State.getSema();
ASTContext &Ctx = S.Context;
- // No need to warn here, that is handled by SemaDeclAttr.
- // Simply disable applying this attribute.
- if (S.getLangOpts().CPlusPlus)
- return;
-
if (!Type->isIntegerType())
S.Diag(Attr.getLoc(), diag::warn_wraps_attr_var_decl_type_not_integer)
<< Type.getAsString();
>From a2f63982920f22d795c4971800bcc5cb55356570 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Wed, 10 Apr 2024 22:31:10 +0000
Subject: [PATCH 07/11] simplify hasWrappingOperand with QualType additions
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/include/clang/AST/Type.h | 2 ++
clang/lib/AST/Expr.cpp | 14 ++------------
clang/lib/AST/Type.cpp | 4 ++++
clang/lib/CodeGen/CGExprScalar.cpp | 5 +----
4 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 99f45d518c7960..687711ecd6c6eb 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -1239,6 +1239,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/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 97067df05bdc24..dcaaabf224e610 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -2238,18 +2238,8 @@ bool BinaryOperator::isNullPointerArithmeticExtension(ASTContext &Ctx,
}
bool BinaryOperator::hasWrappingOperand(const ASTContext &Ctx) const {
- llvm::SmallVector<Expr *, 2> Both = {getLHS(), getRHS()};
-
- for (const Expr *oneOf : Both) {
- if (!oneOf)
- continue;
- if (auto *TypePtr =
- oneOf->IgnoreParenImpCasts()->getType().getTypePtrOrNull())
- if (TypePtr->hasAttr(attr::Wraps)) {
- return true;
- }
- }
- return false;
+ return getLHS()->getType().hasWrapsAttr() ||
+ getRHS()->getType().hasWrapsAttr();
}
SourceLocExpr::SourceLocExpr(const ASTContext &Ctx, SourceLocIdentKind Kind,
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index cb22c91a12aa89..3dddfaf328886a 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -2807,6 +2807,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/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 005affc4ffafa2..a001cdc7b5f7f7 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -151,10 +151,7 @@ struct BinOpInfo {
/// Does the BinaryOperator have the wraps attribute?
/// If so, we can ellide overflow sanitizer checks.
bool hasWrappingOperand() const {
- const Type *TyPtr = E->getType().getTypePtrOrNull();
- if (TyPtr)
- return TyPtr->hasAttr(attr::Wraps);
- return false;
+ return E->getType().hasWrapsAttr();
}
};
>From a3e9abdfdeb3ca4ae0cddd9c9c76b9c7a717be57 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Wed, 10 Apr 2024 22:37:17 +0000
Subject: [PATCH 08/11] run git-clang-format
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/lib/CodeGen/CGExprScalar.cpp | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index a001cdc7b5f7f7..4d996b4c5741bc 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -150,9 +150,7 @@ struct BinOpInfo {
/// Does the BinaryOperator have the wraps attribute?
/// If so, we can ellide overflow sanitizer checks.
- bool hasWrappingOperand() const {
- return E->getType().hasWrapsAttr();
- }
+ bool hasWrappingOperand() const { return E->getType().hasWrapsAttr(); }
};
static bool MustVisitNullValue(const Expr *E) {
@@ -4418,7 +4416,8 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) {
bool SanitizeSignedBase = CGF.SanOpts.has(SanitizerKind::ShiftBase) &&
Ops.Ty->hasSignedIntegerRepresentation() &&
!CGF.getLangOpts().isSignedOverflowDefined() &&
- !CGF.getLangOpts().CPlusPlus20 && !Ops.hasWrappingOperand();
+ !CGF.getLangOpts().CPlusPlus20 &&
+ !Ops.hasWrappingOperand();
bool SanitizeUnsignedBase =
CGF.SanOpts.has(SanitizerKind::UnsignedShiftBase) &&
Ops.Ty->hasUnsignedIntegerRepresentation();
>From eb08ca00a05f3339ed389a5cca88042002e7e7e7 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Wed, 10 Apr 2024 22:57:35 +0000
Subject: [PATCH 09/11] fixup cases where hasWrapsAttr should be used
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/lib/AST/ExprConstant.cpp | 4 ++--
clang/lib/CodeGen/CGExprScalar.cpp | 2 +-
clang/lib/Sema/SemaChecking.cpp | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 51dd73bf1ee897..269f22ab4b56d0 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2776,7 +2776,7 @@ static bool CheckedIntArithmetic(EvalInfo &Info, const Expr *E,
Result = Value.trunc(LHS.getBitWidth());
if (Result.extend(BitWidth) != Value) {
if (Info.checkingForUndefinedBehavior() &&
- !E->getType().getTypePtr()->hasAttr(attr::Wraps))
+ !E->getType().hasWrapsAttr())
Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
diag::warn_integer_constant_overflow)
<< toString(Result, 10, Result.isSigned(), /*formatAsCLiteral=*/false,
@@ -13995,7 +13995,7 @@ bool IntExprEvaluator::VisitUnaryOperator(const UnaryOperator *E) {
const APSInt &Value = Result.getInt();
if (Value.isSigned() && Value.isMinSignedValue() && E->canOverflow()) {
if (Info.checkingForUndefinedBehavior() &&
- !E->getType().getTypePtr()->hasAttr(attr::Wraps))
+ !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/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 4d996b4c5741bc..5a12f1187a68f6 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -1104,7 +1104,7 @@ void ScalarExprEmitter::EmitIntegerTruncationCheck(Value *Src, QualType SrcType,
// Do we care about this type of truncation?
if (!CGF.SanOpts.has(Check.second.second) ||
- DstType.getTypePtr()->hasAttr(attr::Wraps))
+ DstType.hasWrapsAttr())
return;
llvm::Constant *StaticArgs[] = {
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 65504da36f9751..05e1dfde5f6281 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -16440,7 +16440,7 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
IntRange TargetRange = IntRange::forTargetOfCanonicalType(S.Context, Target);
if (LikelySourceRange.Width > TargetRange.Width &&
- !T.getTypePtr()->hasAttr(attr::Wraps)) {
+ !T.hasWrapsAttr()) {
// If the source is a constant, use a default-on diagnostic.
// TODO: this should happen for bitfield stores, too.
Expr::EvalResult Result;
@@ -16488,7 +16488,7 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
if (TargetRange.Width == LikelySourceRange.Width &&
!TargetRange.NonNegative && LikelySourceRange.NonNegative &&
- Source->isSignedIntegerType() && !T.getTypePtr()->hasAttr(attr::Wraps)) {
+ 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.
>From 85a9b2f0e71eb1893d28aa3a8e35b29172c10521 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Thu, 11 Apr 2024 23:38:27 +0000
Subject: [PATCH 10/11] git-clang-format
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/lib/AST/ExprConstant.cpp | 6 ++----
clang/lib/CodeGen/CGExprScalar.cpp | 3 +--
clang/lib/Sema/SemaChecking.cpp | 3 +--
3 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 269f22ab4b56d0..9ff28863fb9a74 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2775,8 +2775,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() &&
- !E->getType().hasWrapsAttr())
+ if (Info.checkingForUndefinedBehavior() && !E->getType().hasWrapsAttr())
Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
diag::warn_integer_constant_overflow)
<< toString(Result, 10, Result.isSigned(), /*formatAsCLiteral=*/false,
@@ -13994,8 +13993,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() &&
- !E->getType().hasWrapsAttr())
+ 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/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 5a12f1187a68f6..f13128df57b849 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -1103,8 +1103,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) ||
- DstType.hasWrapsAttr())
+ if (!CGF.SanOpts.has(Check.second.second) || DstType.hasWrapsAttr())
return;
llvm::Constant *StaticArgs[] = {
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 05e1dfde5f6281..cddcc1e9ae752d 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -16439,8 +16439,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 &&
- !T.hasWrapsAttr()) {
+ 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;
>From 7b59be9460b735cf0b04228bfc07fbc6fa8272bf Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Mon, 22 Apr 2024 20:02:07 +0000
Subject: [PATCH 11/11] fix doc build issues
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/include/clang/Basic/AttrDocs.td | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index d8a65dbeca0aaf..def00c2c03e244 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -8063,6 +8063,7 @@ 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() {
@@ -8071,7 +8072,6 @@ definition when building with ``-fsanitize=signed-integer-overflow``
}
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
@@ -8079,9 +8079,10 @@ 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.
@@ -8091,10 +8092,8 @@ sanitizers (like ``-fsanitize=unsigned-integer-overflow``).
return;
}
- // now, handle non-overflow case
- // ...
+ // 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 +
@@ -8110,5 +8109,6 @@ 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.}];
+expressions containing UB.
+}];
}
More information about the cfe-commits
mailing list