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; -} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
