artem updated this revision to Diff 546261.
artem added a comment.

Fixed comments and failing test. Added a new compiler-rt test for ubsan


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156821/new/

https://reviews.llvm.org/D156821

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-macros.c
  clang/test/CodeGen/abs-overflow.c
  clang/test/CodeGen/builtin-abs.c
  clang/test/CodeGenCXX/builtins.cpp
  compiler-rt/lib/ubsan/ubsan_handlers.cpp
  compiler-rt/lib/ubsan/ubsan_handlers.h
  compiler-rt/test/ubsan/TestCases/Misc/abs.cpp

Index: compiler-rt/test/ubsan/TestCases/Misc/abs.cpp
===================================================================
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Misc/abs.cpp
@@ -0,0 +1,26 @@
+// REQUIRES: arch=x86_64
+//
+// RUN: %clangxx -fsanitize=builtin -fno-sanitize-recover=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=builtin -w %s -O3 -o %t
+// RUN: %run %t 2>&1 | FileCheck %s --check-prefix=RECOVER
+//
+// RUN: %clangxx -fsanitize=builtin -fsanitize=signed-integer-overflow -fno-sanitize-recover=builtin -w %s -O3 -o %t.abort
+// RUN: not %run %t.abort 2>&1 | FileCheck %s --check-prefix=ABORT
+// RUN: %clangxx -fsanitize=builtin -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>
+
+int main() {
+  // ABORT: abs.cpp:[[@LINE+2]]:17: runtime error: passing a minimal signed integer to abs(), which results in overflow
+  // RECOVER: abs.cpp:[[@LINE+1]]:17: runtime error: passing a minimal signed integer to abs(), which results in overflow
+  __builtin_abs(INT_MIN);
+
+  // RECOVER: abs.cpp:[[@LINE+1]]:18: runtime error: passing a minimal signed integer to abs(), which results in overflow
+  __builtin_labs(LONG_MIN);
+
+  // RECOVER: abs.cpp:[[@LINE+1]]:19: runtime error: passing a minimal signed integer to abs(), which results in overflow
+  __builtin_llabs(LLONG_MIN);
+  return 0;
+}
Index: compiler-rt/lib/ubsan/ubsan_handlers.h
===================================================================
--- compiler-rt/lib/ubsan/ubsan_handlers.h
+++ compiler-rt/lib/ubsan/ubsan_handlers.h
@@ -158,6 +158,7 @@
 enum BuiltinCheckKind : unsigned char {
   BCK_CTZPassedZero,
   BCK_CLZPassedZero,
+  BCK_AbsPassedBadVal,
 };
 
 struct InvalidBuiltinData {
Index: compiler-rt/lib/ubsan/ubsan_handlers.cpp
===================================================================
--- compiler-rt/lib/ubsan/ubsan_handlers.cpp
+++ compiler-rt/lib/ubsan/ubsan_handlers.cpp
@@ -626,9 +626,13 @@
 
   ScopedReport R(Opts, Loc, ET);
 
-  Diag(Loc, DL_Error, ET,
-       "passing zero to %0, which is not a valid argument")
-    << ((Data->Kind == BCK_CTZPassedZero) ? "ctz()" : "clz()");
+  if (Data->Kind == BCK_AbsPassedBadVal) {
+    Diag(Loc, DL_Error, ET,
+         "passing minimal signed integer to abs(), which results in overflow");
+  } else {
+    Diag(Loc, DL_Error, ET, "passing zero to %0, which is not a valid argument")
+        << ((Data->Kind == BCK_CTZPassedZero) ? "ctz()" : "clz()");
+  }
 }
 
 void __ubsan::__ubsan_handle_invalid_builtin(InvalidBuiltinData *Data) {
Index: clang/test/CodeGenCXX/builtins.cpp
===================================================================
--- clang/test/CodeGenCXX/builtins.cpp
+++ clang/test/CodeGenCXX/builtins.cpp
@@ -53,7 +53,8 @@
 extern "C" int __builtin_abs(int); // #3
 
 int x = __builtin_abs(-2);
-// CHECK:  store i32 2, ptr @x, align 4
+// CHECK:  [[X:%.*]] = call i32 @llvm.abs.i32(i32 -2, i1 true)
+// CHECK:  store i32 [[X]], ptr @x, align 4
 
 long y = __builtin_abs(-2l);
 // CHECK:  [[Y:%.+]] = call noundef i64 @_Z13__builtin_absl(i64 noundef -2)
Index: clang/test/CodeGen/builtin-abs.c
===================================================================
--- clang/test/CodeGen/builtin-abs.c
+++ clang/test/CodeGen/builtin-abs.c
@@ -2,27 +2,21 @@
 
 int absi(int x) {
 // CHECK-LABEL: @absi(
-// CHECK:   [[NEG:%.*]] = sub nsw i32 0, [[X:%.*]]
-// CHECK:   [[CMP:%.*]] = icmp slt i32 [[X]], 0
-// CHECK:   [[SEL:%.*]] = select i1 [[CMP]], i32 [[NEG]], i32 [[X]]
+// CHECK:   [[ABS:%.*]] = call i32 @llvm.abs.i32(i32 [[X:%.*]], i1 true)
 //
   return __builtin_abs(x);
 }
 
 long absl(long x) {
 // CHECK-LABEL: @absl(
-// CHECK:   [[NEG:%.*]] = sub nsw i64 0, [[X:%.*]]
-// CHECK:   [[CMP:%.*]] = icmp slt i64 [[X]], 0
-// CHECK:   [[SEL:%.*]] = select i1 [[CMP]], i64 [[NEG]], i64 [[X]]
+// CHECK:   [[ABS:%.*]] = call i64 @llvm.abs.i64(i64 [[X:%.*]], i1 true)
 //
   return __builtin_labs(x);
 }
 
 long long absll(long long x) {
 // CHECK-LABEL: @absll(
-// CHECK:   [[NEG:%.*]] = sub nsw i64 0, [[X:%.*]]
-// CHECK:   [[CMP:%.*]] = icmp slt i64 [[X]], 0
-// CHECK:   [[SEL:%.*]] = select i1 [[CMP]], i64 [[NEG]], i64 [[X]]
+// CHECK:   [[ABS:%.*]] = call i64 @llvm.abs.i64(i64 [[X:%.*]], i1 true)
 //
   return __builtin_llabs(x);
 }
Index: clang/test/CodeGen/abs-overflow.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/abs-overflow.c
@@ -0,0 +1,44 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 2
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm %s -o - -fwrapv -fsanitize=builtin | 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
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm %s -o - -fsanitize=builtin | FileCheck %s --check-prefixes=BOTH-TRAP,CATCH_UB
+// COM: TODO: Support -ftrapv-handler.
+
+extern int abs(int x);
+
+int absi(int x) {
+// WRAPV:   call i32 @llvm.abs.i32(i32 [[X:%.*]], i1 false)
+//
+// 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-NEXT:     @__ubsan_handle_invalid_builtin
+// 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:   call i32 @llvm.abs.i32(i32 [[X:%.*]], i1 false)
+//
+// 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-NEXT:     @__ubsan_handle_invalid_builtin
+// 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);
+}
Index: clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-macros.c
===================================================================
--- clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-macros.c
+++ clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-macros.c
@@ -17,9 +17,7 @@
 // BOTH-NEXT:    [[A_ADDR:%.*]] = alloca i32, align 4
 // BOTH-NEXT:    store i32 [[A:%.*]], ptr [[A_ADDR]], align 4
 // BOTH-NEXT:    [[TMP0:%.*]] = load i32, ptr [[A_ADDR]], align 4
-// BOTH-NEXT:    [[NEG:%.*]] = sub nsw i32 0, [[TMP0]]
-// BOTH-NEXT:    [[ABSCOND:%.*]] = icmp slt i32 [[TMP0]], 0
-// BOTH-NEXT:    [[ABS:%.*]] = select i1 [[ABSCOND]], i32 [[NEG]], i32 [[TMP0]]
+// BOTH-NEXT:    [[ABS:%.*]] = call i32 @llvm.abs.i32(i32 [[TMP0]], i1 true)
 // BOTH-NEXT:    ret i32 [[ABS]]
 signed int testabs(signed int a) {
   return __abs(a);
@@ -30,9 +28,7 @@
 // 64BIT-NEXT:    [[A_ADDR:%.*]] = alloca i64, align 8
 // 64BIT-NEXT:    store i64 [[A:%.*]], ptr [[A_ADDR]], align 8
 // 64BIT-NEXT:    [[TMP0:%.*]] = load i64, ptr [[A_ADDR]], align 8
-// 64BIT-NEXT:    [[NEG:%.*]] = sub nsw i64 0, [[TMP0]]
-// 64BIT-NEXT:    [[ABSCOND:%.*]] = icmp slt i64 [[TMP0]], 0
-// 64BIT-NEXT:    [[ABS:%.*]] = select i1 [[ABSCOND]], i64 [[NEG]], i64 [[TMP0]]
+// 64BIT-NEXT:    [[ABS:%.*]] = call i64 @llvm.abs.i64(i64 [[TMP0]], i1 true)
 // 64BIT-NEXT:    ret i64 [[ABS]]
 //
 // 32BIT-LABEL: @testlabs(
@@ -40,9 +36,7 @@
 // 32BIT-NEXT:    [[A_ADDR:%.*]] = alloca i32, align 4
 // 32BIT-NEXT:    store i32 [[A:%.*]], ptr [[A_ADDR]], align 4
 // 32BIT-NEXT:    [[TMP0:%.*]] = load i32, ptr [[A_ADDR]], align 4
-// 32BIT-NEXT:    [[NEG:%.*]] = sub nsw i32 0, [[TMP0]]
-// 32BIT-NEXT:    [[ABSCOND:%.*]] = icmp slt i32 [[TMP0]], 0
-// 32BIT-NEXT:    [[ABS:%.*]] = select i1 [[ABSCOND]], i32 [[NEG]], i32 [[TMP0]]
+// 32BIT-NEXT:    [[ABS:%.*]] = call i32 @llvm.abs.i32(i32 [[TMP0]], i1 true)
 // 32BIT-NEXT:    ret i32 [[ABS]]
 //
 signed long testlabs(signed long a) {
@@ -54,9 +48,7 @@
 // 64BIT-NEXT:    [[A_ADDR:%.*]] = alloca i64, align 8
 // 64BIT-NEXT:    store i64 [[A:%.*]], ptr [[A_ADDR]], align 8
 // 64BIT-NEXT:    [[TMP0:%.*]] = load i64, ptr [[A_ADDR]], align 8
-// 64BIT-NEXT:    [[NEG:%.*]] = sub nsw i64 0, [[TMP0]]
-// 64BIT-NEXT:    [[ABSCOND:%.*]] = icmp slt i64 [[TMP0]], 0
-// 64BIT-NEXT:    [[ABS:%.*]] = select i1 [[ABSCOND]], i64 [[NEG]], i64 [[TMP0]]
+// 64BIT-NEXT:    [[ABS:%.*]] = call i64 @llvm.abs.i64(i64 [[TMP0]], i1 true)
 // 64BIT-NEXT:    ret i64 [[ABS]]
 //
 // 32BIT-LABEL: @testllabs(
@@ -64,9 +56,7 @@
 // 32BIT-NEXT:    [[A_ADDR:%.*]] = alloca i64, align 8
 // 32BIT-NEXT:    store i64 [[A:%.*]], ptr [[A_ADDR]], align 8
 // 32BIT-NEXT:    [[TMP0:%.*]] = load i64, ptr [[A_ADDR]], align 8
-// 32BIT-NEXT:    [[NEG:%.*]] = sub nsw i64 0, [[TMP0]]
-// 32BIT-NEXT:    [[ABSCOND:%.*]] = icmp slt i64 [[TMP0]], 0
-// 32BIT-NEXT:    [[ABS:%.*]] = select i1 [[ABSCOND]], i64 [[NEG]], i64 [[TMP0]]
+// 32BIT-NEXT:    [[ABS:%.*]] = call i64 @llvm.abs.i64(i64 [[TMP0]], i1 true)
 // 32BIT-NEXT:    ret i64 [[ABS]]
 //
 signed long long testllabs(signed long long a) {
Index: clang/lib/CodeGen/CodeGenFunction.h
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -4657,6 +4657,7 @@
   enum BuiltinCheckKind {
     BCK_CTZPassedZero,
     BCK_CLZPassedZero,
+    BCK_AbsPassedBadVal,
   };
 
   /// Emits an argument for a call to a builtin. If the builtin sanitizer is
Index: clang/lib/CodeGen/CGBuiltin.cpp
===================================================================
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -1782,6 +1782,49 @@
   return ArgValue;
 }
 
+static Value *EmitOverflowCheckedAbs(CodeGenFunction &CGF, const CallExpr *E,
+                                     bool SanitizeBuiltin,
+                                     bool SanitizeOverflow) {
+  Value *ArgValue = CGF.EmitScalarExpr(E->getArg(0));
+
+  // Try to eliminate overflow check.
+  if (auto *VCI = dyn_cast<llvm::ConstantInt>(ArgValue)) {
+    if (!VCI->isMinSignedValue()) {
+      return CGF.Builder.CreateBinaryIntrinsic(
+          Intrinsic::abs, ArgValue, CGF.Builder.getTrue(), nullptr, "abs");
+    }
+  }
+
+  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));
+
+  // Do not emit checks for disabled sanitizers to support recovery.
+  SmallVector<std::pair<Value *, SanitizerMask>, 2> Checks;
+  if (SanitizeBuiltin)
+    Checks.emplace_back(NotOverflow, SanitizerKind::Builtin);
+  if (SanitizeOverflow)
+    Checks.emplace_back(NotOverflow, SanitizerKind::SignedIntegerOverflow);
+
+  // TODO: support -ftrapv-handler.
+  if (!Checks.empty()) {
+    CGF.EmitCheck(Checks, SanitizerHandler::InvalidBuiltin,
+                  {CGF.EmitCheckSourceLocation(E->getArg(0)->getExprLoc()),
+                   ConstantInt::get(CGF.Builder.getInt8Ty(),
+                                    CodeGenFunction::BCK_AbsPassedBadVal)},
+                  std::nullopt);
+  } 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);
@@ -2644,16 +2687,35 @@
     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 SanitizeBuiltin = SanOpts.has(SanitizerKind::Builtin);
+    bool SanitizeOverflow = SanOpts.has(SanitizerKind::SignedIntegerOverflow);
+
+    Value *Result;
+    switch (getLangOpts().getSignedOverflowBehavior()) {
+    case LangOptions::SOB_Defined:
+      Result = Builder.CreateBinaryIntrinsic(
+          Intrinsic::abs, EmitScalarExpr(E->getArg(0)), Builder.getFalse(),
+          nullptr, "abs");
+      break;
+    case LangOptions::SOB_Undefined:
+      if (!SanitizeBuiltin && !SanitizeOverflow) {
+        Result = Builder.CreateBinaryIntrinsic(
+            Intrinsic::abs, EmitScalarExpr(E->getArg(0)), Builder.getTrue(),
+            nullptr, "abs");
+        break;
+      }
+      [[fallthrough]];
+    case LangOptions::SOB_Trapping:
+      Result =
+          EmitOverflowCheckedAbs(*this, E, SanitizeBuiltin, SanitizeOverflow);
+      break;
+    }
     return RValue::get(Result);
   }
   case Builtin::BI__builtin_complex: {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to