Skip to content

[Clang] Add wraps attribute (for granular integer overflow handling) #86618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,16 @@ Attribute Changes in Clang
- Introduced a new attribute ``[[clang::coro_await_elidable]]`` on coroutine return types
to express elideability at call sites where the coroutine is co_awaited as a prvalue.

- Introduced ``__attribute((wraps))__`` which can be added to type or variable
declarations. Using an attributed type or variable in an arithmetic
expression will define the overflow behavior for that expression as having
two's complement wrap-around. These expressions cannot trigger integer
overflow warnings or sanitizer warnings. They also cannot be optimized away
by some eager UB optimizations.

This attribute is only valid for C, as there are built-in language
alternatives for other languages.

Improvements to Clang's diagnostics
-----------------------------------

Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -4098,6 +4098,9 @@ class BinaryOperator : public Expr {
return getFPFeaturesInEffect(LO).getAllowFEnvAccess();
}

/// Does one of the subexpressions have the wraps attribute?
bool hasWrappingOperand(const ASTContext &Ctx) const;

protected:
BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs, Opcode opc,
QualType ResTy, ExprValueKind VK, ExprObjectKind OK,
Expand Down
2 changes: 2 additions & 0 deletions clang/include/clang/AST/Type.h
Original file line number Diff line number Diff line change
Expand Up @@ -1454,6 +1454,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;

Expand Down
7 changes: 7 additions & 0 deletions clang/include/clang/Basic/Attr.td
Original file line number Diff line number Diff line change
Expand Up @@ -4796,3 +4796,10 @@ def ClspvLibclcBuiltin: InheritableAttr {
let Documentation = [ClspvLibclcBuiltinDoc];
let SimpleHandler = 1;
}

def Wraps : DeclOrTypeAttr {
let Spellings = [Clang<"wraps">];
let Subjects = SubjectList<[Var, TypedefName, Field]>;
let Documentation = [WrapsDocs];
let LangOpts = [COnly];
}
69 changes: 69 additions & 0 deletions clang/include/clang/Basic/AttrDocs.td
Original file line number Diff line number Diff line change
Expand Up @@ -8446,3 +8446,72 @@ Declares that a function potentially allocates heap memory, and prevents any pot
of ``nonallocating`` by the compiler.
}];
}

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;

// check for overflow using a common pattern, however we may accidentally
// perform a real overflow thus triggering sanitizers to step in. Since "A"
// is "wrapping", we can avoid 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", rendering
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.
}];
}

6 changes: 6 additions & 0 deletions clang/include/clang/Basic/DiagnosticGroups.td
Original file line number Diff line number Diff line change
Expand Up @@ -1577,3 +1577,9 @@ def ExtractAPIMisuse : DiagGroup<"extractapi-misuse">;
// Warnings about using the non-standard extension having an explicit specialization
// with a storage class specifier.
def ExplicitSpecializationStorageClass : DiagGroup<"explicit-specialization-storage-class">;

// Warnings regarding the usage of __attribute__((wraps)) on non-integer types.
def UselessWrapsAttr : DiagGroup<"useless-wraps-attribute">;

// Warnings about the wraps attribute getting implicitly discarded
def ImpDiscardedWrapsAttr : DiagGroup<"implicitly-discarded-wraps-attribute">;
7 changes: 7 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -6633,6 +6633,13 @@ def warn_counted_by_attr_elt_type_unknown_size :
Warning<err_counted_by_attr_pointee_unknown_size.Summary>,
InGroup<BoundsSafetyCountedByEltTyUnknownSize>;

def warn_wraps_attr_var_decl_type_not_integer : Warning<
"using attribute 'wraps' with non-integer type '%0' has no function">,
InGroup<UselessWrapsAttr>;
def warn_wraps_attr_maybe_lost : Warning<
"'wraps' attribute may be implicitly discarded when converted to %0">,
InGroup<ImpDiscardedWrapsAttr>;

let CategoryName = "ARC Semantic Issue" in {

// ARC-mode diagnostics.
Expand Down
9 changes: 9 additions & 0 deletions clang/lib/AST/Expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2240,6 +2240,11 @@ bool BinaryOperator::isNullPointerArithmeticExtension(ASTContext &Ctx,
return true;
}

bool BinaryOperator::hasWrappingOperand(const ASTContext &Ctx) const {
return getLHS()->getType().hasWrapsAttr() ||
getRHS()->getType().hasWrapsAttr();
}

SourceLocExpr::SourceLocExpr(const ASTContext &Ctx, SourceLocIdentKind Kind,
QualType ResultTy, SourceLocation BLoc,
SourceLocation RParenLoc,
Expand Down Expand Up @@ -4856,6 +4861,8 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
if (hasStoredFPFeatures())
setStoredFPFeatures(FPFeatures);
setDependence(computeDependence(this));
if (hasWrappingOperand(Ctx))
setType(Ctx.getAttributedType(attr::Wraps, getType(), getType()));
}

BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
Expand All @@ -4874,6 +4881,8 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
if (hasStoredFPFeatures())
setStoredFPFeatures(FPFeatures);
setDependence(computeDependence(this));
if (hasWrappingOperand(Ctx))
setType(Ctx.getAttributedType(attr::Wraps, getType(), getType()));
}

