[clang] fc06cce - Revert "Respect integer overflow handling in abs builtin"
Thurston Dang via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 18 13:00:53 PDT 2023
Author: Thurston Dang
Date: 2023-08-18T19:59:34Z
New Revision: fc06cce30d2b7d49778b9a27420ca239e0c49856
URL: https://github.com/llvm/llvm-project/commit/fc06cce30d2b7d49778b9a27420ca239e0c49856
DIFF: https://github.com/llvm/llvm-project/commit/fc06cce30d2b7d49778b9a27420ca239e0c49856.diff
LOG: Revert "Respect integer overflow handling in abs builtin"
This reverts commit 1783185790de29b24d3850d33d9a9d586e6bbd39,
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.)
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/CodeGen/CGBuiltin.cpp
Removed:
clang/test/CodeGen/abs-overflow.c
clang/test/ubsan/TestCases/Misc/abs.cpp
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e34fc2e10b2df0..054c06ffa0f5c9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -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
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -293,9 +289,6 @@ Static Analyzer
Sanitizers
----------
-- ``-fsanitize=signed-integer-overflow`` now instruments ``__builtin_abs`` and
- ``abs`` builtins.
-
Python Binding Changes
----------------------
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 4ded5a10d529ae..5a183d3553279e 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -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);
@@ -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: {
diff --git a/clang/test/CodeGen/abs-overflow.c b/clang/test/CodeGen/abs-overflow.c
deleted file mode 100644
index 76ca7671ccf446..00000000000000
--- a/clang/test/CodeGen/abs-overflow.c
+++ /dev/null
@@ -1,46 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm %s -o - -fwrapv -fsanitize=signed-integer-overflow | FileCheck %s --check-prefix=WRAPV
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm %s -o - -ftrapv | FileCheck %s --check-prefixes=BOTH-TRAP,TRAPV
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm %s -o - -fsanitize=signed-integer-overflow | FileCheck %s --check-prefixes=BOTH-TRAP,CATCH_UB
-// COM: TODO: Support -ftrapv-handler.
-
-extern int abs(int x);
-
-int absi(int x) {
-// WRAPV: [[NEG:%.*]] = sub i32 0, [[X:%.*]]
-// WRAPV: [[CMP:%.*]] = icmp slt i32 [[X]], 0
-// WRAPV: [[SEL:%.*]] = select i1 [[CMP]], i32 [[NEG]], i32 [[X]]
-//
-// BOTH-TRAP: [[NEG:%.*]] = call { i32, i1 } @llvm.ssub.with.overflow.i32(i32 0, i32 [[X:%.*]])
-// BOTH-TRAP: [[NEGV:%.*]] = extractvalue { i32, i1 } [[NEG]], 0
-// BOTH-TRAP: [[OFL:%.*]] = extractvalue { i32, i1 } [[NEG]], 1
-// BOTH-TRAP: [[NOFL:%.*]] = xor i1 [[OFL]], true
-// BOTH-TRAP: br i1 [[NOFL]], label %[[CONT:.*]], label %[[TRAP:[a-zA-Z_.]*]]
-// BOTH-TRAP: [[TRAP]]:
-// TRAPV-NEXT: llvm.ubsantrap
-// CATCH_UB: @__ubsan_handle_negate_overflow
-// BOTH-TRAP-NEXT: unreachable
-// BOTH-TRAP: [[CONT]]:
-// BOTH-TRAP-NEXT: [[ABSCOND:%.*]] = icmp slt i32 [[X]], 0
-// BOTH-TRAP-NEXT: select i1 [[ABSCOND]], i32 [[NEGV]], i32 [[X]]
- return abs(x);
-}
-
-int babsi(int x) {
-// WRAPV: [[NEG:%.*]] = sub i32 0, [[X:%.*]]
-// WRAPV: [[CMP:%.*]] = icmp slt i32 [[X]], 0
-// WRAPV: [[SEL:%.*]] = select i1 [[CMP]], i32 [[NEG]], i32 [[X]]
-//
-// BOTH-TRAP: [[NEG:%.*]] = call { i32, i1 } @llvm.ssub.with.overflow.i32(i32 0, i32 [[X:%.*]])
-// BOTH-TRAP: [[NEGV:%.*]] = extractvalue { i32, i1 } [[NEG]], 0
-// BOTH-TRAP: [[OFL:%.*]] = extractvalue { i32, i1 } [[NEG]], 1
-// BOTH-TRAP: [[NOFL:%.*]] = xor i1 [[OFL]], true
-// BOTH-TRAP: br i1 [[NOFL]], label %[[CONT:.*]], label %[[TRAP:[a-zA-Z_.]*]]
-// BOTH-TRAP: [[TRAP]]:
-// TRAPV-NEXT: llvm.ubsantrap
-// CATCH_UB: @__ubsan_handle_negate_overflow
-// BOTH-TRAP-NEXT: unreachable
-// BOTH-TRAP: [[CONT]]:
-// BOTH-TRAP-NEXT: [[ABSCOND:%.*]] = icmp slt i32 [[X]], 0
-// BOTH-TRAP-NEXT: select i1 [[ABSCOND]], i32 [[NEGV]], i32 [[X]]
- return __builtin_abs(x);
-}
diff --git a/clang/test/ubsan/TestCases/Misc/abs.cpp b/clang/test/ubsan/TestCases/Misc/abs.cpp
deleted file mode 100644
index 198153cd369950..00000000000000
--- a/clang/test/ubsan/TestCases/Misc/abs.cpp
+++ /dev/null
@@ -1,30 +0,0 @@
-// REQUIRES: arch=x86_64
-//
-// RUN: %clangxx -fsanitize=signed-integer-overflow -w %s -O3 -o %t
-// RUN: %run %t 2>&1 | FileCheck %s --check-prefix=RECOVER
-//
-// RUN: %clangxx -fsanitize=signed-integer-overflow -fno-sanitize-recover=signed-integer-overflow -w %s -O3 -o %t.abort
-// RUN: not %run %t.abort 2>&1 | FileCheck %s --check-prefix=ABORT
-
-#include <limits.h>
-#include <stdlib.h>
-
-int main() {
- // ABORT: abs.cpp:[[@LINE+3]]:17: runtime error: negation of -{{[0-9]+}} cannot be represented in type 'int'; cast to an unsigned type to negate this value to itself
- // RECOVER: abs.cpp:[[@LINE+2]]:17: runtime error: negation of -{{[0-9]+}} cannot be represented in type 'int'; cast to an unsigned type to negate this value to itself
- // RECOVER: abs.cpp:[[@LINE+2]]:7: runtime error: negation of -{{[0-9]+}} cannot be represented in type 'int'; cast to an unsigned type to negate this value to itself
- __builtin_abs(INT_MIN);
- abs(INT_MIN);
-
- // RECOVER: abs.cpp:[[@LINE+2]]:18: runtime error: negation of -{{[0-9]+}} cannot be represented in type 'long'; cast to an unsigned type to negate this value to itself
- // RECOVER: abs.cpp:[[@LINE+2]]:8: runtime error: negation of -{{[0-9]+}} cannot be represented in type 'long'; cast to an unsigned type to negate this value to itself
- __builtin_labs(LONG_MIN);
- labs(LONG_MIN);
-
- // RECOVER: abs.cpp:[[@LINE+2]]:19: runtime error: negation of -{{[0-9]+}} cannot be represented in type 'long long'; cast to an unsigned type to negate this value to itself
- // RECOVER: abs.cpp:[[@LINE+2]]:9: runtime error: negation of -{{[0-9]+}} cannot be represented in type 'long long'; cast to an unsigned type to negate this value to itself
- __builtin_llabs(LLONG_MIN);
- llabs(LLONG_MIN);
-
- return 0;
-}
More information about the cfe-commits
mailing list