[clang] 8799ebb - [clang] Fix or emit diagnostic for checked arithmetic builtins with

Erich Keane via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 15 06:52:18 PDT 2020


Author: Jeff Mott
Date: 2020-06-15T06:51:54-07:00
New Revision: 8799ebbc1f03a348f732fc14242ad4c395076bcc

URL: https://github.com/llvm/llvm-project/commit/8799ebbc1f03a348f732fc14242ad4c395076bcc
DIFF: https://github.com/llvm/llvm-project/commit/8799ebbc1f03a348f732fc14242ad4c395076bcc.diff

LOG: [clang] Fix or emit diagnostic for checked arithmetic builtins with
_ExtInt types

- Fix computed size for _ExtInt types passed to checked arithmetic
  builtins.
- Emit diagnostic when signed _ExtInt larger than 128-bits is passed
    to __builtin_mul_overflow.
- Change Sema checks for builtins to accept placeholder types.

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

Added: 
    clang/test/Sema/builtins-overflow.m

Modified: 
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/lib/CodeGen/CGBuiltin.cpp
    clang/lib/Sema/SemaChecking.cpp
    clang/test/CodeGen/builtins-overflow.c
    clang/test/Sema/builtins-overflow.c

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a295b3778cf0..b0a6184906c2 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7948,6 +7948,9 @@ def err_overflow_builtin_must_be_int : Error<
 def err_overflow_builtin_must_be_ptr_int : Error<
   "result argument to overflow builtin must be a pointer "
   "to a non-const integer (%0 invalid)">;
+def err_overflow_builtin_ext_int_max_size : Error<
+  "__builtin_mul_overflow does not support signed _ExtInt operands of more "
+  "than %0 bits">;
 
 def err_atomic_load_store_uses_lib : Error<
   "atomic %select{load|store}0 requires runtime support that is not "

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 85bff3d9a674..209b5a2b00e3 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -589,7 +589,9 @@ static WidthAndSignedness
 getIntegerWidthAndSignedness(const clang::ASTContext &context,
                              const clang::QualType Type) {
   assert(Type->isIntegerType() && "Given type is not an integer.");
-  unsigned Width = Type->isBooleanType() ? 1 : context.getTypeInfo(Type).Width;
+  unsigned Width = Type->isBooleanType()  ? 1
+                   : Type->isExtIntType() ? context.getIntWidth(Type)
+                                          : context.getTypeInfo(Type).Width;
   bool Signed = Type->isSignedIntegerType();
   return {Width, Signed};
 }

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 85126e029244..22f25f9042d2 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -283,48 +283,60 @@ static bool SemaBuiltinAlignment(Sema &S, CallExpr *TheCall, unsigned ID) {
   return false;
 }
 
