Re: [PATCH]: middle-end, fix bug in fold_builtin_cexp using COMPLEX_FLOAT_TYPE_P

[Richard Guenther <richard dot guenther at gmail dot com>]

On Thu, Oct 22, 2009 at 7:29 AM, Kaveh R. GHAZI <ghazi@caip.rutgers.edu> wrote:
> While reading the source code, I noticed a latent bug in builtins.c
> fold_builtin_cexp. ?I say "latent" because these are sanity checks and I
> don't know of a way to trigger it just now. ?The argument check does
> something like this pseudo-code:
>
> ? ? ? ?if (! (type == COMPLEX_TYPE) && subtype == REAL_TYPE)
> ? ? ? ? ?return NULL_TREE;
>
> whereas it really should do:
>
> ? ? ? ?if (! (type == COMPLEX_TYPE && subtype == REAL_TYPE))
> ? ? ? ? ?return NULL_TREE;
>
> I.e. the '!' should cover both sides of the &&.
>
> I decided to convert builtins.c to use the more compact and readable
> COMPLEX_FLOAT_TYPE_P() macro which will hopefully avoid these kinds of
> errors. ?While doing it, I noticed that builtin cimag() was missing the
> subtype check, so by doing this change that's fixed as well.
>
> Note some of the places check the TREE_CODE manually, while others use
> validate_arg(). They're essentially the same thing when considering
> COMPLEX_TYPE.
>
> Since these changes touch code using MPC, I tested it with MPC svn
> revision 706, mpc-0.7 and without MPC. ?No regressions.
>
> Okay for mainline?

I think you can now ICE the compiler with

  double cimag();
  double foo()
  {
     return cimag();
  }

because only validate_arg checks for the argument being NULL.

Thus I think you should switch all places to use validate_arg
instead.

Ok with that change.

Thanks,
Richard.

