adjust sprintf range for AIX QNaN output (PR 86571)

Message ID 9fafd917-d668-5234-adcb-d99c7546c3cb@gmail.com
State New
Headers show
Series
  • adjust sprintf range for AIX QNaN output (PR 86571)
Related show

Commit Message

Martin Sebor Aug. 2, 2018, 7:46 p.m.
The recently added test gcc.dg/torture/builtin-sprintf.c
to verify that the sprintf result computed by GCC matches
libc's for Infinity and NaN has been failing on AIX which
formats NaN as either QNaN or SNaN, contrary to C/POSIX
requirements.  The attached tweak adjusts the result
computed by GCC to include the AIX format.  If there are
no objections I'll commit the tweak later this week and
backport it to GCC 8 the next.

Martin

Comments

Martin Sebor Aug. 4, 2018, 10:19 p.m. | #1
On 08/02/2018 01:46 PM, Martin Sebor wrote:
> The recently added test gcc.dg/torture/builtin-sprintf.c
> to verify that the sprintf result computed by GCC matches
> libc's for Infinity and NaN has been failing on AIX which
> formats NaN as either QNaN or SNaN, contrary to C/POSIX
> requirements.  The attached tweak adjusts the result
> computed by GCC to include the AIX format.  If there are
> no objections I'll commit the tweak later this week and
> backport it to GCC 8 the next.

I have committed this change as r263312.

In the longer term, I think it might be best to introduce an OS
target hook to describe sprintf implementation-defined details
like the format of infinite values (i.e., inf or infinity, nan
or qnan/snan), and perhaps also %p format and anything else that
might be relevant.

I'll wait a bit before backporting it GCC 8 and 7 in case there
are any comments/concerns.

Martin
David Edelsohn Aug. 4, 2018, 10:25 p.m. | #2
On Sat, Aug 4, 2018, 18:19 Martin Sebor <msebor@gmail.com> wrote:

> On 08/02/2018 01:46 PM, Martin Sebor wrote:
> > The recently added test gcc.dg/torture/builtin-sprintf.c
> > to verify that the sprintf result computed by GCC matches
> > libc's for Infinity and NaN has been failing on AIX which
> > formats NaN as either QNaN or SNaN, contrary to C/POSIX
> > requirements.  The attached tweak adjusts the result
> > computed by GCC to include the AIX format.  If there are
> > no objections I'll commit the tweak later this week and
> > backport it to GCC 8 the next.
>
> I have committed this change as r263312.
>
> In the longer term, I think it might be best to introduce an OS
> target hook to describe sprintf implementation-defined details
> like the format of infinite values (i.e., inf or infinity, nan
> or qnan/snan), and perhaps also %p format and anything else that
> might be relevant.
>

Another option would be to calculate the length of NAN string at gcc build
time. Set the optimization constant  to the value returned by sprintf (os
libc function, not compiler inlined value) while building gcc.

David

>
Martin Sebor Aug. 4, 2018, 10:36 p.m. | #3
On 08/04/2018 04:25 PM, David Edelsohn wrote:
> On Sat, Aug 4, 2018, 18:19 Martin Sebor <msebor@gmail.com
> <mailto:msebor@gmail.com>> wrote:
>
>     On 08/02/2018 01:46 PM, Martin Sebor wrote:
>     > The recently added test gcc.dg/torture/builtin-sprintf.c
>     > to verify that the sprintf result computed by GCC matches
>     > libc's for Infinity and NaN has been failing on AIX which
>     > formats NaN as either QNaN or SNaN, contrary to C/POSIX
>     > requirements.  The attached tweak adjusts the result
>     > computed by GCC to include the AIX format.  If there are
>     > no objections I'll commit the tweak later this week and
>     > backport it to GCC 8 the next.
>
>     I have committed this change as r263312.
>
>     In the longer term, I think it might be best to introduce an OS
>     target hook to describe sprintf implementation-defined details
>     like the format of infinite values (i.e., inf or infinity, nan
>     or qnan/snan), and perhaps also %p format and anything else that
>     might be relevant.
>
>
> Another option would be to calculate the length of NAN string at gcc
> build time. Set the optimization constant  to the value returned by
> sprintf (os libc function, not compiler inlined value) while building gcc.

That sounds even better, thanks.  I've opened bug 86857 to look
into it.

Martin

Patch

PR tree-optimization/86571 - AIX NaNQ and NaNS output format conflicts with __builtin_sprintf

gcc/ChangeLog:

	PR tree-optimization/86571
	* gimple-ssa-sprintf.c (format_floating): Extend upper bound of
	NaN output to 4.

Index: gcc/gimple-ssa-sprintf.c
===================================================================
--- gcc/gimple-ssa-sprintf.c	(revision 263268)
+++ gcc/gimple-ssa-sprintf.c	(working copy)
@@ -2014,8 +2014,15 @@  format_floating (const directive &dir, tree arg, v
 
       res.range.likely = res.range.min;
       res.range.max = res.range.min;
-      /* The inlikely maximum is "[-/+]infinity" or "[-/+]nan".  */
-      res.range.unlikely = sign + (real_isinf (rvp) ? 8 : 3);
+      /* The unlikely maximum is "[-/+]infinity" or "[-/+][qs]nan".
+	 For NaN, the C/POSIX standards specify two formats:
+	   "[-/+]nan"
+	 and
+	   "[-/+]nan(n-char-sequence)"
+	 No known printf implementation outputs the latter format but AIX
+	 outputs QNaN and SNaN for quiet and signalling NaN, respectively,
+	 so the unlikely maximum reflects that.  */
+      res.range.unlikely = sign + (real_isinf (rvp) ? 8 : 4);
 
       /* The range for infinity and NaN is known unless either width
 	 or precision is unknown.  Width has the same effect regardless