BinaryOperator *BinaryOperator::CreateEmpty(const ASTContext &C,
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2809,7 +2809,7 @@ static bool CheckedIntArithmetic(EvalInfo &Info, const Expr *E,
APSInt Value(Op(LHS.extend(BitWidth), RHS.extend(BitWidth)), false);
Result = Value.trunc(LHS.getBitWidth());
if (Result.extend(BitWidth) != Value) {
if (Info.checkingForUndefinedBehavior())
if (Info.checkingForUndefinedBehavior() && !E->getType().hasWrapsAttr())
Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
diag::warn_integer_constant_overflow)
<< toString(Result, 10, Result.isSigned(), /*formatAsCLiteral=*/false,
Expand Down Expand Up @@ -14412,7 +14412,7 @@ bool IntExprEvaluator::VisitUnaryOperator(const UnaryOperator *E) {
if (!Result.isInt()) return Error(E);
const APSInt &Value = Result.getInt();
if (Value.isSigned() && Value.isMinSignedValue() && E->canOverflow()) {
if (Info.checkingForUndefinedBehavior())
if (Info.checkingForUndefinedBehavior() && !E->getType().hasWrapsAttr())
Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
diag::warn_integer_constant_overflow)
<< toString(Value, 10, Value.isSigned(), /*formatAsCLiteral=*/false,
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/AST/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2850,6 +2850,10 @@ bool QualType::isWebAssemblyFuncrefType() const {
getAddressSpace() == LangAS::wasm_funcref;
}

bool QualType::hasWrapsAttr() const {
return !isNull() && getTypePtr()->hasAttr(attr::Wraps);
}

QualType::PrimitiveDefaultInitializeKind
QualType::isNonTrivialToPrimitiveDefaultInitialize() const {
if (const auto *RT =
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/AST/TypePrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2024,6 +2024,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;
Expand Down
46 changes: 34 additions & 12 deletions clang/lib/CodeGen/CGExprScalar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ struct BinOpInfo {
}
return false;
}

/// Does the BinaryOperator have the wraps attribute?
/// If so, we can elide overflow sanitizer checks.
bool hasWrappingOperand() const { return E->getType().hasWrapsAttr(); }
};

static bool MustVisitNullValue(const Expr *E) {
Expand Down Expand Up @@ -750,7 +754,8 @@ class ScalarExprEmitter

// Binary Operators.
Value *EmitMul(const BinOpInfo &Ops) {
if (Ops.Ty->isSignedIntegerOrEnumerationType()) {
if (Ops.Ty->isSignedIntegerOrEnumerationType() &&
!Ops.hasWrappingOperand()) {
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
case LangOptions::SOB_Defined:
if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
Expand Down Expand Up @@ -786,7 +791,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()) {
Expand Down Expand Up @@ -1118,7 +1124,7 @@ void ScalarExprEmitter::EmitIntegerTruncationCheck(Value *Src, QualType SrcType,
// If the comparison result is 'i1 false', then the truncation was lossy.

// Do we care about this type of truncation?
if (!CGF.SanOpts.has(Check.second.second))
if (!CGF.SanOpts.has(Check.second.second) || DstType.hasWrapsAttr())
return;

llvm::Constant *StaticArgs[] = {
Expand Down Expand Up @@ -1351,6 +1357,14 @@ void CodeGenFunction::EmitBitfieldConversionCheck(Value *Src, QualType SrcType,
bool SrcSigned = SrcType->isSignedIntegerOrEnumerationType();
bool DstSigned = DstType->isSignedIntegerOrEnumerationType();

bool SrcWraps = SrcType.hasWrapsAttr();
bool DstWraps = DstType.hasWrapsAttr();

// The wraps attribute should silence any sanitizer warnings
// regarding truncation or overflow
if (SrcWraps || DstWraps)
return;

CodeGenFunction::SanitizerScope SanScope(this);

std::pair<ScalarExprEmitter::ImplicitConversionCheckKind,
Expand Down Expand Up @@ -2926,6 +2940,9 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV,
bool excludeOverflowPattern =
matchesPostDecrInWhile(E, isInc, isPre, CGF.getContext());

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.");
Expand Down Expand Up @@ -2982,11 +2999,12 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV,
// Note that signed integer inc/dec with width less than int can't
// overflow because of promotion rules; we're just eliding a few steps
// here.
} else if (E->canOverflow() && type->isSignedIntegerOrEnumerationType()) {
} else if (E->canOverflow() && type->isSignedIntegerOrEnumerationType() &&
!Ops.hasWrappingOperand()) {
value = EmitIncDecConsiderOverflowBehavior(E, value, isInc);
} else if (E->canOverflow() && type->isUnsignedIntegerType() &&
CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) &&
!excludeOverflowPattern) {
!Ops.hasWrappingOperand() && !excludeOverflowPattern) {
value = EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(
E, value, isInc, E->getFPFeaturesInEffect(CGF.getLangOpts())));
} else {
Expand Down Expand Up @@ -3775,7 +3793,8 @@ Value *ScalarExprEmitter::EmitDiv(const BinOpInfo &Ops) {
if ((CGF.SanOpts.has(SanitizerKind::IntegerDivideByZero) ||
CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) &&
Ops.Ty->isIntegerType() &&
(Ops.mayHaveIntegerDivisionByZero() || Ops.mayHaveIntegerOverflow())) {
(Ops.mayHaveIntegerDivisionByZero() || Ops.mayHaveIntegerOverflow()) &&
!Ops.hasWrappingOperand()) {
llvm::Value *Zero = llvm::Constant::getNullValue(ConvertType(Ops.Ty));
EmitUndefinedBehaviorIntegerDivAndRemCheck(Ops, Zero, true);
} else if (CGF.SanOpts.has(SanitizerKind::FloatDivideByZero) &&
Expand Down Expand Up @@ -3824,7 +3843,8 @@ Value *ScalarExprEmitter::EmitRem(const BinOpInfo &Ops) {
if ((CGF.SanOpts.has(SanitizerKind::IntegerDivideByZero) ||
CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) &&
Ops.Ty->isIntegerType() &&
(Ops.mayHaveIntegerDivisionByZero() || Ops.mayHaveIntegerOverflow())) {
(Ops.mayHaveIntegerDivisionByZero() || Ops.mayHaveIntegerOverflow()) &&
!Ops.hasWrappingOperand()) {
CodeGenFunction::SanitizerScope SanScope(&CGF);
llvm::Value *Zero = llvm::Constant::getNullValue(ConvertType(Ops.Ty));
EmitUndefinedBehaviorIntegerDivAndRemCheck(Ops, Zero, false);
Expand Down Expand Up @@ -4189,7 +4209,7 @@ Value *ScalarExprEmitter::EmitAdd(const BinOpInfo &op) {
op.RHS->getType()->isPointerTy())
return emitPointerArithmetic(CGF, op, CodeGenFunction::NotSubtraction);

if (op.Ty->isSignedIntegerOrEnumerationType()) {
if (op.Ty->isSignedIntegerOrEnumerationType() && !op.hasWrappingOperand()) {
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
case LangOptions::SOB_Defined:
if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
Expand Down Expand Up @@ -4222,7 +4242,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()) {
Expand Down Expand Up @@ -4345,7 +4365,7 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) {
Value *ScalarExprEmitter::EmitSub(const BinOpInfo &op) {
// The LHS is always a pointer if either side is.
if (!op.LHS->getType()->isPointerTy()) {
if (op.Ty->isSignedIntegerOrEnumerationType()) {
if (op.Ty->isSignedIntegerOrEnumerationType() && !op.hasWrappingOperand()) {
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
case LangOptions::SOB_Defined:
if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
Expand Down Expand Up @@ -4378,7 +4398,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()) {
Expand Down Expand Up @@ -4498,7 +4519,8 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) {
bool SanitizeSignedBase = CGF.SanOpts.has(SanitizerKind::ShiftBase) &&
Ops.Ty->hasSignedIntegerRepresentation() &&
!CGF.getLangOpts().isSignedOverflowDefined() &&
!CGF.getLangOpts().CPlusPlus20;
!CGF.getLangOpts().CPlusPlus20 &&
!Ops.hasWrappingOperand();
bool SanitizeUnsignedBase =
CGF.SanOpts.has(SanitizerKind::UnsignedShiftBase) &&
Ops.Ty->hasUnsignedIntegerRepresentation();
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/Sema/Sema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,9 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty,
QualType ExprTy = Context.getCanonicalType(E->getType());
QualType TypeTy = Context.getCanonicalType(Ty);

if (E->getType().getTypePtr()->isIntegerType() && E->getType().hasWrapsAttr())
Ty = Context.getAttributedType(attr::Wraps, Ty, Ty);

if (ExprTy == TypeTy)
return E;

Expand Down
Loading
Loading