-static bool SemaBuiltinOverflow(Sema &S, CallExpr *TheCall) {
+static bool SemaBuiltinOverflow(Sema &S, CallExpr *TheCall,
+                                unsigned BuiltinID) {
   if (checkArgCount(S, TheCall, 3))
     return true;
 
   // First two arguments should be integers.
   for (unsigned I = 0; I < 2; ++I) {
-    ExprResult Arg = TheCall->getArg(I);
+    ExprResult Arg = S.DefaultFunctionArrayLvalueConversion(TheCall->getArg(I));
+    if (Arg.isInvalid()) return true;
+    TheCall->setArg(I, Arg.get());
+
     QualType Ty = Arg.get()->getType();
     if (!Ty->isIntegerType()) {
       S.Diag(Arg.get()->getBeginLoc(), diag::err_overflow_builtin_must_be_int)
           << Ty << Arg.get()->getSourceRange();
       return true;
     }
-    InitializedEntity Entity = InitializedEntity::InitializeParameter(
-        S.getASTContext(), Ty, /*consume*/ false);
-    Arg = S.PerformCopyInitialization(Entity, SourceLocation(), Arg);
-    if (Arg.isInvalid())
-      return true;
-    TheCall->setArg(I, Arg.get());
   }
 
   // Third argument should be a pointer to a non-const integer.
   // IRGen correctly handles volatile, restrict, and address spaces, and
   // the other qualifiers aren't possible.
   {
-    ExprResult Arg = TheCall->getArg(2);
+    ExprResult Arg = S.DefaultFunctionArrayLvalueConversion(TheCall->getArg(2));
+    if (Arg.isInvalid()) return true;
+    TheCall->setArg(2, Arg.get());
+
     QualType Ty = Arg.get()->getType();
     const auto *PtrTy = Ty->getAs<PointerType>();
-    if (!(PtrTy && PtrTy->getPointeeType()->isIntegerType() &&
-          !PtrTy->getPointeeType().isConstQualified())) {
+    if (!PtrTy ||
+        !PtrTy->getPointeeType()->isIntegerType() ||
+        PtrTy->getPointeeType().isConstQualified()) {
       S.Diag(Arg.get()->getBeginLoc(),
              diag::err_overflow_builtin_must_be_ptr_int)
-          << Ty << Arg.get()->getSourceRange();
+        << Ty << Arg.get()->getSourceRange();
       return true;
     }
-    InitializedEntity Entity = InitializedEntity::InitializeParameter(
-        S.getASTContext(), Ty, /*consume*/ false);
-    Arg = S.PerformCopyInitialization(Entity, SourceLocation(), Arg);
-    if (Arg.isInvalid())
-      return true;
-    TheCall->setArg(2, Arg.get());
   }
+
+  // Disallow signed ExtIntType args larger than 128 bits to mul function until
+  // we improve backend support.
+  if (BuiltinID == Builtin::BI__builtin_mul_overflow) {
+    for (unsigned I = 0; I < 3; ++I) {
+      const auto Arg = TheCall->getArg(I);
+      // Third argument will be a pointer.
+      auto Ty = I < 2 ? Arg->getType() : Arg->getType()->getPointeeType();
+      if (Ty->isExtIntType() && Ty->isSignedIntegerType() &&
+          S.getASTContext().getIntWidth(Ty) > 128)
+        return S.Diag(Arg->getBeginLoc(),
+                      diag::err_overflow_builtin_ext_int_max_size)
+               << 128;
+    }
+  }
+
   return false;
 }
 
@@ -1728,7 +1740,7 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
   case Builtin::BI__builtin_add_overflow:
   case Builtin::BI__builtin_sub_overflow:
   case Builtin::BI__builtin_mul_overflow:
-    if (SemaBuiltinOverflow(*this, TheCall))
+    if (SemaBuiltinOverflow(*this, TheCall, BuiltinID))
       return ExprError();
     break;
   case Builtin::BI__builtin_operator_new:

diff  --git a/clang/test/CodeGen/builtins-overflow.c b/clang/test/CodeGen/builtins-overflow.c
index 79a3186b7457..b7f8d7437e21 100644
--- a/clang/test/CodeGen/builtins-overflow.c
+++ b/clang/test/CodeGen/builtins-overflow.c
@@ -41,6 +41,20 @@ int test_add_overflow_int_int_int(int x, int y) {
   return r;
 }
 
+int test_add_overflow_xint31_xint31_xint31(_ExtInt(31) x, _ExtInt(31) y) {
+  // CHECK-LABEL: define {{(dso_local )?}}i32 @test_add_overflow_xint31_xint31_xint31({{.+}})
+  // CHECK-NOT: ext
+  // CHECK: [[S:%.+]] = call { i31, i1 } @llvm.sadd.with.overflow.i31(i31 %{{.+}}, i31 %{{.+}})
+  // CHECK-DAG: [[C:%.+]] = extractvalue { i31, i1 } [[S]], 1
+  // CHECK-DAG: [[Q:%.+]] = extractvalue { i31, i1 } [[S]], 0
+  // CHECK: store i31 [[Q]], i31*
+  // CHECK: br i1 [[C]]
+  _ExtInt(31) r;
+  if (__builtin_add_overflow(x, y, &r))
+    overflowed();
+  return r;
+}
+
 unsigned test_sub_overflow_uint_uint_uint(unsigned x, unsigned y) {
   // CHECK-LABEL: define {{(dso_local )?}}i32 @test_sub_overflow_uint_uint_uint
   // CHECK-NOT: ext
@@ -69,6 +83,20 @@ int test_sub_overflow_int_int_int(int x, int y) {
   return r;
 }
 
+int test_sub_overflow_xint31_xint31_xint31(_ExtInt(31) x, _ExtInt(31) y) {
+  // CHECK-LABEL: define {{(dso_local )?}}i32 @test_sub_overflow_xint31_xint31_xint31({{.+}})
+  // CHECK-NOT: ext
+  // CHECK: [[S:%.+]] = call { i31, i1 } @llvm.ssub.with.overflow.i31(i31 %{{.+}}, i31 %{{.+}})
+  // CHECK-DAG: [[C:%.+]] = extractvalue { i31, i1 } [[S]], 1
+  // CHECK-DAG: [[Q:%.+]] = extractvalue { i31, i1 } [[S]], 0
+  // CHECK: store i31 [[Q]], i31*
+  // CHECK: br i1 [[C]]
+  _ExtInt(31) r;
+  if (__builtin_sub_overflow(x, y, &r))
+    overflowed();
+  return r;
+}
+
 unsigned test_mul_overflow_uint_uint_uint(unsigned x, unsigned y) {
   // CHECK-LABEL: define {{(dso_local )?}}i32 @test_mul_overflow_uint_uint_uint
   // CHECK-NOT: ext
@@ -97,6 +125,48 @@ int test_mul_overflow_int_int_int(int x, int y) {
   return r;
 }
 
+int test_mul_overflow_xint31_xint31_xint31(_ExtInt(31) x, _ExtInt(31) y) {
+  // CHECK-LABEL: define {{(dso_local )?}}i32 @test_mul_overflow_xint31_xint31_xint31({{.+}})
+  // CHECK-NOT: ext
+  // CHECK: [[S:%.+]] = call { i31, i1 } @llvm.smul.with.overflow.i31(i31 %{{.+}}, i31 %{{.+}})
+  // CHECK-DAG: [[C:%.+]] = extractvalue { i31, i1 } [[S]], 1
+  // CHECK-DAG: [[Q:%.+]] = extractvalue { i31, i1 } [[S]], 0
+  // CHECK: store i31 [[Q]], i31*
+  // CHECK: br i1 [[C]]
+  _ExtInt(31) r;
+  if (__builtin_mul_overflow(x, y, &r))
+    overflowed();
+  return r;
+}
+
+int test_mul_overflow_xint127_xint127_xint127(_ExtInt(127) x, _ExtInt(127) y) {
+  // CHECK-LABEL: define {{(dso_local )?}}i32 @test_mul_overflow_xint127_xint127_xint127({{.+}})
+  // CHECK-NOT: ext
+  // CHECK: [[S:%.+]] = call { i127, i1 } @llvm.smul.with.overflow.i127(i127 %{{.+}}, i127 %{{.+}})
+  // CHECK-DAG: [[C:%.+]] = extractvalue { i127, i1 } [[S]], 1
+  // CHECK-DAG: [[Q:%.+]] = extractvalue { i127, i1 } [[S]], 0
+  // CHECK: store i127 [[Q]], i127*
+  // CHECK: br i1 [[C]]
+  _ExtInt(127) r;
+  if (__builtin_mul_overflow(x, y, &r))
+    overflowed();
+  return r;
+}
+
+int test_mul_overflow_xint128_xint128_xint128(_ExtInt(128) x, _ExtInt(128) y) {
+  // CHECK-LABEL: define {{(dso_local )?}}i32 @test_mul_overflow_xint128_xint128_xint128({{.+}})
+  // CHECK-NOT: ext
+  // CHECK: [[S:%.+]] = call { i128, i1 } @llvm.smul.with.overflow.i128(i128 %{{.+}}, i128 %{{.+}})
+  // CHECK-DAG: [[C:%.+]] = extractvalue { i128, i1 } [[S]], 1
+  // CHECK-DAG: [[Q:%.+]] = extractvalue { i128, i1 } [[S]], 0
+  // CHECK: store i128 [[Q]], i128*
+  // CHECK: br i1 [[C]]
+  _ExtInt(128) r;
+  if (__builtin_mul_overflow(x, y, &r))
+    overflowed();
+  return r;
+}
+
 int test_add_overflow_uint_int_int(unsigned x, int y) {
   // CHECK-LABEL: define {{(dso_local )?}}i32 @test_add_overflow_uint_int_int
   // CHECK: [[XE:%.+]] = zext i32 %{{.+}} to i33

diff  --git a/clang/test/Sema/builtins-overflow.c b/clang/test/Sema/builtins-overflow.c
index e2c07f08ec9f..0e6152f2e935 100644
--- a/clang/test/Sema/builtins-overflow.c
+++ b/clang/test/Sema/builtins-overflow.c
@@ -19,4 +19,23 @@ void test(void) {
   __builtin_add_overflow(1, 1, 3);  // expected-error {{result argument to overflow builtin must be a pointer to a non-const integer ('int' invalid)}}
   __builtin_add_overflow(1, 1, &f);  // expected-error {{result argument to overflow builtin must be a pointer to a non-const integer ('float *' invalid)}}
   __builtin_add_overflow(1, 1, &q);  // expected-error {{result argument to overflow builtin must be a pointer to a non-const integer ('const unsigned int *' invalid)}}
+
+  {
+    _ExtInt(128) x = 1;
+    _ExtInt(128) y = 1;
+    _ExtInt(128) result;
+    _Bool status = __builtin_mul_overflow(x, y, &result); // expect ok
+  }
+  {
+    unsigned _ExtInt(129) x = 1;
+    unsigned _ExtInt(129) y = 1;
+    unsigned _ExtInt(129) result;
+    _Bool status = __builtin_mul_overflow(x, y, &result); // expect ok
+  }
+  {
+    _ExtInt(129) x = 1;
+    _ExtInt(129) y = 1;
+    _ExtInt(129) result;
+    _Bool status = __builtin_mul_overflow(x, y, &result); // expected-error {{__builtin_mul_overflow does not support signed _ExtInt operands of more than 128 bits}}
+  }
 }

diff  --git a/clang/test/Sema/builtins-overflow.m b/clang/test/Sema/builtins-overflow.m
new file mode 100644
index 000000000000..58c294666f44
--- /dev/null
+++ b/clang/test/Sema/builtins-overflow.m
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+void test() {
+  int z[1];
+  __builtin_add_overflow(1, 1, z);
+}


        


More information about the cfe-commits mailing list