Skip to content

Commit

Permalink
Revert "Respect integer overflow handling in abs builtin"
Browse files Browse the repository at this point in the history
This reverts commit 1783185,
which broke the buildbots, starting with when it was first built in https://lab.llvm.org/buildbot/#/builders/85/builds/18390

(N.B. I think the patch is uncovering real bugs; the revert
is simply to keep the tree green and the buildbots useful, because I'm not confident how to
fix-forward all the found bugs.)
  • Loading branch information
thurstond committed Aug 18, 2023
1 parent 9a44eed commit fc06cce
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 146 deletions.
7 changes: 0 additions & 7 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,6 @@ Bug Fixes in This Version
- Fix a hang on valid C code passing a function type as an argument to
``typeof`` to form a function declaration.
(`#64713 <https://github.com/llvm/llvm-project/issues/64713>_`)
- Clang now respects ``-fwrapv`` and ``-ftrapv`` for ``__builtin_abs`` and
``abs`` builtins.
(`#45129 <https://github.com/llvm/llvm-project/issues/45129>`_,
`#45794 <https://github.com/llvm/llvm-project/issues/45794>`_)

Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down Expand Up @@ -293,9 +289,6 @@ Static Analyzer

Sanitizers
----------
- ``-fsanitize=signed-integer-overflow`` now instruments ``__builtin_abs`` and
``abs`` builtins.


Python Binding Changes
----------------------
Expand Down
70 changes: 7 additions & 63 deletions clang/lib/CodeGen/CGBuiltin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1768,49 +1768,6 @@ Value *CodeGenFunction::EmitCheckedArgForBuiltin(const Expr *E,
return ArgValue;
}

static Value *EmitAbs(CodeGenFunction &CGF, Value *ArgValue, bool HasNSW) {
// X < 0 ? -X : X
// TODO: Use phi-node (for better SimplifyCFGPass)
Value *NegOp = CGF.Builder.CreateNeg(ArgValue, "neg", false, HasNSW);
Constant *Zero = llvm::Constant::getNullValue(ArgValue->getType());
Value *CmpResult = CGF.Builder.CreateICmpSLT(ArgValue, Zero, "abscond");
return CGF.Builder.CreateSelect(CmpResult, NegOp, ArgValue, "abs");
}

static Value *EmitOverflowCheckedAbs(CodeGenFunction &CGF, const CallExpr *E,
bool SanitizeOverflow) {
Value *ArgValue = CGF.EmitScalarExpr(E->getArg(0));

// Try to eliminate overflow check.
if (const auto *VCI = dyn_cast<llvm::ConstantInt>(ArgValue)) {
if (!VCI->isMinSignedValue()) {
return EmitAbs(CGF, ArgValue, true);
}
}

CodeGenFunction::SanitizerScope SanScope(&CGF);

Constant *Zero = Constant::getNullValue(ArgValue->getType());
Value *ResultAndOverflow = CGF.Builder.CreateBinaryIntrinsic(
Intrinsic::ssub_with_overflow, Zero, ArgValue);
Value *Result = CGF.Builder.CreateExtractValue(ResultAndOverflow, 0);
Value *NotOverflow = CGF.Builder.CreateNot(
CGF.Builder.CreateExtractValue(ResultAndOverflow, 1));

// TODO: support -ftrapv-handler.
if (SanitizeOverflow) {
CGF.EmitCheck({{NotOverflow, SanitizerKind::SignedIntegerOverflow}},
SanitizerHandler::NegateOverflow,
{CGF.EmitCheckSourceLocation(E->getArg(0)->getExprLoc()),
CGF.EmitCheckTypeDescriptor(E->getType())},
{ArgValue});
} else
CGF.EmitTrapCheck(NotOverflow, SanitizerHandler::SubOverflow);

Value *CmpResult = CGF.Builder.CreateICmpSLT(ArgValue, Zero, "abscond");
return CGF.Builder.CreateSelect(CmpResult, Result, ArgValue, "abs");
}

/// Get the argument type for arguments to os_log_helper.
static CanQualType getOSLogArgType(ASTContext &C, int Size) {
QualType UnsignedTy = C.getIntTypeForBitwidth(Size * 8, /*Signed=*/false);
Expand Down Expand Up @@ -2669,29 +2626,16 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
Builder.CreateCall(CGM.getIntrinsic(Intrinsic::vacopy), {DstPtr, SrcPtr});
return RValue::get(nullptr);
}
case Builtin::BIabs:
case Builtin::BIlabs:
case Builtin::BIllabs:
case Builtin::BI__builtin_abs:
case Builtin::BI__builtin_labs:
case Builtin::BI__builtin_llabs: {
bool SanitizeOverflow = SanOpts.has(SanitizerKind::SignedIntegerOverflow);

Value *Result;
switch (getLangOpts().getSignedOverflowBehavior()) {
case LangOptions::SOB_Defined:
Result = EmitAbs(*this, EmitScalarExpr(E->getArg(0)), false);
break;
case LangOptions::SOB_Undefined:
if (!SanitizeOverflow) {
Result = EmitAbs(*this, EmitScalarExpr(E->getArg(0)), true);
break;
}
[[fallthrough]];
case LangOptions::SOB_Trapping:
Result = EmitOverflowCheckedAbs(*this, E, SanitizeOverflow);
break;
}
// X < 0 ? -X : X
// The negation has 'nsw' because abs of INT_MIN is undefined.
Value *ArgValue = EmitScalarExpr(E->getArg(0));
Value *NegOp = Builder.CreateNSWNeg(ArgValue, "neg");
Constant *Zero = llvm::Constant::getNullValue(ArgValue->getType());
Value *CmpResult = Builder.CreateICmpSLT(ArgValue, Zero, "abscond");
Value *Result = Builder.CreateSelect(CmpResult, NegOp, ArgValue, "abs");
return RValue::get(Result);
}
case Builtin::BI__builtin_complex: {
Expand Down
46 changes: 0 additions & 46 deletions clang/test/CodeGen/abs-overflow.c

This file was deleted.

30 changes: 0 additions & 30 deletions clang/test/ubsan/TestCases/Misc/abs.cpp

This file was deleted.

0 comments on commit fc06cce

Please sign in to comment.