From 8799ebbc1f03a348f732fc14242ad4c395076bcc Mon Sep 17 00:00:00 2001 From: Jeff Mott Date: Mon, 15 Jun 2020 06:08:58 -0700 Subject: [PATCH] [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 --- .../clang/Basic/DiagnosticSemaKinds.td | 3 + clang/lib/CodeGen/CGBuiltin.cpp | 4 +- clang/lib/Sema/SemaChecking.cpp | 50 ++++++++----- clang/test/CodeGen/builtins-overflow.c | 70 +++++++++++++++++++ clang/test/Sema/builtins-overflow.c | 19 +++++ clang/test/Sema/builtins-overflow.m | 7 ++ 6 files changed, 133 insertions(+), 20 deletions(-) create mode 100644 clang/test/Sema/builtins-overflow.m 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(); - 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); +}