> ? ? ? ? ? ? ? ?Thanks,
> ? ? ? ? ? ? ? ?--Kaveh
>
>
> 2009-10-21 ?Kaveh R. Ghazi ?<ghazi@caip.rutgers.edu>
>
> ? ? ? ?* builtins.c (fold_builtin_cabs): Use COMPLEX_FLOAT_TYPE_P to
> ? ? ? ?check for a complex float type.
> ? ? ? ?(fold_builtin_ccos): Likewise.
> ? ? ? ?(fold_builtin_cexp): Likewise.
> ? ? ? ?(fold_builtin_carg): Likewise.
> ? ? ? ?(fold_builtin_1): Likewise.
> ? ? ? ?(fold_builtin_2): Likewise.
>
> diff -rup orig/egcc-SVN20091021/gcc/builtins.c egcc-SVN20091021/gcc/builtins.c
> --- orig/egcc-SVN20091021/gcc/builtins.c ? ? ? ?2009-10-14 03:30:12.000000000 +0200
> +++ egcc-SVN20091021/gcc/builtins.c ? ? 2009-10-21 22:41:22.000000000 +0200
> @@ -7194,8 +7194,7 @@ fold_builtin_cabs (location_t loc, tree
> ?{
> ? tree res;
>
> - ?if (TREE_CODE (TREE_TYPE (arg)) != COMPLEX_TYPE
> - ? ? ?|| TREE_CODE (TREE_TYPE (TREE_TYPE (arg))) != REAL_TYPE)
> + ?if (! COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg)))
> ? ? return NULL_TREE;
>
> ? /* Calculate the result when the argument is a constant. ?*/
> @@ -7484,8 +7483,7 @@ fold_builtin_ccos (location_t loc,
> ? ? ? ? ? ? ? ? ? tree arg, tree type ATTRIBUTE_UNUSED, tree fndecl,
> ? ? ? ? ? ? ? ? ? bool hyper ATTRIBUTE_UNUSED)
> ?{
> - ?if (validate_arg (arg, COMPLEX_TYPE)
> - ? ? ?&& TREE_CODE (TREE_TYPE (TREE_TYPE (arg))) == REAL_TYPE)
> + ?if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg)))
> ? ? {
> ? ? ? tree tmp;
>
> @@ -7582,8 +7580,7 @@ fold_builtin_cexp (location_t loc, tree
> ? tree res;
> ?#endif
>
> - ?if (!validate_arg (arg0, COMPLEX_TYPE)
> - ? ? ?&& TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> + ?if (! COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
> ? ? return NULL_TREE;
>
> ?#ifdef HAVE_mpc
> @@ -9363,8 +9360,7 @@ fold_builtin_fmin_fmax (location_t loc,
> ?static tree
> ?fold_builtin_carg (location_t loc, tree arg, tree type)
> ?{
> - ?if (validate_arg (arg, COMPLEX_TYPE)
> - ? ? ?&& TREE_CODE (TREE_TYPE (TREE_TYPE (arg))) == REAL_TYPE)
> + ?if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg)))
> ? ? {
> ? ? ? tree atan2_fn = mathfn_built_in (type, BUILT_IN_ATAN2);
>
> @@ -10005,19 +10001,17 @@ fold_builtin_1 (location_t loc, tree fnd
> ? ? ? return fold_builtin_abs (loc, arg0, type);
>
> ? ? CASE_FLT_FN (BUILT_IN_CONJ):
> - ? ? ?if (validate_arg (arg0, COMPLEX_TYPE)
> - ? ? ? && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> + ? ? ?if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
> ? ? ? ?return fold_build1_loc (loc, CONJ_EXPR, type, arg0);
> ? ? break;
>
> ? ? CASE_FLT_FN (BUILT_IN_CREAL):
> - ? ? ?if (validate_arg (arg0, COMPLEX_TYPE)
> - ? ? ? && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> + ? ? ?if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
> ? ? ? ?return non_lvalue_loc (loc, fold_build1_loc (loc, REALPART_EXPR, type, arg0));;
> ? ? break;
>
> ? ? CASE_FLT_FN (BUILT_IN_CIMAG):
> - ? ? ?if (validate_arg (arg0, COMPLEX_TYPE))
> + ? ? ?if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
> ? ? ? ?return non_lvalue_loc (loc, fold_build1_loc (loc, IMAGPART_EXPR, type, arg0));
> ? ? break;
>
> @@ -10029,75 +10023,63 @@ fold_builtin_1 (location_t loc, tree fnd
>
> ?#ifdef HAVE_mpc
> ? ? CASE_FLT_FN (BUILT_IN_CSIN):
> - ? ? ?if (validate_arg (arg0, COMPLEX_TYPE)
> - ? ? ? ? && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> + ? ? ?if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
> ? ? ? ?return do_mpc_arg1 (arg0, type, mpc_sin);
> ? ? break;
>
> ? ? CASE_FLT_FN (BUILT_IN_CSINH):
> - ? ? ?if (validate_arg (arg0, COMPLEX_TYPE)
> - ? ? ? ? && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> + ? ? ?if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
> ? ? ? ?return do_mpc_arg1 (arg0, type, mpc_sinh);
> ? ? break;
>
> ? ? CASE_FLT_FN (BUILT_IN_CTAN):
> - ? ? ?if (validate_arg (arg0, COMPLEX_TYPE)
> - ? ? ? ? && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> + ? ? ?if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
> ? ? ? ?return do_mpc_arg1 (arg0, type, mpc_tan);
> ? ? break;
>
> ? ? CASE_FLT_FN (BUILT_IN_CTANH):
> - ? ? ?if (validate_arg (arg0, COMPLEX_TYPE)
> - ? ? ? ? && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> + ? ? ?if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
> ? ? ? ?return do_mpc_arg1 (arg0, type, mpc_tanh);
> ? ? break;
>
> ? ? CASE_FLT_FN (BUILT_IN_CLOG):
> - ? ? ?if (validate_arg (arg0, COMPLEX_TYPE)
> - ? ? ? ? && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> + ? ? ?if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
> ? ? ? ?return do_mpc_arg1 (arg0, type, mpc_log);
> ? ? break;
>
> ? ? CASE_FLT_FN (BUILT_IN_CSQRT):
> - ? ? ?if (validate_arg (arg0, COMPLEX_TYPE)
> - ? ? ? ? && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> + ? ? ?if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
> ? ? ? ?return do_mpc_arg1 (arg0, type, mpc_sqrt);
> ? ? break;
>
> ?#ifdef HAVE_mpc_arc
> ? ? CASE_FLT_FN (BUILT_IN_CASIN):
> - ? ? ?if (validate_arg (arg0, COMPLEX_TYPE)
> - ? ? ? ? && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> + ? ? ?if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
> ? ? ? ?return do_mpc_arg1 (arg0, type, mpc_asin);
> ? ? break;
>
> ? ? CASE_FLT_FN (BUILT_IN_CACOS):
> - ? ? ?if (validate_arg (arg0, COMPLEX_TYPE)
> - ? ? ? ? && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> + ? ? ?if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
> ? ? ? ?return do_mpc_arg1 (arg0, type, mpc_acos);
> ? ? break;
>
> ? ? CASE_FLT_FN (BUILT_IN_CATAN):
> - ? ? ?if (validate_arg (arg0, COMPLEX_TYPE)
> - ? ? ? ? && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> + ? ? ?if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
> ? ? ? ?return do_mpc_arg1 (arg0, type, mpc_atan);
> ? ? break;
>
> ? ? CASE_FLT_FN (BUILT_IN_CASINH):
> - ? ? ?if (validate_arg (arg0, COMPLEX_TYPE)
> - ? ? ? ? && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> + ? ? ?if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
> ? ? ? ?return do_mpc_arg1 (arg0, type, mpc_asinh);
> ? ? break;
>
> ? ? CASE_FLT_FN (BUILT_IN_CACOSH):
> - ? ? ?if (validate_arg (arg0, COMPLEX_TYPE)
> - ? ? ? ? && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> + ? ? ?if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
> ? ? ? ?return do_mpc_arg1 (arg0, type, mpc_acosh);
> ? ? break;
>
> ? ? CASE_FLT_FN (BUILT_IN_CATANH):
> - ? ? ?if (validate_arg (arg0, COMPLEX_TYPE)
> - ? ? ? ? && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> + ? ? ?if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
> ? ? ? ?return do_mpc_arg1 (arg0, type, mpc_atanh);
> ? ? break;
> ?#endif /* HAVE_mpc_arc */
> @@ -10421,10 +10403,8 @@ fold_builtin_2 (location_t loc, tree fnd
>
> ?#ifdef HAVE_mpc_pow
> ? ? CASE_FLT_FN (BUILT_IN_CPOW):
> - ? ? ?if (validate_arg (arg0, COMPLEX_TYPE)
> - ? ? ? ? && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE
> - ? ? ? ? && validate_arg (arg1, COMPLEX_TYPE)
> - ? ? ? ? && TREE_CODE (TREE_TYPE (TREE_TYPE (arg1))) == REAL_TYPE)
> + ? ? ?if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0))
> + ? ? ? ? && COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg1)))
> ? ? ? ?return do_mpc_arg2 (arg0, arg1, type, /*do_nonfinite=*/ 0, mpc_pow);
> ? ? break;
> ?#endif
>