[Bug c++/93934] New: Unnecessary fld of uninitialized float stack variable results in ub of valid C++ code
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93934
Bug ID: 93934
Summary: Unnecessary fld of uninitialized float stack variable
results in ub of valid C++ code
Product: gcc
Version: 7.4.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: vajdaz at protonmail dot com
Target Milestone: ---
Valid C++ code is compiled to assembly that seems to be buggy. Below source
code and assembly output of the compiler.
Contents of source file float.cc:
extern void foo();
extern void bar();
double func(int size, double d, bool b) {
double result;
for(int i = 0; i < size; ++i) {
if (i == 0) {
foo();
if(b) result = d;
}
else {
bar();
if(b) result = 42.0;
}
}
return b && size > 0 ? result : 7.0;
}
I assume that this is valid C++ code. On C++ level there is no way to access an
uninitialized variable. The variable result is only used as a return value if
size is greater than zero, and b is true, in which case result will be
initialized before, for sure.
Create assembly:
$ g++ -m32 -O2 -fno-pic -fno-omit-frame-pointer -S float.cc
Output is:
.file "float.cc"
.text
.p2align 4,,15
.globl _Z4funcidb
.type _Z4funcidb, @function
_Z4funcidb:
.LFB0:
.cfi_startproc
pushl %ebp
.cfi_def_cfa_offset 8
.cfi_offset 5, -8
movl%esp, %ebp
.cfi_def_cfa_register 5
pushl %edi
pushl %esi
pushl %ebx
subl$44, %esp
.cfi_offset 7, -12
.cfi_offset 6, -16
.cfi_offset 3, -20
movl8(%ebp), %ebx
movl20(%ebp), %eax
fldl12(%ebp)
testl %ebx, %ebx
fstpl -48(%ebp)
movl%eax, -36(%ebp)
jle .L2
movl%eax, %esi
xorl%edi, %edi
.p2align 4,,10
.p2align 3
.L5:
testl %edi, %edi
je .L16
call_Z3barv
movl%esi, %eax
testb %al, %al
je .L4
flds.LC0
fstpl -32(%ebp)
.L4:
addl$1, %edi
cmpl%edi, %ebx
jne .L5
cmpb$0, -36(%ebp)
fldl-32(%ebp)
je .L17
addl$44, %esp
popl%ebx
.cfi_remember_state
.cfi_restore 3
popl%esi
.cfi_restore 6
popl%edi
.cfi_restore 7
popl%ebp
.cfi_restore 5
.cfi_def_cfa 4, 4
ret
.p2align 4,,10
.p2align 3
.L17:
.cfi_restore_state
fstp%st(0)
.L2:
flds.LC1
addl$44, %esp
popl%ebx
.cfi_remember_state
.cfi_restore 3
popl%esi
.cfi_restore 6
popl%edi
.cfi_restore 7
popl%ebp
.cfi_restore 5
.cfi_def_cfa 4, 4
ret
.p2align 4,,10
.p2align 3
.L16:
.cfi_restore_state
call_Z3foov
fldl-48(%ebp)
movl%esi, %eax
testb %al, %al
fldl-32(%ebp)
fcmovne %st(1), %st
fstp%st(1)
fstpl -32(%ebp)
jmp .L4
.cfi_endproc
.LFE0:
.size _Z4funcidb, .-_Z4funcidb
.section.rodata.cst4,"aM",@progbits,4
.align 4
.LC0:
.long 1109917696
.align 4
.LC1:
.long 1088421888
.ident "GCC: (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0"
.section.note.GNU-stack,"",@progbits
Following things happen in the assembly code when size is 1 and b is true for
example.
* The value of size is stored into %ebx
* The value of b is stored into %eax and -36(%ebp), later in %esi
* The value of d is loaded into the fpu register stack and stored into
-48(%ebp)
We continue straight to the line with "je .L16" and jump to the label .L16. And
here comes the interesting part:
.L16:
.cfi_restore_state
call_Z3foov
fldl-48(%ebp)<-- loading value of d into fpu
movl%esi, %eax
testb %al, %al <-- testing if b is true (for fcmovne below)
fldl-32(%ebp)<-- loading uninitialized stack variable result
fcmovne %st(1), %st <-- choosing which value to put back into result
fstp%st(1)
fstpl -32(%ebp)
jmp .L4
Why we have here a combination of two fldl instructions and a fcmovne? This
causes a possible fpu exception if the value of the uninitialized variable
named result is accidentally a SNaN value. Which means, the behavior of this
code is unpredictable.
This can be reproduced with any GCC since 4.4.7, if the -march option is set to
somethi
[Bug target/93934] Unnecessary fld of uninitialized float stack variable results in ub of valid C++ code
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93934 Zoltan Vajda changed: What|Removed |Added Status|RESOLVED|UNCONFIRMED Resolution|INVALID |--- --- Comment #3 from Zoltan Vajda --- (In reply to Alexander Monakov from comment #2) > fcmov can only raise an x87 fpu exception on x87 stack underflow, which > cannot happen here. > > Even if it did raise FE_INVALID for SNaNs, note that GCC does not support > SNaNs by default; -fsignaling-nans can be specified to request that, but > note that documentation says the support is incomplete. > > No bug here afaict. The problem is not the fcmov instruction, but the "fldl -32(%ebp)". It loads an uninitialized value from the variable result, which may be a SNaN.
[Bug target/93934] Unnecessary fld of uninitialized float stack variable results in ub of valid C++ code
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93934 --- Comment #9 from Zoltan Vajda --- As I understand it, it is acknowledged, that this is a bug. However, the issue is in state NEW for a quite long time. The issue is still present in GCC 11.2. Do you see any chances for some progress in this case?
[Bug target/93934] Unnecessary fld of uninitialized float stack variable results in ub of valid C++ code
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93934 --- Comment #12 from Zoltan Vajda --- Using -mfpmath=sse here does not help on a 32 bit platfrom. https://gcc.godbolt.org/z/hs1Ef6aj4 At line 31 the assembly code performs the speculative load.
[Bug target/93934] Unnecessary fld of uninitialized float stack variable results in ub of valid C++ code
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93934 --- Comment #15 from Zoltan Vajda --- In my special case, I have an embedded realtime application with a lot of FP atithmetic on Intel 32 bit architecture (huge and complex legacy codebase). FPU exceptions are enabled, so loading an SNaN results in an exception. This is intended, and we will don't want to change this configuration. In this context the generated ASM code does result in an fld of an uninitialized local variable, where looking on the C++ code such an access should not be possible. If the content of the uninitialized local variable happens to be a SNaN by accident (chances are very small, but not zero), an FPU exception happens. And again, based on the C++ code no FPU exception should be possible (assuming d is not an SNaN). Here is a synthetic example that triggers the exception by "placing a bomb" on the stack. https://gcc.godbolt.org/z/aooex6dcT Function place_bomb() has an effect on what happens in func(). That should not be the case! This is all valid C++ code. This may now accidentally happen in our application. The behavior is unpredictable, because it depends on what previous function calls left on the stack. If you change "-march=i686" to "-march=i386" in the example linked above, everything goes fine.
[Bug target/93934] Unnecessary fld of uninitialized float stack variable results in ub of valid C++ code
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93934 --- Comment #19 from Zoltan Vajda --- (In reply to Uroš Bizjak from comment #18) > The following patch fixes the PR, see the comment inline: > > --cut here-- > diff --git a/gcc/config/i386/i386-expand.c b/gcc/config/i386/i386-expand.c > index 6e2b7920d2b..b87490fe544 100644 > --- a/gcc/config/i386/i386-expand.c > +++ b/gcc/config/i386/i386-expand.c > @@ -4094,6 +4094,15 @@ ix86_expand_fp_movcc (rtx operands[]) > && !TARGET_64BIT)) > return false; > > + /* Disable SFmode and DFmode x87 FCMOV with trapping math > + to prevent speculative load using FLDL/FLDS from uninitialized > + memory location, which can contain sNaN value. FLDL/FLDS traps > + on sNaN, see PR93934. */ > + > + if ((mode == SFmode || mode == DFmode) > + && flag_trapping_math) > +return false; > + >/* The floating point conditional move instructions don't directly > support conditions resulting from a signed integer comparison. */ > > --cut here-- The problem does not only apply for conditional moves! I can turn on sse, for example. https://gcc.godbolt.org/z/jP3Kne8T5 Then the problematic code with the conditional move disappears, but I have a similar speculative fld problem in another situation. .L10: inc esi cmp edi, esi jne .L11 testbl, bl<= test input variable 'b' fld QWORD PTR [ebp-24]<= load of (maybe) uninitialized 'result' je .L24 <= jump based on value of 'b' add esp, 20 pop ebx pop esi pop edi pop ebp ret .L24: fstpst(0) .L8: fld QWORD PTR .LC1 add esp, 20 pop ebx pop esi pop edi pop ebp ret
[Bug target/93934] Unnecessary fld of uninitialized float stack variable results in ub of valid C++ code
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93934
--- Comment #23 from Zoltan Vajda ---
(In reply to Uroš Bizjak from comment #20)
> (In reply to jos...@codesourcery.com from comment #16)
> > I don't think this bug is anything to do with -fsignaling-nans, for the
> > same reason as applies to bug 58416 and bug 71460.
>
> The situation is hopeless from the beginning. Please consider this testcase:
>
> --cut here--
> #include
> #include
>
> double
> __attribute__((noinline,noipa))
> foo (double a, double b, char c)
> {
> return c ? a : b;
> }
>
> int main ()
> {
> double a = __builtin_nans ("");
> double b = 42.0;
>
> feclearexcept (FE_INVALID);
> foo (a, b, 0);
> if (fetestexcept (FE_INVALID))
> __builtin_abort ();
>
> return 0;
> }
> --cut here--
>
> $ gcc -O2 -m32 -march=i686 -lm fcmov.c
> $ ./a.out
> Aborted (core dumped)
> $ gcc -O2 -m32 -march=i386 -lm fcmov.c
> $ ./a.out
> Aborted (core dumped)
>
> Because the compiler generates:
>
> foo:
> cmpb$0, 20(%esp)
> fldl12(%esp)
> fldl4(%esp)
> fcmove %st(1), %st
> fstp%st(1)
> ret
>
> in the former case and:
>
> foo:
> fldl4(%esp)
> fldl12(%esp)
> cmpb$0, 20(%esp)
> jne .L4
> fstp%st(1)
> jmp .L2
> .L4:
> fstp%st(0)
> .L2:
> ret
>
> in the later.
>
> Since the ABI specifies the operand size on the stack, the above code will
> always trap.
There is a small but very important difference between this example and my
example.
In this example the compiler may assume that objects 'a' and 'b' both are
initialized at the beginning of the function execution (calling a function with
uninitialized input by value would be UB). Therefore you may argue that
accessing both of them is fine. If you feed foo() with SNaN through 'a', it
will always abort, independent of 'c'.
In my example there is an uninitialized object when the function execution
starts (local variable 'result') , and at C++ level there is no execution path
that would result in accessing this object before initialization. In spite of
this, at ASM level, the object is accessed before initialization. I don't see
any argument that makes this access valid. My example function aborts randomly,
independent of the input. The behavior depends on some random data on the
stack.
[Bug c++/119120] New: Unnecessary fld when assigning real or imaginary part of a complex variable
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119120
Bug ID: 119120
Summary: Unnecessary fld when assigning real or imaginary part
of a complex variable
Product: gcc
Version: 14.2.1
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: vajdaz at protonmail dot com
Target Milestone: ---
Following C++ code generates the assembly underneath when compiling with -O0
for x86 architecture (32 bit).
C++ source:
#include
void foo(const std::complex& z) {
_Complex double __t;
__real__ __t = z.real();
__imag__ __t = z.imag();
}
Generated asm code:
foo(std::complex const&):
pushl %ebp
movl%esp, %ebp
subl$40, %esp
subl$12, %esp
pushl 8(%ebp)
callstd::complex::real[abi:cxx11]() const
addl$16, %esp
fstpl -16(%ebp)
fldl-24(%ebp) ; <= here an uninitialized load happens
fldl-16(%ebp)
fstpl -40(%ebp)
fstpl -32(%ebp)
subl$12, %esp
pushl 8(%ebp)
callstd::complex::imag[abi:cxx11]() const
addl$16, %esp
fstpl -24(%ebp)
fldl-24(%ebp)
fldl-16(%ebp)
fstpl -40(%ebp)
fstpl -32(%ebp)
nop
leave
ret
At the marked position loading of an uninitialized floating point value is done
unnecessarily. This may cause an unintended FPU exception if that value
accidentally is a signaling NaN.
The expected code here would be this one (no unnecessary load/store pairs):
foo(std::complex const&):
pushl %ebp
movl%esp, %ebp
subl$40, %esp
movl8(%ebp), %eax
movl%eax, (%esp)
callstd::complex::real() const
fldl(%eax)
fstpl -24(%ebp)
movl8(%ebp), %eax
movl%eax, (%esp)
callstd::complex::imag() const
fldl(%eax)
fstpl -16(%ebp)
leave
ret
And this is the code which is generated by GCC 4.5.3 and earlier. Later GCCs
generate the (in my opinion) wrong code.
This is not an academic problem. The implementation of operator /= of
std::complex has this behavior until GCC 8.5 (inclusive).
