[PATCH] D22334: Fix for Bug 28172 : clang crashes on invalid code (with too few arguments to __builtin_signbit) without any proper diagnostics.

[Mayur Pandey via cfe-commits]

ping.

On Tue, Aug 30, 2016 at 7:32 PM, Mayur Pandey <mayurthebond at gmail.com>
wrote:

> mayurpandey updated this revision to Diff 69674.
> mayurpandey added a comment.
>
> Hi,
>
> Updated the patch to add a template based testcase. As suggested by you, I
> tried updating the error diagnostic to err_typecheck_cond_expect_float
> and to issue diagnostic in similar fashion to err_typecheck_cond_expect_int_float,
> something like this :
> def err_typecheck_cond_expect_float : Error<
>
>   "used type %0 where floating point type is required">;
>
> But on updating this clang was crashing with the following message:
> clang-3.9: llvm/tools/clang/include/clang/Basic/Diagnostic.h:1167:
> clang::DiagnosticsEngine::ArgumentKind clang::Diagnostic::getArgKind(unsigned
> int) const: Assertion `Idx < getNumArgs() && "Argument index out of
> range!"' failed.
>
> So reverted it to as it was earlier and similar to other builtin error
> err_builtin_annotation_first_arg.
>
> Please review the changes and let me know if any other change is needed.
>
> Thanks,
> Mayur
>
>
> https://reviews.llvm.org/D22334
>
> Files:
>   include/clang/Basic/DiagnosticSemaKinds.td
>   lib/Sema/SemaChecking.cpp
>   test/Sema/builtins.c
>   test/SemaCXX/builtins.cpp
>
> Index: test/SemaCXX/builtins.cpp
> ===================================================================
> --- test/SemaCXX/builtins.cpp
> +++ test/SemaCXX/builtins.cpp
> @@ -44,3 +44,10 @@
>    __noop(1); // expected-error {{use of undeclared}}
>    __debugbreak(); // expected-error {{use of undeclared}}
>  }
> +
> +template <typename T>
> +int test_signbit(T t) { return __builtin_signbit(t); } // expected-error
> {{Argument type mismatch}}
> +
> +int test_signbit_call () {
> +return test_signbit("1"); // expected-note {{instantiation of function
> template specialization}}
> +}
> Index: test/Sema/builtins.c
> ===================================================================
> --- test/Sema/builtins.c
> +++ test/Sema/builtins.c
> @@ -248,3 +248,11 @@
>
>      return buf;
>  }
> +
> +int test21(double a) {
> +  return __builtin_signbit();  // expected-error {{too few arguments}}
> +}
> +
> +int test22(void) {
> +  return __builtin_signbit("1");  // expected-error {{Argument type
> mismatch}}
> +}
> Index: lib/Sema/SemaChecking.cpp
> ===================================================================
> --- lib/Sema/SemaChecking.cpp
> +++ lib/Sema/SemaChecking.cpp
> @@ -99,6 +99,22 @@
>    return false;
>  }
>
> +static bool SemaBuiltinSignbit(Sema &S, CallExpr *TheCall) {
> +  if (checkArgCount(S, TheCall, 1))
> +    return true;
> +
> +  // Argument should be an float, double or long double.
> +  Expr *ValArg = TheCall->getArg(0);
> +  QualType Ty = ValArg->getType();
> +  if (!Ty->isRealFloatingType()) {
> +    S.Diag(ValArg->getLocStart(), diag::err_builtin_signbit_
> wrong_argument)
> +      << ValArg->getSourceRange();
> +    return true;
> +  }
> +
> +  return false;
> +}
> +
>  /// Check that the argument to __builtin_addressof is a glvalue, and set
> the
>  /// result type to the corresponding pointer type.
>  static bool SemaBuiltinAddressof(Sema &S, CallExpr *TheCall) {
> @@ -763,6 +779,10 @@
>      }
>      break;
>    }
> +  case Builtin::BI__builtin_signbit:
> +    if (SemaBuiltinSignbit(*this, TheCall))
> +      return ExprError();
> +    break;
>    case Builtin::BI__builtin_isgreater:
>    case Builtin::BI__builtin_isgreaterequal:
>    case Builtin::BI__builtin_isless:
> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSemaKinds.td
> +++ include/clang/Basic/DiagnosticSemaKinds.td
> @@ -7398,6 +7398,9 @@
>  def err_builtin_annotation_second_arg : Error<
>    "second argument to __builtin_annotation must be a non-wide string
> constant">;
>
> +def err_builtin_signbit_wrong_argument : Error<
> +  "Argument type mismatch, must be float, double or long double">;
> +
>  // CFString checking
>  def err_cfstring_literal_not_string_constant : Error<
>    "CFString literal is not a string constant">;
>
>
>


-- 
Thanx & Regards
*Mayur Pandey *
+91-9742959541
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160908/b2312d76/attachment.html>