Respect integer overflow handling in abs builtin

Currenly both Clang and GCC support the following set of flags that
control code gen of signed overflow:

* -fwrapv: overflow is defined as in two-complement
* -ftrapv: overflow traps
* -fsanitize=signed-integer-overflow: if undefined (no -fwrapv), then
overflow behaviour is controlled by UBSan runtime, overrides -ftrapv.

However, clang ignores these flags for __builtin_abs(int) and its
higher-width versions, so passing minimum integer value always causes
poison.

The same holds for *abs(), which are not handled in frontend at all but
folded to llvm.abs.* intrinsics during InstCombinePass. The intrinsics
are not instrumented by UBSan, so the functions need special handling
as well.

This patch does a few things:

* Handle *abs() in CGBuiltin the same way as __builtin_*abs()
* -fsanitize=signed-integer-overflow now properly instruments abs() with
UBSan
* -fwrapv and -ftrapv handling for abs() is made consistent with GCC

Fixes llvm#45129
Fixes llvm#45794

Differential Revision: https://reviews.llvm.org/D156821

Author: artem

clang/docs/ReleaseNotes.rst

@@ -152,6 +152,10 @@ 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
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -283,6 +287,9 @@ Static Analyzer
 
 Sanitizers
 ----------
+- ``-fsanitize=signed-integer-overflow`` now instruments ``__builtin_abs`` and
+  ``abs`` builtins.
+
 
 Python Binding Changes
 ----------------------

clang/lib/CodeGen/CGBuiltin.cpp

@@ -1768,6 +1768,49 @@ 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);
@@ -2626,16 +2669,29 @@ 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: {
-    // 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");
+    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;
+    }
     return RValue::get(Result);
   }
   case Builtin::BI__builtin_complex: {

clang/test/CodeGen/abs-overflow.c

@@ -0,0 +1,46 @@
+// 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);
+}

clang/test/ubsan/TestCases/Misc/abs.cpp

@@ -0,0 +1,30 @@
+// 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;
+}