builtins: Store unspecified value to *exp for inf/nan [PR114877]

The fold_builtin_frexp folding for NaN/Inf just returned the first argument
with evaluating second arguments side-effects, rather than storing something
to what the second argument points to.

The PR argues that the C standard requires the function to store something
there but what exactly is stored is unspecified, so not storing there
anything can result in UB if the value isn't initialized and is read later.

glibc and newlib store there 0, musl apparently doesn't store anything.

The following patch stores there zero (or would you prefer storing there
some other value, 42, INT_MAX, INT_MIN, etc.?; zero is cheapest to form
in assembly though) and adjusts the test so that it
doesn't rely on not storing there anything but instead checks for
-Wmaybe-uninitialized warning to find out that something has been stored
there.
Unfortunately I had to disable the NaN tests for -O0, while we can fold
__builtin_isnan (__builtin_nan ("")) at compile time, we can't fold
__builtin_isnan ((i = 0, __builtin_nan (""))) at compile time.
fold_builtin_classify uses just tree_expr_nan_p and if that isn't true
(because expr is a COMPOUND_EXPR with tree_expr_nan_p on the second arg),
it does
      arg = builtin_save_expr (arg);
      return fold_build2_loc (loc, UNORDERED_EXPR, type, arg, arg);
and that isn't folded at -O0 further, as we wrap it into SAVE_EXPR and
nothing propagates the NAN to the comparison.
I think perhaps tree_expr_nan_p etc. could have case COMPOUND_EXPR:
added and recurse on the second argument, but that feels like stage1
material to me if we want to do that at all.

2025-01-23  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/114877
	* builtins.cc (fold_builtin_frexp): Handle rvc_nan and rvc_inf cases
	like rvc_zero, return passed in arg and set *exp = 0.

	* gcc.dg/torture/builtin-frexp-1.c: Add -Wmaybe-uninitialized as
	dg-additional-options.
	(bar): New function.
	(TESTIT_FREXP2): Rework the macro so that it doesn't test whether
	nothing has been stored to what the second argument points to, but
	instead that something has been stored there, whatever it is.
	(main): Temporarily don't enable the nan tests for -O0.

diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index 8bb91263f5f7b887f2db98b7f7a107bbf27b8a48..468bd65bc42ab75cdd0b5c54f2d6ebfce1e848dc 100644 (file)
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -9574,14 +9574,16 @@ fold_builtin_frexp (location_t loc, tree arg0, tree arg1, tree rettype)
       switch (value->cl)
       {
       case rvc_zero:
+      case rvc_nan:
+      case rvc_inf:
        /* For +-0, return (*exp = 0, +-0).  */
+       /* For +-NaN or +-Inf, *exp is unspecified, but something should
+          be stored there so that it isn't read from uninitialized object.
+          As glibc and newlib store *exp = 0 for +-Inf/NaN, storing
+          0 here as well is easiest.  */
        exp = integer_zero_node;
        frac = arg0;
        break;
-      case rvc_nan:
-      case rvc_inf:
-       /* For +-NaN or +-Inf, *exp is unspecified, return arg0.  */
-       return omit_one_operand_loc (loc, rettype, arg0, arg1);
       case rvc_normal:
        {
          /* Since the frexp function always expects base 2, and in

diff --git a/gcc/testsuite/gcc.dg/torture/builtin-frexp-1.c b/gcc/testsuite/gcc.dg/torture/builtin-frexp-1.c
index 2d1c1847b267f627c1ac02523293bde901df5c7e..328b803e9a1e7e85fc262cff85d2d4aead8d5d88 100644 (file)
--- a/gcc/testsuite/gcc.dg/torture/builtin-frexp-1.c
+++ b/gcc/testsuite/gcc.dg/torture/builtin-frexp-1.c
@@ -11,6 +11,7 @@
    floating point formats need -funsafe-math-optimizations.  */
 /* { dg-require-effective-target inf } */
 /* { dg-options "-funsafe-math-optimizations" { target powerpc*-*-* } } */
+/* { dg-additional-options "-Wmaybe-uninitialized" } */
 
 extern void link_error(int);
 
@@ -52,22 +53,36 @@ extern void link_error(int);
     link_error(__LINE__); \
   } while (0)
 
+int __attribute__ ((__noipa__))
+bar (int x)
+{
+  (void) x;
+  return 42;
+} 
+
 /* Test that FUNCRES(frexp(NEG FUNCARG(ARGARG),&i)) is false.  Check
-   the sign as well.  Ensure side-effects are evaluated in i.  */
+   the sign as well.  Ensure side-effects are evaluated in the second
+   frexp argument.  */
 #define TESTIT_FREXP2(NEG,FUNCARG,ARGARG,FUNCRES) do { \
-  int i=5; \
+  int i, j = 5; \
   if (!__builtin_##FUNCRES##f(__builtin_frexpf(NEG __builtin_##FUNCARG##f(ARGARG),&i)) \
-      || CKSGN_F(__builtin_frexpf(NEG __builtin_##FUNCARG##f(ARGARG),(i++,&i)), NEG __builtin_##FUNCARG##f(ARGARG)) \
-      || CKEXP(i,6)) \
+      || CKSGN_F(__builtin_frexpf(NEG __builtin_##FUNCARG##f(ARGARG),(j++,&i)), NEG __builtin_##FUNCARG##f(ARGARG)) \
+      || CKEXP(j,6)) \
     link_error(__LINE__); \
+  if (CKEXP(bar(i),42)) \
+    __builtin_abort(); \
   if (!__builtin_##FUNCRES(__builtin_frexp(NEG __builtin_##FUNCARG(ARGARG),&i)) \
-      || CKSGN(__builtin_frexp(NEG __builtin_##FUNCARG(ARGARG),(i++,&i)), NEG __builtin_##FUNCARG(ARGARG)) \
-      || CKEXP(i,7)) \
+      || CKSGN(__builtin_frexp(NEG __builtin_##FUNCARG(ARGARG),(j++,&i)), NEG __builtin_##FUNCARG(ARGARG)) \
+      || CKEXP(j,7)) \
     link_error(__LINE__); \
+  if (CKEXP(bar(i),42)) \
+    __builtin_abort(); \
   if (!__builtin_##FUNCRES##l(__builtin_frexpl(NEG __builtin_##FUNCARG##l(ARGARG),&i)) \
-      || CKSGN_L(__builtin_frexpl(NEG __builtin_##FUNCARG##l(ARGARG),(i++,&i)), NEG __builtin_##FUNCARG##l(ARGARG)) \
-      || CKEXP(i,8)) \
+      || CKSGN_L(__builtin_frexpl(NEG __builtin_##FUNCARG##l(ARGARG),(j++,&i)), NEG __builtin_##FUNCARG##l(ARGARG)) \
+      || CKEXP(j,8)) \
     link_error(__LINE__); \
+  if (CKEXP(bar(i),42)) \
+    __builtin_abort(); \
   } while (0)
 
 void __attribute__ ((__noinline__))
@@ -111,8 +126,10 @@ foo(void)
      Exponent is left unspecified, but we test for side-effects.  */
   TESTIT_FREXP2 ( ,inf, , isinf);
   TESTIT_FREXP2 (- ,inf, , isinf);
+#ifdef __OPTIMIZE__
   TESTIT_FREXP2 ( ,nan, "", isnan);
   TESTIT_FREXP2 (- ,nan, "", isnan);
+#endif
 }
 
 int main()