[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[pinskia at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

Andrew Pinski  changed:

   What|Removed |Added

   Last reconfirmed|2021-08-22 00:00:00 |2023-6-5

--- Comment #29 from Andrew Pinski  ---
;; y = x;

(insn 19 18 20 (set (reg:DF 91)
(mem/c:DF (plus:SI (reg/f:SI 77 virtual-stack-vars)
(const_int -8 [0xfff8])) [1 xD.3254+0 S8 A64]))
"/app/example.cpp":16:5 -1
 (nil))

(insn 20 19 0 (set (mem/c:DF (plus:SI (reg/f:SI 77 virtual-stack-vars)
(const_int -16 [0xfff0])) [1 yD.3255+0 S8 A64])
(reg:DF 91)) "/app/example.cpp":16:5 -1
 (nil))


Oh yes because the struct has one element so it will have a DFmode so it will
copy via DF here ...

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[joseph at codesourcery dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #28 from joseph at codesourcery dot com  ---
On Thu, 16 Jun 2016, ch3root at openwall dot com wrote:

> > All these memory model issues would best be raised directly with WG14,
> 
> What is the best way to do it?

If your National Body is willing to add you to the ISO Global Directory 
for WG14, then you can join the reflector for email discussions (I think 
it may no longer be possible to be on the reflector without being listed 
as a member of WG14 in the ISO Global Directory).

It should be possible to submit an N-document as a non-member.  See the 
instructions I gave at 
.

> > As soon as you use feenableexcept you are completely outside the scope of
> > any standards.
> 
> Isn't feenableexcept what IEC 60559 describes in the clause 8.1: 
> "Language standards should define, and require implementations to 
> provide, means for the user to associate alternate exception handling 
> attributes with blocks (see 4.1)."? Why it's not in TS 18661?

No.  See (draft) TS 18661-5 (out for ballot as SC22 N5112).  Alternate 
exception handling is proposed to be provided through various pragmas.  
It might or might not be possible to implement those using signal handling 
and enabling traps on some architectures.

> Or you mean that a mode with traps for the invalid operation exception 
> is not supported by gcc at all? Then this bug is mostly non-issue 

What I mean is that using feenableexcept confuses the issue, when raising 
the "invalid" exception (that is, the flag) is sufficient evidence of a 
bug if the operation doing so is one that is not permitted to do so.

> > 57484 is a closed C++ bug.  I think you mean 56831 as the substantive QoI
> > bug for such cases.
> 
> No. 56831 is about passing sNaNs to functions as arguments. This 
> conforms to TS 18661 and this could be fixed in gcc. The same for 
> assignment.
> 
> 57484 is about returning a value from a function. This is technically 
> not conforming and this couldn't be fixed in gcc without breaking ABI. 
> If a separate bug for C is required I can file one.

Return values are clearly a bug in the wording of the standard, given how 
Annex F envisages that conversion-to-same-type may occur on return.

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[ch3root at openwall dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #27 from Alexander Cherepanov  ---
On 06/12/2016 01:09 AM, joseph at codesourcery dot com wrote:
>>> C11 does not
>>> consider sNaNs, and TS 18661 is explicitly stating otherwise for them.
>>
>> You are talking about C11 + TS 18661 which is a different version of C.
>
> The semantics of C11 + TS 18661 are a refinement of those of C11 (with
> sNaNs in C11 being trap representations by virtue of not acting like any
> kind of value considered by C11, so a refinement of C11 semantics can
> apply any semantics it likes to them).

Ok, I see. It seems that it would be fine with C11 to consider sNaN and 
qNaN alternative representations of the same value when traps are off 
but it would be incompatible with TS 18661 so it's more convenient to 
consider sNaN a trap representation. This makes the question of traps 
irrelevant and makes it compatible with TS 18661.

>> I'm not sure. It would be nice to have such a clear distinction between
>> values and representations but C is tricky. What is the value of a union
>> after memset? Suppose a value stored into an active member of a union
>> admits several representations, is taking inactive member of this union
>> (aka type-punning) is required to give the same result before and after
>> applying lvalue-to-rvalue conversion to the union?
>>
>> Heck, even the example that started this PR is not that easy. If a
>> member of a structure has a trap representation what is the value of
>> this structure? Is copying of this structure required to preserve the
>> exact representation of this member? Can this trap representation be
>> changed to a non-trap one?
>
> All these memory model issues would best be raised directly with WG14,

What is the best way to do it?

> possibly working together with the Cerberus people, not with individual
> implementations.

I'm afraid I don't quite understand their approach/aim and the choice of 
questions. OTOH one of the most useful to me results from the whole work 
was your reply to their questionnaire. Thank you for this!

>> So the idea is that gcc on x86-32 with feenableexcept(FE_INVALID) could
>> be made conforming with C11 + TS 18661? Is there anything there that
>
> As soon as you use feenableexcept you are completely outside the scope of
> any standards.

Isn't feenableexcept what IEC 60559 describes in the clause 8.1: 
"Language standards should define, and require implementations to 
provide, means for the user to associate alternate exception handling 
attributes with blocks (see 4.1)."? Why it's not in TS 18661?

Or you mean that a mode with traps for the invalid operation exception 
is not supported by gcc at all? Then this bug is mostly non-issue 
because IMHO the main problem is crashing and not the changes of 
representations of NaNs.

>> allows to trap on a return from a function (when the result of the
>> function is not used)? C11, 6.8.6.4p3, talks about a conversion "as if
>> by assignment" only for a case of differing types and I don't see TS
>> 18661 patching this clause of C11.
>
> The intent is clear, because C11 + Annex F semantics are meant to be a
> refinement of those of base C11 (Annex F being an optional extension to
> base C11), and from F.6 you can tell that such cases must be permitted to
> do a conversion to the same type, so it must be intended to allow such a
> conversion.  But I'll raise this for clarification in TS 18661 anyway.

Yes, this is probably a simple case. I haven't seen F.6 before but it 
surprising doesn't change 6.8.6.4p3 much.

I'm less sure about mere lvalue-to-rvalue conversion. Trapping at lvalue 
conversion makes impossible to apply isnan  to sNaNs even though 
"[...] these macros raise no floating-point exceptions, even if an 
argument is a signaling NaN."

>> I mean bug 57484 is more important case than lone loads from volatiles.
>
> 57484 is a closed C++ bug.  I think you mean 56831 as the substantive QoI
> bug for such cases.

No. 56831 is about passing sNaNs to functions as arguments. This 
conforms to TS 18661 and this could be fixed in gcc. The same for 
assignment.

57484 is about returning a value from a function. This is technically 
not conforming and this couldn't be fixed in gcc without breaking ABI. 
If a separate bug for C is required I can file one.

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[joseph at codesourcery dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #26 from joseph at codesourcery dot com  ---
On Sat, 11 Jun 2016, ch3root at openwall dot com wrote:

> > C11 does not
> > consider sNaNs, and TS 18661 is explicitly stating otherwise for them.
> 
> You are talking about C11 + TS 18661 which is a different version of C. 

The semantics of C11 + TS 18661 are a refinement of those of C11 (with 
sNaNs in C11 being trap representations by virtue of not acting like any 
kind of value considered by C11, so a refinement of C11 semantics can 
apply any semantics it likes to them).

> Even TS 18661-1 acknowledges that it's incompatible with C11, e.g. 
> clause 12 reads: "[...] so that an implementation could not add 
> signaling NaN support as an extension without contradicting C11." Will 
> there be a new value for the -std option?

The option is -fsignaling-nans.  In future -std=c2x may imply 
-fno-fp-int-builtin-inexact if TS 18661-1 is integrated in C2x, but I 
expect -fsignaling-nans still to be needed if you want such values to 
behave in a particular consistent way.

> I'm not sure. It would be nice to have such a clear distinction between 
> values and representations but C is tricky. What is the value of a union 
> after memset? Suppose a value stored into an active member of a union 
> admits several representations, is taking inactive member of this union 
> (aka type-punning) is required to give the same result before and after 
> applying lvalue-to-rvalue conversion to the union?
> 
> Heck, even the example that started this PR is not that easy. If a 
> member of a structure has a trap representation what is the value of 
> this structure? Is copying of this structure required to preserve the 
> exact representation of this member? Can this trap representation be 
> changed to a non-trap one?

All these memory model issues would best be raised directly with WG14, 
possibly working together with the Cerberus people, not with individual 
implementations.

> So the idea is that gcc on x86-32 with feenableexcept(FE_INVALID) could 
> be made conforming with C11 + TS 18661? Is there anything there that 

As soon as you use feenableexcept you are completely outside the scope of 
any standards.

> allows to trap on a return from a function (when the result of the 
> function is not used)? C11, 6.8.6.4p3, talks about a conversion "as if 
> by assignment" only for a case of differing types and I don't see TS 
> 18661 patching this clause of C11.

The intent is clear, because C11 + Annex F semantics are meant to be a 
refinement of those of base C11 (Annex F being an optional extension to 
base C11), and from F.6 you can tell that such cases must be permitted to 
do a conversion to the same type, so it must be intended to allow such a 
conversion.  But I'll raise this for clarification in TS 18661 anyway.

> I mean bug 57484 is more important case than lone loads from volatiles. 

57484 is a closed C++ bug.  I think you mean 56831 as the substantive QoI 
bug for such cases.

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[ch3root at openwall dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #25 from Alexander Cherepanov  ---
On 06/10/2016 12:18 AM, joseph at codesourcery dot com wrote:
>>   > For *scalar* assignment that would be fine because of TS 18661-1 saying
>>   > "Whether C assignment (6.5.16) (and conversion as if by assignment)
>> to the
>>   > same format is an IEC 60559 convertFormat or copy operation is
>>   > implementation-defined, even if  defines the macro
>>   > FE_SNANS_ALWAYS_SIGNAL (F.2.1).".  It's rather less clear for struct
>>   > assignment.
>>
>> How is it compatible with C11, 6.3p2: "Conversion of an operand value to
>> a compatible type causes no change to the value or the representation."?
>> Ha, even changing representation is prohibited.
>
> That paragraph starts "Unless explicitly stated otherwise,".

Ugh, it's not in N1570.

> C11 does not
> consider sNaNs, and TS 18661 is explicitly stating otherwise for them.

You are talking about C11 + TS 18661 which is a different version of C. 
Even TS 18661-1 acknowledges that it's incompatible with C11, e.g. 
clause 12 reads: "[...] so that an implementation could not add 
signaling NaN support as an extension without contradicting C11." Will 
there be a new value for the -std option?

> "or the representation" is clearly nonsensical here.  When something is
> converted it's an rvalue, and rvalues aren't associated with
> representations unless and until stored in an object; they are just
> members of the set of values of a type (and on assignment, 6.2.6.1#8
> applies).

I'm not sure. It would be nice to have such a clear distinction between 
values and representations but C is tricky. What is the value of a union 
after memset? Suppose a value stored into an active member of a union 
admits several representations, is taking inactive member of this union 
(aka type-punning) is required to give the same result before and after 
applying lvalue-to-rvalue conversion to the union?

Heck, even the example that started this PR is not that easy. If a 
member of a structure has a trap representation what is the value of 
this structure? Is copying of this structure required to preserve the 
exact representation of this member? Can this trap representation be 
changed to a non-trap one?

And the whole DR 260 is written as if an indeterminate value is a value.

>>> Rather, it's OK for the loads
>>> to raise an exception because assignment / argument passing etc. can be
>>
>> I mean that "volatile double x; /* store sNaN to x */; x;" gives SIGFPE
>> currently. It doesn't look like (re)definitions of assignment etc. would
>> be applicable to this case.
>
> That seems rather like an omission in the specification relating to excess
> range and precision, which should be considered like a conversion
> (convertFormat) to the relevant wider format.

So the idea is that gcc on x86-32 with feenableexcept(FE_INVALID) could 
be made conforming with C11 + TS 18661? Is there anything there that 
allows to trap on a return from a function (when the result of the 
function is not used)? C11, 6.8.6.4p3, talks about a conversion "as if 
by assignment" only for a case of differing types and I don't see TS 
18661 patching this clause of C11.

I mean bug 57484 is more important case than lone loads from volatiles. 
But fixing the problem for lvalue-to-rvalue conversions in general 
should cover everything.

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #24 from rguenther at suse dot de  ---
On Fri, 10 Jun 2016, ubizjak at gmail dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460
> 
> --- Comment #21 from Uroš Bizjak  ---
> (In reply to Uroš Bizjak from comment #19)
> > Following test doesn't compile with patched compiler:
> 
> BTW: This is preexisting problem, on 64bit target we can trigger the issue 
> with
> transparent unions with:
> 
> --cut here--
> union U { __int128 x; long double y; }  __attribute__ ((transparent_union));
> 
> void foo (union U);
> 
> void test (void)
> {
>   union U t;
> 
>   foo (1.0L);
> 
>   foo ( (__int128) 1);
> }
> --cut here--
> 
> -O2:
> 
> union.c:1:7: warning: union cannot be made transparent
>  union U { __int128 x; long double y; }  __attribute__ ((transparent_union));

But __int128 and long double are not passed the same way (rdi/rsi vs. on 
the stack).

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[ubizjak at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #23 from Uroš Bizjak  ---
(In reply to Uroš Bizjak from comment #21)
> BTW: This is preexisting problem, on 64bit target we can trigger the issue
> with transparent unions with:

Followup in PR71486.

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #22 from Richard Biener  ---
(In reply to Alexander Cherepanov from comment #12)
> On 2016-06-09 11:22, rguenth at gcc dot gnu.org wrote:
> > Would be nice to have a testcase for the SRA case as well.
> 
> Source code:
> 
> --
> #define _GNU_SOURCE
> #include 
> 
> struct s { double d; char c; };
> 
> __attribute__((noinline,noclone))
> void copy(struct s *q, struct s *p)
> {
>struct s tmp = *p;
>*q = tmp;
> }
> 
> int main()
> {
>feenableexcept(FE_INVALID);
> 
>struct s x = {0}, y;
>((unsigned char *))[7] = 0x7f;
>((unsigned char *))[6] = 0xf0; // sNaN
>((unsigned char *))[0] = 0x01;
> 
>copy(, );
> }
> --
> 
> Results:
> 
> --
> $ gcc -std=c11 -pedantic -Wall -Wextra -fsignaling-nans -lm -m32 -O3 
> test.c && ./a.out
> Floating point exception
> --
> 
> gcc version: gcc (GCC) 7.0.0 20160608 (experimental)

So for this one the following would work together with the proposed backend
fix (but not for the array case).  It also makes me think using this target
hook is not really the thing to test here given its docs:

"
@deftypefn {Target Hook} bool TARGET_MEMBER_TYPE_FORCES_BLK (const_tree
@var{field}, machine_mode @var{mode})
Return true if a structure, union or array containing @var{field} should
be accessed using @code{BLKMODE}.

If @var{field} is the only field in the structure, @var{mode} is its
mode, otherwise @var{mode} is VOIDmode.  @var{mode} is provided in the
case where structures of one field would require the structure's mode to
retain the field's mode.

Normally, this is not needed.
"

another thing to test here would be MODE_HAS_NANS, but that's too general
I think.  It seems we lack sth to query from the target, like
MODE_ACCESS_CAN_TRAP or so (just thinking of IA64 NaT stuff).  At least
the following points out the place in SRA to put the fix in - it guards
the case artificial accesses are created for a struct copy (though not
all of them are ultimatively used - SRA works too "local" for them).  We
need to be careful to be not too conservative and disable too much of SRA.
The hook seems to be used to avoid various issues, not only trapping.

Index: gcc/tree-sra.c
===
--- gcc/tree-sra.c  (revision 237286)
+++ gcc/tree-sra.c  (working copy)
@@ -969,6 +969,9 @@ scalarizable_type_p (tree type)
  if (DECL_BIT_FIELD (fld))
return false;

+ if (targetm.member_type_forces_blk (fld, TYPE_MODE (ft)))
+   return false;
+
  if (!is_gimple_reg_type (ft)
  && !scalarizable_type_p (ft))
return false;
@@ -994,6 +997,10 @@ scalarizable_type_p (tree type)
return false;

   tree elem = TREE_TYPE (type);
+
+  if (targetm.member_type_forces_blk (elem, TYPE_MODE (elem)))
+   return false;
+
   if (!is_gimple_reg_type (elem)
 && !scalarizable_type_p (elem))
return false;

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[ubizjak at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #21 from Uroš Bizjak  ---
(In reply to Uroš Bizjak from comment #19)
> Following test doesn't compile with patched compiler:

BTW: This is preexisting problem, on 64bit target we can trigger the issue with
transparent unions with:

--cut here--
union U { __int128 x; long double y; }  __attribute__ ((transparent_union));

void foo (union U);

void test (void)
{
  union U t;

  foo (1.0L);

  foo ( (__int128) 1);
}
--cut here--

-O2:

union.c:1:7: warning: union cannot be made transparent
 union U { __int128 x; long double y; }  __attribute__ ((transparent_union));

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #20 from rguenther at suse dot de  ---
On Fri, 10 Jun 2016, ubizjak at gmail dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460
> 
> --- Comment #19 from Uroš Bizjak  ---
> (In reply to Uroš Bizjak from comment #18)
> > (In reply to rguent...@suse.de from comment #17)
> > > But this also hints at the ABI for
> > > 
> > > void foo (union U { int x; float y; });
> > > 
> > > changing with the patch, no?  Or ultimatively at the FE using a bogus
> > > check to verify ABI compatibility (just looking at TYPE_MODE).
> 
> Following test doesn't compile with patched compiler:
> 
> --cut here--
> union U { int x; float y; } __attribute__ ((transparent_union));
> 
> void foo (union U);
> 
> void test (void)
> {
>   union U t;
> 
>   foo (1.0f);
> 
>   foo (1);
> }
> --cut here--
> 
> -O2 -mfpmath=387 -m32:
> 
> union.c:1:7: warning: union cannot be made transparent
>  union U { int x; float y; } __attribute__ ((transparent_union));
> 
> but does compile OK with:
> 
> -msse -O2 -mfpmath=sse -m32

So another way to avoid the issue in this PR is to not change the
types mode but avoid using TYPE_MODE when expanding aggregate copies
and those modes would be "bad" (not sure if we can somehow use the
same target hook for this).  But it really seems the hook was designed
for this ...

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[ubizjak at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #19 from Uroš Bizjak  ---
(In reply to Uroš Bizjak from comment #18)
> (In reply to rguent...@suse.de from comment #17)
> > But this also hints at the ABI for
> > 
> > void foo (union U { int x; float y; });
> > 
> > changing with the patch, no?  Or ultimatively at the FE using a bogus
> > check to verify ABI compatibility (just looking at TYPE_MODE).

Following test doesn't compile with patched compiler:

--cut here--
union U { int x; float y; } __attribute__ ((transparent_union));

void foo (union U);

void test (void)
{
  union U t;

  foo (1.0f);

  foo (1);
}
--cut here--

-O2 -mfpmath=387 -m32:

union.c:1:7: warning: union cannot be made transparent
 union U { int x; float y; } __attribute__ ((transparent_union));

but does compile OK with:

-msse -O2 -mfpmath=sse -m32

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[ubizjak at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #18 from Uroš Bizjak  ---
(In reply to rguent...@suse.de from comment #17)
> But this also hints at the ABI for
> 
> void foo (union U { int x; float y; });
> 
> changing with the patch, no?  Or ultimatively at the FE using a bogus
> check to verify ABI compatibility (just looking at TYPE_MODE).

No, there is no ABI violation. The testcase:

--cut here--
union U { int x; float y; };

void foo (union U);

void test (void)
{
  union U t;

  t.y = 1.0f;
  foo (t);

  t.x = 1;
  foo (t);
}
--cut here--

results in (-O2 -mfpmath=387 -m32):

pushl   $1065353216
callfoo
movl$1, (%esp)
callfoo

The asm is also correct with -mregparm=1:

movl$1065353216, %eax
callfoo
movl$1, %eax
callfoo

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #17 from rguenther at suse dot de  ---
On Thu, 9 Jun 2016, ubizjak at gmail dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460
> 
> --- Comment #13 from Uroš Bizjak  ---
> (In reply to Richard Biener from comment #7)
> 
> > Oh - I hope TYPE_MODE does not affect the ABI here ;)  You might want to
> > double-check that (struct-layout tests plus call ABI)
> The patch introduces one testsuite failure with -m32:
> 
> FAIL: g++.dg/torture/pr67581.C
> 
> where:
> 
> /home/uros/gcc-svn/trunk/gcc/testsuite/g++.dg/torture/pr67581.C:2:7: error:
> type transparent 'union U' cannot be made transparent because the type of the
> first field has a different ABI from the class overall
> 
> I don't know if this is c++ frontend issue, where BLKmode should be allowed.

But this also hints at the ABI for

void foo (union U { int x; float y; });

changing with the patch, no?  Or ultimatively at the FE using a bogus
check to verify ABI compatibility (just looking at TYPE_MODE).

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[joseph at codesourcery dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #16 from joseph at codesourcery dot com  ---
I've now raised the void context issue on the WG14 reflector 
.

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[joseph at codesourcery dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #15 from joseph at codesourcery dot com  ---
On Thu, 9 Jun 2016, ch3root at openwall dot com wrote:

>  > For *scalar* assignment that would be fine because of TS 18661-1 saying
>  > "Whether C assignment (6.5.16) (and conversion as if by assignment) 
> to the
>  > same format is an IEC 60559 convertFormat or copy operation is
>  > implementation-defined, even if  defines the macro
>  > FE_SNANS_ALWAYS_SIGNAL (F.2.1).".  It's rather less clear for struct
>  > assignment.
> 
> How is it compatible with C11, 6.3p2: "Conversion of an operand value to 
> a compatible type causes no change to the value or the representation."? 
> Ha, even changing representation is prohibited.

That paragraph starts "Unless explicitly stated otherwise,".  C11 does not 
consider sNaNs, and TS 18661 is explicitly stating otherwise for them.

"or the representation" is clearly nonsensical here.  When something is 
converted it's an rvalue, and rvalues aren't associated with 
representations unless and until stored in an object; they are just 
members of the set of values of a type (and on assignment, 6.2.6.1#8 
applies).

> > Rather, it's OK for the loads
> > to raise an exception because assignment / argument passing etc. can be
> 
> I mean that "volatile double x; /* store sNaN to x */; x;" gives SIGFPE 
> currently. It doesn't look like (re)definitions of assignment etc. would 
> be applicable to this case.

That seems rather like an omission in the specification relating to excess 
range and precision, which should be considered like a conversion 
(convertFormat) to the relevant wider format.

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[ch3root at openwall dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #14 from Alexander Cherepanov  ---
On 2016-06-08 20:49, joseph at codesourcery dot com wrote:
 >> - sNaNs are turned into qNaNs on x86-32 if traps are turned off. 
Probably ok if
 >> sNaNs and qNaNs are considered different representations of the same 
value;
 >
 > For *scalar* assignment that would be fine because of TS 18661-1 saying
 > "Whether C assignment (6.5.16) (and conversion as if by assignment) 
to the
 > same format is an IEC 60559 convertFormat or copy operation is
 > implementation-defined, even if  defines the macro
 > FE_SNANS_ALWAYS_SIGNAL (F.2.1).".  It's rather less clear for struct
 > assignment.

How is it compatible with C11, 6.3p2: "Conversion of an operand value to 
a compatible type causes no change to the value or the representation."? 
Ha, even changing representation is prohibited.

 > I expect the present bug is essentially the same as bug 58416 (which
 > concerns a union, where the floating-point member isn't even the active
 > one).

Indeed. It seems there are 3 variations of this bug:
- a struct with one FP member -- triggered even without optimizations;
- scalarization of a struct;
- scalarization of a union.
There are testcases for all of them.

On 2016-06-09 19:59, joseph at codesourcery dot com wrote:
>> - mere load (e.g. from a volatile var) of a float or double sNaN traps
>> on x86-32 when traps are enabled;
>
> It raises an exception.  Exception trapping is outside the scope of ISO C.

Yes, so on level of ISO C it's just a trap.

>> - this perfectly fits the description of trap representation.
>> So gcc de facto doesn't support sNaNs in this configuration and any its
>> use is UB.
>
> No, it's not UB (given -fsignaling-nans).

I was too concise. I mean that it's UB from POV of ISO C. But an 
implementation is free define some of it. And the standard doesn't place 
any restrictions on this definition. For example, an implementation can 
say that it's fine to pass sNaN to a function as an argument but it's 
not fine to return it from a function. Or it can say that passing it as 
an argument is not fine too.

So the current behavior of gcc on x86-32 can be considered conforming 
(modulo this bug).

> Rather, it's OK for the loads
> to raise an exception because assignment / argument passing etc. can be

I mean that "volatile double x; /* store sNaN to x */; x;" gives SIGFPE 
currently. It doesn't look like (re)definitions of assignment etc. would 
be applicable to this case.

> implemented as convertFormat rather than copy, which means raising the
> exception and converting to qNaN.

As I wrote above, it's not clear to me that this is compatible with the 
current text of C11. Not that it means much...

> (As a quality-of-implementation issue I
> still think that with -fsignaling-nans, floating-point loads should be
> avoided for simple copies of float and double values where they aren't
> needed in a floating-point register for arithmetic, function return etc. -
> that would allow sNaNs to be passed as function arguments, for example.)

AIUI ABI restricts how FP values are returned from functions so this 
cannot be fixed (bug 57484). But there are 2 cases that can be fixed 
(bug 56831):
- assignments;
- passing arguments to functions.
Plus loads in a void context:-)

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[ubizjak at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #13 from Uroš Bizjak  ---
(In reply to Richard Biener from comment #7)

> Oh - I hope TYPE_MODE does not affect the ABI here ;)  You might want to
> double-check that (struct-layout tests plus call ABI)
The patch introduces one testsuite failure with -m32:

FAIL: g++.dg/torture/pr67581.C

where:

/home/uros/gcc-svn/trunk/gcc/testsuite/g++.dg/torture/pr67581.C:2:7: error:
type transparent 'union U' cannot be made transparent because the type of the
first field has a different ABI from the class overall

I don't know if this is c++ frontend issue, where BLKmode should be allowed.

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[ch3root at openwall dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #12 from Alexander Cherepanov  ---
On 2016-06-09 11:22, rguenth at gcc dot gnu.org wrote:
> Would be nice to have a testcase for the SRA case as well.

Source code:

--
#define _GNU_SOURCE
#include 

struct s { double d; char c; };

__attribute__((noinline,noclone))
void copy(struct s *q, struct s *p)
{
   struct s tmp = *p;
   *q = tmp;
}

int main()
{
   feenableexcept(FE_INVALID);

   struct s x = {0}, y;
   ((unsigned char *))[7] = 0x7f;
   ((unsigned char *))[6] = 0xf0; // sNaN
   ((unsigned char *))[0] = 0x01;

   copy(, );
}
--

Results:

--
$ gcc -std=c11 -pedantic -Wall -Wextra -fsignaling-nans -lm -m32 -O3 
test.c && ./a.out
Floating point exception
--

gcc version: gcc (GCC) 7.0.0 20160608 (experimental)

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[joseph at codesourcery dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #11 from joseph at codesourcery dot com  ---
On Thu, 9 Jun 2016, ch3root at openwall dot com wrote:

> - mere load (e.g. from a volatile var) of a float or double sNaN traps 
> on x86-32 when traps are enabled;

It raises an exception.  Exception trapping is outside the scope of ISO C.

> - this perfectly fits the description of trap representation.
> So gcc de facto doesn't support sNaNs in this configuration and any its 
> use is UB.

No, it's not UB (given -fsignaling-nans).  Rather, it's OK for the loads 
to raise an exception because assignment / argument passing etc. can be 
implemented as convertFormat rather than copy, which means raising the 
exception and converting to qNaN.  (As a quality-of-implementation issue I 
still think that with -fsignaling-nans, floating-point loads should be 
avoided for simple copies of float and double values where they aren't 
needed in a floating-point register for arithmetic, function return etc. - 
that would allow sNaNs to be passed as function arguments, for example.)

> IOW this bug is not about wrong handling of sNaNs, it's about 
> introducing sNaN-related problems where there were none. Like copying a 
> partially initialized struct where one of unknown fields happened to 
> have the double type and sNaN as representation.

Indeed, which is why a fix for this issue / bug 58416 should *not* be 
conditional on -fsignaling-nans - it affects code that doesn't use 
signaling NaNs, just bit patterns that are never actually interpreted with 
type float or double (but if so interpreted, would be signaling NaNs).

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[ch3root at openwall dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #10 from Alexander Cherepanov  ---
On 2016-06-08 20:47, pinskia at gcc dot gnu.org wrote:
> --- Comment #1 from Andrew Pinski  ---
> I think this is really a dup of bug 57484.  The problem is x87 related and
> there is not much to be done.

I don't think so.

I haven't looked into floating-point arithmetic in any details but, as a 
first approximation, I see the situation like this:
- sNaNs are not required by C11 (including by Annex F);
- mere load (e.g. from a volatile var) of a float or double sNaN traps 
on x86-32 when traps are enabled;
- this perfectly fits the description of trap representation.
So gcc de facto doesn't support sNaNs in this configuration and any its 
use is UB.

OTOH copying of structures doesn't invoke UB and is required not to 
trap. (At least if its address is taken:-)
(Not sure if this is contradicted by C11, 6.2.6.1p5: "If such a 
representation is produced by a side effect that modifies all or any 
part of the object by an lvalue expression that does not have character 
type, the behavior is undefined.50)". But the intention of DR 222 is 
quite clear.)

IOW this bug is not about wrong handling of sNaNs, it's about 
introducing sNaN-related problems where there were none. Like copying a 
partially initialized struct where one of unknown fields happened to 
have the double type and sNaN as representation.

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[ubizjak at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #9 from Uroš Bizjak  ---
(In reply to jos...@codesourcery.com from comment #8)
> It's not correct to use flag_signaling_nans for a fix.  
> flag_signaling_nans is only for cases where a bit-pattern for a signaling 
> NaN is interpreted as a floating-point value.  It's not where a struct or 
> union element has floating-point type but is never interpreted as a value 
> of that type, just the whole aggregate copied.  It's completely valid 
> without flag_signaling_nans to copy a struct that happens to have such a 
> bit pattern.

Indeed. I have removed this condition, and the patch I'm testing is:

--cut here--
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index b807a9a..a719d62 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10311,9 +10311,25 @@ static bool
 ix86_member_type_forces_blk (const_tree field, machine_mode mode)
 {
   /* Union with XFmode must be in BLKmode.  */
-  return (mode == XFmode
- && (TREE_CODE (DECL_FIELD_CONTEXT (field)) == UNION_TYPE
- || TREE_CODE (DECL_FIELD_CONTEXT (field)) == QUAL_UNION_TYPE));
+  if (mode == XFmode
+  && (TREE_CODE (DECL_FIELD_CONTEXT (field)) == UNION_TYPE
+ || TREE_CODE (DECL_FIELD_CONTEXT (field)) == QUAL_UNION_TYPE))
+return true;
+
+  /* Early exit for XFmode moves, they don't trap
+ on sNaNs and don't convert sNaNs into qNaNs.  */
+  if (mode == XFmode)
+return false;
+
+  /* DFmode and SFmode moves through X87 stack trap on sNaNs if traps
+ are turned on or convert sNaNs to qNaNs if traps are turned off.
+ This violates C11, 6.2.6.1p6, that specifically says that
+ "[t]he value of a structure or union object is never a trap
+ representation, even though the value of a member of the structure
+ or union object may be a trap representation."  */
+
+  return (IS_STACK_MODE (mode)
+ && RECORD_OR_UNION_TYPE_P (DECL_FIELD_CONTEXT (field)));
 }

 rtx
--cut here--

> Does the patch (without flag_signaling_nans) help with the bug 58416 case 
> or not?

Using the patched compiler, I get:

$ ~/gcc-build/gcc/xgcc -B ~/gcc-build/gcc/ -O2 -m32 -mfpmath=387 pr58416.C 
$ ./a.out
magic=fff1
copy= fff1

... which looks OK to me.

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[joseph at codesourcery dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #8 from joseph at codesourcery dot com  ---
It's not correct to use flag_signaling_nans for a fix.  
flag_signaling_nans is only for cases where a bit-pattern for a signaling 
NaN is interpreted as a floating-point value.  It's not where a struct or 
union element has floating-point type but is never interpreted as a value 
of that type, just the whole aggregate copied.  It's completely valid 
without flag_signaling_nans to copy a struct that happens to have such a 
bit pattern.

Does the patch (without flag_signaling_nans) help with the bug 58416 case 
or not?

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #7 from Richard Biener  ---
(In reply to Richard Biener from comment #6)
> (In reply to Uroš Bizjak from comment #5)
> > Following patch fixes the failure:
> > 
> > --cut here--
> > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> > index b807a9a..4526c7d 100644
> > --- a/gcc/config/i386/i386.c
> > +++ b/gcc/config/i386/i386.c
> > @@ -10310,10 +10310,8 @@ ix86_promote_function_mode (const_tree type,
> > machine_mode mode,
> >  static bool
> >  ix86_member_type_forces_blk (const_tree field, machine_mode mode)
> >  {
> > -  /* Union with XFmode must be in BLKmode.  */
> > -  return (mode == XFmode
> > - && (TREE_CODE (DECL_FIELD_CONTEXT (field)) == UNION_TYPE
> > - || TREE_CODE (DECL_FIELD_CONTEXT (field)) == 
> > QUAL_UNION_TYPE));
> > +  return (IS_STACK_MODE (mode) && flag_signaling_nans
> > + && RECORD_OR_UNION_TYPE_P (DECL_FIELD_CONTEXT (field)));
> >  }
> >  
> >  rtx
> > --cut here--
> > 
> > Patch was tested with various -mfpmath=... compile flags. IMO, we don't need
> > BLKmode when we copy the value using SSE registers (and the test indeed does
> > not fail with -mfpmath=sse), and we still can copy XFmode using x87 regs
> > without -fsignalling-nans.
> > 
> > Richi, Joseph - does the new condition look OK to you?
> 
> Looks good to me.

Oh - I hope TYPE_MODE does not affect the ABI here ;)  You might want to
double-check that (struct-layout tests plus call ABI)

> Btw, the mentioned SRA issue probably still exists for full scalarization
> and re-mat for a call.  SRA should probably check member_type_forces_blk
> as well before creating full scalarization accesses.  Would be nice to
> have a testcase for the SRA case as well.

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

Richard Biener  changed:

   What|Removed |Added

 CC||jamborm at gcc dot gnu.org,
   ||rguenth at gcc dot gnu.org

--- Comment #6 from Richard Biener  ---
(In reply to Uroš Bizjak from comment #5)
> Following patch fixes the failure:
> 
> --cut here--
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index b807a9a..4526c7d 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -10310,10 +10310,8 @@ ix86_promote_function_mode (const_tree type,
> machine_mode mode,
>  static bool
>  ix86_member_type_forces_blk (const_tree field, machine_mode mode)
>  {
> -  /* Union with XFmode must be in BLKmode.  */
> -  return (mode == XFmode
> - && (TREE_CODE (DECL_FIELD_CONTEXT (field)) == UNION_TYPE
> - || TREE_CODE (DECL_FIELD_CONTEXT (field)) == QUAL_UNION_TYPE));
> +  return (IS_STACK_MODE (mode) && flag_signaling_nans
> + && RECORD_OR_UNION_TYPE_P (DECL_FIELD_CONTEXT (field)));
>  }
>  
>  rtx
> --cut here--
> 
> Patch was tested with various -mfpmath=... compile flags. IMO, we don't need
> BLKmode when we copy the value using SSE registers (and the test indeed does
> not fail with -mfpmath=sse), and we still can copy XFmode using x87 regs
> without -fsignalling-nans.
> 
> Richi, Joseph - does the new condition look OK to you?

Looks good to me.

Btw, the mentioned SRA issue probably still exists for full scalarization
and re-mat for a call.  SRA should probably check member_type_forces_blk
as well before creating full scalarization accesses.  Would be nice to
have a testcase for the SRA case as well.

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[ubizjak at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

Uroš Bizjak  changed:

   What|Removed |Added

 CC||jsm28 at gcc dot gnu.org

--- Comment #5 from Uroš Bizjak  ---
Following patch fixes the failure:

--cut here--
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index b807a9a..4526c7d 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10310,10 +10310,8 @@ ix86_promote_function_mode (const_tree type,
machine_mode mode,
 static bool
 ix86_member_type_forces_blk (const_tree field, machine_mode mode)
 {
-  /* Union with XFmode must be in BLKmode.  */
-  return (mode == XFmode
- && (TREE_CODE (DECL_FIELD_CONTEXT (field)) == UNION_TYPE
- || TREE_CODE (DECL_FIELD_CONTEXT (field)) == QUAL_UNION_TYPE));
+  return (IS_STACK_MODE (mode) && flag_signaling_nans
+ && RECORD_OR_UNION_TYPE_P (DECL_FIELD_CONTEXT (field)));
 }

 rtx
--cut here--

Patch was tested with various -mfpmath=... compile flags. IMO, we don't need
BLKmode when we copy the value using SSE registers (and the test indeed does
not fail with -mfpmath=sse), and we still can copy XFmode using x87 regs
without -fsignalling-nans.

Richi, Joseph - does the new condition look OK to you?

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[ubizjak at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #4 from Uroš Bizjak  ---
(In reply to Richard Biener from comment #3)
> It looks like RTL expansion uses DFmode to copy the aggregate which is likely
> because the backend (or stor-layout.c) assigns DFmode to struct s.  A similar
> issue should exist on Itanium.
> 
> This means the x86 target, to fix this particular case, should implement
> targetm.member_type_forces_blk for x87 modes (and -fsignalling-nans?).

We already have this hook defined for XFmode. IMO, we just have to use
IS_STACK_MODE macro instead (and perhaps flag_signaling_nans).

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

Richard Biener  changed:

   What|Removed |Added

   Keywords||wrong-code
 Status|UNCONFIRMED |NEW
   Last reconfirmed||2016-06-09
 CC||uros at gcc dot gnu.org
 Ever confirmed|0   |1

--- Comment #3 from Richard Biener  ---
It looks like RTL expansion uses DFmode to copy the aggregate which is likely
because the backend (or stor-layout.c) assigns DFmode to struct s.  A similar
issue should exist on Itanium.

This means the x86 target, to fix this particular case, should implement
targetm.member_type_forces_blk for x87 modes (and -fsignalling-nans?).


;; y = x;

(insn 19 18 20 (set (reg:DF 96)
(mem/c:DF (plus:SI (reg/f:SI 82 virtual-stack-vars)
(const_int -8 [0xfff8])) [0 x+0 S8 A32])) t2.c:16
-1
 (nil))

(insn 20 19 0 (set (mem/c:DF (plus:SI (reg/f:SI 82 virtual-stack-vars)
(const_int -16 [0xfff0])) [0 y+0 S8 A32])
(reg:DF 96)) t2.c:16 -1
 (nil))

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[joseph at codesourcery dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #2 from joseph at codesourcery dot com  ---
On Wed, 8 Jun 2016, ch3root at openwall dot com wrote:

> - padding in long double is not copied. Probably ok (in the same way as 
> padding
> in substructures is not always copied);

Yes, I think that's fine in struct copying.

> - sNaNs are turned into qNaNs on x86-32 if traps are turned off. Probably ok 
> if
> sNaNs and qNaNs are considered different representations of the same value;

For *scalar* assignment that would be fine because of TS 18661-1 saying 
"Whether C assignment (6.5.16) (and conversion as if by assignment) to the 
same format is an IEC 60559 convertFormat or copy operation is 
implementation-defined, even if  defines the macro 
FE_SNANS_ALWAYS_SIGNAL (F.2.1).".  It's rather less clear for struct 
assignment.

I expect the present bug is essentially the same as bug 58416 (which 
concerns a union, where the floating-point member isn't even the active 
one).

[Bug target/71460] Copying structs can trap (on x86-32) due to SNaN to QNaN

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71460

--- Comment #1 from Andrew Pinski  ---
I think this is really a dup of bug 57484.  The problem is x87 related and
there is not much to be done.