[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 16:27:59 PDT 2024
https://github.com/JustinStitt updated https://github.com/llvm/llvm-project/pull/86618
>From 50e7b1039e514dacc05bb8cd9ff9a3e3df9ed24d 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/13] 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 2b3bafa1c30548..fa7acb894cd76f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -296,6 +296,15 @@ Attribute Changes in Clang
- Clang now warns that the ``exclude_from_explicit_instantiation`` attribute
is ignored when applied to a local class or a member thereof.
+- 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 a0bbe5861c5722..5f6e5a8de79954 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -8057,3 +8057,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 63e951daec7477..c7bc69462133e6 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6537,6 +6537,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 64607b91acbfc9..b5fbf3cdcde3c8 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3665,6 +3665,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 9eec7edc9d1a3e..d8af147657e516 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 de3c2a63913e94..1461ccf344f233 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 9602f448e94279..10cc69640a7e6b 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -1965,6 +1965,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 40a5cd20c3d715..59202f0066872d 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -156,6 +156,15 @@ struct BinOpInfo {
}
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) {
@@ -735,6 +744,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:
@@ -2831,6 +2845,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.");
@@ -2887,10 +2904,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 {
@@ -3679,7 +3698,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) &&
@@ -3728,7 +3748,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);
@@ -4093,6 +4114,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:
@@ -4249,6 +4275,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:
@@ -4402,7 +4432,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 363ae93cb62df1..2e67c9397ed0c4 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -4455,6 +4455,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();
@@ -9703,10 +9711,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 fddc3545ecb61c..da6452ab61f190 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -6958,6 +6958,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.
@@ -8948,6 +8960,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 dbd3a2be9189b8b7bfc3b4c7f62d3eb2908c7cee 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/13] 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 59202f0066872d..0708029067d22f 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -1117,7 +1117,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 51757f4cf727d6..473e99263e9e8e 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -16479,7 +16479,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;
@@ -16527,7 +16528,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 039c7d1ef7a412612a235d6f5d690072641cc797 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/13] 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 2e67c9397ed0c4..f4ad5aa0c66f6e 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -4456,6 +4456,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 da6452ab61f190..a5e24e25fa57cc 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -6963,6 +6963,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 b21987bb668262afb0d29e40f5076b80f97d1d1b 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/13] 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 5f6e5a8de79954..72a7a903b86d8e 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -8084,6 +8084,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
@@ -8106,6 +8107,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 5b65165e43dfbdf53cdb4d86df5eb06aec181343 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/13] 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 d8af147657e516..e666bb2ecdac16 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 0708029067d22f..3b6b88c80f368a 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -159,7 +159,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);
@@ -746,7 +746,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()) {
@@ -2906,11 +2906,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 {
@@ -3700,7 +3700,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) &&
@@ -3750,7 +3750,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);
@@ -4117,7 +4117,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()) {
@@ -4278,7 +4278,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()) {
@@ -4433,7 +4433,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 7c6581f2ccf23a55df614553830d4ed485ec073d 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/13] 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 fa7acb894cd76f..1365d713a8972e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -303,7 +303,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 f4ad5aa0c66f6e..2e67c9397ed0c4 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -4456,11 +4456,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 a5e24e25fa57cc..da6452ab61f190 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -6963,11 +6963,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 8e2e8c268da4df4549f6030c4ddb3b0f4abf7421 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/13] 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 e666bb2ecdac16..bb5aea2f08626d 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 3b6b88c80f368a..9d9ab086e70cdb 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -160,10 +160,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 d92d8963eef84dbd5add85f7b2cc5afebc96326d 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/13] 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 9d9ab086e70cdb..05aea60d2e9c91 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -159,9 +159,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) {
@@ -4430,7 +4428,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 d80d76ab6e0d5a6d51e161654350637daa8a2820 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/13] 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 1461ccf344f233..1ac499d2f573f8 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 05aea60d2e9c91..069403e84316d8 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -1113,7 +1113,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 473e99263e9e8e..5253a99594f228 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -16480,7 +16480,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;
@@ -16528,7 +16528,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 4f8d5f50099ec8a23a55aaf558e53ddfa78f298b 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/13] 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 1ac499d2f573f8..330b6ebf7c88ac 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 069403e84316d8..bb8385579841b8 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -1112,8 +1112,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 5253a99594f228..3ef946b2b00f47 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -16479,8 +16479,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 a898deceae1ec6d58c35dd7c97870abd45e283ec 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/13] 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 72a7a903b86d8e..12e4d14514687e 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -8076,6 +8076,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() {
@@ -8084,7 +8085,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
@@ -8092,9 +8092,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.
@@ -8104,10 +8105,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 +
@@ -8123,5 +8122,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.
+}];
}
>From 872e5d6a2791c7de1268ff3d0bc090578f42067c Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Mon, 22 Apr 2024 23:25:02 +0000
Subject: [PATCH 12/13] remove early returns and cleanup typos
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/include/clang/AST/Expr.h | 2 +-
clang/include/clang/Sema/Sema.h | 4 ----
clang/lib/CodeGen/CGExprScalar.cpp | 35 ++++++++++--------------------
clang/lib/Sema/SemaDeclAttr.cpp | 6 +----
4 files changed, 14 insertions(+), 33 deletions(-)
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 6d053cf4304f07..b01d953be39e4a 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -4078,7 +4078,7 @@ class BinaryOperator : public Expr {
return HasFPFeatures * sizeof(FPOptionsOverride);
}
- /// Do one of the subexpressions have the wraps attribute?
+ /// Does one of the subexpressions have the wraps attribute?
bool hasWrappingOperand(const ASTContext &Ctx) const;
};
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index b5fbf3cdcde3c8..64607b91acbfc9 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3665,10 +3665,6 @@ 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/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index bb8385579841b8..2c5e0523a1fbe7 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -158,7 +158,7 @@ struct BinOpInfo {
}
/// Does the BinaryOperator have the wraps attribute?
- /// If so, we can ellide overflow sanitizer checks.
+ /// If so, we can elide overflow sanitizer checks.
bool hasWrappingOperand() const { return E->getType().hasWrapsAttr(); }
};
@@ -739,12 +739,8 @@ class ScalarExprEmitter
// Binary Operators.
Value *EmitMul(const BinOpInfo &Ops) {
- if ((Ops.Ty->isSignedIntegerOrEnumerationType() ||
- Ops.Ty->isUnsignedIntegerType()) &&
- Ops.hasWrappingOperand())
- return Builder.CreateMul(Ops.LHS, Ops.RHS, "mul");
-
- if (Ops.Ty->isSignedIntegerOrEnumerationType()) {
+ if (Ops.Ty->isSignedIntegerOrEnumerationType() &&
+ !Ops.hasWrappingOperand()) {
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
case LangOptions::SOB_Defined:
if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
@@ -780,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()) {
@@ -2840,8 +2837,8 @@ 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())));
+ BinOpInfo Ops = createBinOpInfoFromIncDec(
+ E, value, isInc, E->getFPFeaturesInEffect(CGF.getLangOpts()));
if (CGF.getContext().isPromotableIntegerType(type)) {
promotedType = CGF.getContext().getPromotedIntegerType(type);
@@ -4109,12 +4106,7 @@ Value *ScalarExprEmitter::EmitAdd(const BinOpInfo &op) {
op.RHS->getType()->isPointerTy())
return emitPointerArithmetic(CGF, op, CodeGenFunction::NotSubtraction);
- if ((op.Ty->isSignedIntegerOrEnumerationType() ||
- op.Ty->isUnsignedIntegerType()) &&
- op.hasWrappingOperand())
- return Builder.CreateAdd(op.LHS, op.RHS, "add");
-
- if (op.Ty->isSignedIntegerOrEnumerationType()) {
+ if (op.Ty->isSignedIntegerOrEnumerationType() && !op.hasWrappingOperand()) {
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
case LangOptions::SOB_Defined:
if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
@@ -4147,7 +4139,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()) {
@@ -4270,11 +4262,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() ||
- op.Ty->isUnsignedIntegerType()) &&
- op.hasWrappingOperand())
- return Builder.CreateSub(op.LHS, op.RHS, "sub");
- if (op.Ty->isSignedIntegerOrEnumerationType()) {
+ if (op.Ty->isSignedIntegerOrEnumerationType() && !op.hasWrappingOperand()) {
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
case LangOptions::SOB_Defined:
if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
@@ -4307,7 +4295,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()) {
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 2e67c9397ed0c4..bf95e63cbfabd7 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -4456,11 +4456,7 @@ void Sema::AddAlignValueAttr(Decl *D, const AttributeCommonInfo &CI, Expr *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));
+ D->addAttr(::new (S.Context) WrapsAttr(S.Context, AL));
}
static void handleAlignedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
>From 1aa85e159d5768a7059bb35e457af28a3753bf66 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Mon, 22 Apr 2024 23:27:44 +0000
Subject: [PATCH 13/13] cleanup test pattern
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/test/Sema/attr-wraps.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/clang/test/Sema/attr-wraps.c b/clang/test/Sema/attr-wraps.c
index 97aff317120633..7a3de9a0988f0c 100644
--- a/clang/test/Sema/attr-wraps.c
+++ b/clang/test/Sema/attr-wraps.c
@@ -2,8 +2,7 @@
// expected-no-diagnostics
typedef int __attribute__((wraps)) wrapping_int;
-void foo(void) {
+int foo(void) {
const wrapping_int A = 1;
- int D = 2147483647 + A;
- (void)D;
+ return 2147483647 + A;
}
More information about the cfe-commits
mailing list