Bug 85661 - double comparison illegally statically reduced
Summary: double comparison illegally statically reduced
Status: RESOLVED DUPLICATE of bug 323
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 7.3.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-05-05 04:54 UTC by manuel.serrano
Modified: 2018-05-05 15:36 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description manuel.serrano 2018-05-05 04:54:24 UTC
Double comparisons are sometime erroneously resolved by the compiler. Let's us
consider the following C code:

% cat bar.c
int BGl_testz00zzbugz002(double BgL_lhs1029z00_4) {
   double BgL_tmpz00_1191 = BgL_lhs1029z00_4 * ((double) 0.1);
   return (((double) 0.0) == BgL_tmpz00_1191);
}

The generated assembly code is:
% gcc -v -O1 -c  bar.c -S
% cat bar.s
	.file	"bar.c"
	.text
	.globl	BGl_testz00zzbugz002
	.type	BGl_testz00zzbugz002, @function
BGl_testz00zzbugz002:
.LFB0:
	.cfi_startproc
	call	__x86.get_pc_thunk.ax
	addl	$_GLOBAL_OFFSET_TABLE_, %eax
	fldl	.LC0@GOTOFF(%eax)
	fmull	4(%esp)
	fldz
	fucomip	%st(1), %st
	fstp	%st(0)
	setnp	%al
	movzbl	%al, %eax
	movl	$0, %edx
	cmovne	%edx, %eax
	ret
	.cfi_endproc
.LFE0:
	.size	BGl_testz00zzbugz002, .-BGl_testz00zzbugz002
	.section	.rodata.cst8,"aM",@progbits,8
	.align 8
.LC0:
	.long	-1717986918
	.long	1069128089
	.section	.text.__x86.get_pc_thunk.ax,"axG",@progbits,__x86.get_pc_thunk.ax,comdat
	.globl	__x86.get_pc_thunk.ax
	.hidden	__x86.get_pc_thunk.ax
	.type	__x86.get_pc_thunk.ax, @function
__x86.get_pc_thunk.ax:
.LFB1:
	.cfi_startproc
	movl	(%esp), %eax
	ret
	.cfi_endproc
.LFE1:
	.ident	"GCC: (Debian 7.3.0-17) 7.3.0"
	.section	.note.GNU-stack,"",@progbits

As far I understand it, BGl_testz00zzbugz002 always returns 0, even is
called with 5e-324 (min double value) which yields the multiplication to
return 0.

Now, lets modify the function for:

#include <stdio.h>

int BGl_testz00zzbugz002(double BgL_lhs1029z00_4) {
   double BgL_tmpz00_1191 = BgL_lhs1029z00_4 * ((double) 0.1);
   fprintf( stderr, "FOO\n" );
   return (((double) 0.0) == BgL_tmpz00_1191);
}

That is, let's just add a call to fprintf before the return statement.
The generated assembly code is now correct:

% gcc -v -O1 -c  bar.c -S
% cat bar.s
.file	"bar.c"
	.text
	.section	.rodata.str1.1,"aMS",@progbits,1
.LC0:
	.string	"FOO\n"
	.text
	.globl	BGl_testz00zzbugz002
	.type	BGl_testz00zzbugz002, @function
BGl_testz00zzbugz002:
.LFB11:
	.cfi_startproc
	pushl	%ebx
	.cfi_def_cfa_offset 8
	.cfi_offset 3, -8
	subl	$24, %esp
	.cfi_def_cfa_offset 32
	call	__x86.get_pc_thunk.bx
	addl	$_GLOBAL_OFFSET_TABLE_, %ebx
	fldl	32(%esp)
	fstpl	8(%esp)
	movl	stderr@GOT(%ebx), %eax
	pushl	(%eax)
	.cfi_def_cfa_offset 36
	pushl	$4
	.cfi_def_cfa_offset 40
	pushl	$1
	.cfi_def_cfa_offset 44
	leal	.LC0@GOTOFF(%ebx), %eax
	pushl	%eax
	.cfi_def_cfa_offset 48
	call	fwrite@PLT
	fldl	.LC1@GOTOFF(%ebx)
	fmull	24(%esp)
	fldz
	fucomip	%st(1), %st
	fstp	%st(0)
	setnp	%al
	movzbl	%al, %eax
	movl	$0, %edx
	cmovne	%edx, %eax
	addl	$40, %esp
	.cfi_def_cfa_offset 8
	popl	%ebx
	.cfi_restore 3
	.cfi_def_cfa_offset 4
	ret
	.cfi_endproc
.LFE11:
	.size	BGl_testz00zzbugz002, .-BGl_testz00zzbugz002
	.section	.rodata.cst8,"aM",@progbits,8
	.align 8
.LC1:
	.long	-1717986918
	.long	1069128089
	.section	.text.__x86.get_pc_thunk.bx,"axG",@progbits,__x86.get_pc_thunk.bx,comdat
	.globl	__x86.get_pc_thunk.bx
	.hidden	__x86.get_pc_thunk.bx
	.type	__x86.get_pc_thunk.bx, @function
__x86.get_pc_thunk.bx:
.LFB12:
	.cfi_startproc
	movl	(%esp), %ebx
	ret
	.cfi_endproc
.LFE12:
	.ident	"GCC: (Debian 7.3.0-17) 7.3.0"
	.section	.note.GNU-stack,"",@progbits

Here are the other informations you ask to provide when filling a bug report.

% gcc -v -O1 -c  bar.c -S -save-temps 
Using built-in specs.
COLLECT_GCC=gcc
Target: i686-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 7.3.0-17' --with-bugurl=file:///usr/share/doc/gcc-7/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++ --prefix=/usr --with-gcc-major-version-only --with-as=/usr/bin/i686-linux-gnu-as --with-ld=/usr/bin/i686-linux-gnu-ld --program-suffix=-7 --program-prefix=i686-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-plugin --enable-default-pie --with-system-zlib --with-target-system-zlib --enable-objc-gc=auto --enable-targets=all --enable-multiarch --disable-werror --with-arch-32=i686 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-checking=release --build=i686-linux-gnu --host=i686-linux-gnu --target=i686-linux-gnu
Thread model: posix
gcc version 7.3.0 (Debian 7.3.0-17) 
COLLECT_GCC_OPTIONS='-v' '-O1' '-c' '-S' '-save-temps' '-mtune=generic' '-march=i686'
 /usr/lib/gcc/i686-linux-gnu/7/cc1 -E -quiet -v -imultiarch i386-linux-gnu bar.c -mtune=generic -march=i686 -O1 -fpch-preprocess -o bar.i
ignoring nonexistent directory "/usr/local/include/i386-linux-gnu"
ignoring nonexistent directory "/usr/lib/gcc/i686-linux-gnu/7/../../../../i686-linux-gnu/include"
#include "..." search starts here:
#include <...> search starts here:
 /usr/lib/gcc/i686-linux-gnu/7/include
 /usr/local/include
 /usr/lib/gcc/i686-linux-gnu/7/include-fixed
 /usr/include/i386-linux-gnu
 /usr/include
End of search list.
COLLECT_GCC_OPTIONS='-v' '-O1' '-c' '-S' '-save-temps' '-mtune=generic' '-march=i686'
 /usr/lib/gcc/i686-linux-gnu/7/cc1 -fpreprocessed bar.i -quiet -dumpbase bar.c -mtune=generic -march=i686 -auxbase bar -O1 -version -o bar.s
GNU C11 (Debian 7.3.0-17) version 7.3.0 (i686-linux-gnu)
	compiled by GNU C version 7.3.0, GMP version 6.1.2, MPFR version 4.0.1, MPC version 1.1.0, isl version isl-0.19-GMP

GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
GNU C11 (Debian 7.3.0-17) version 7.3.0 (i686-linux-gnu)
	compiled by GNU C version 7.3.0, GMP version 6.1.2, MPFR version 4.0.1, MPC version 1.1.0, isl version isl-0.19-GMP

GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: 0871246de3664cbf1722cc29275b2ed4
COMPILER_PATH=/usr/lib/gcc/i686-linux-gnu/7/:/usr/lib/gcc/i686-linux-gnu/7/:/usr/lib/gcc/i686-linux-gnu/:/usr/lib/gcc/i686-linux-gnu/7/:/usr/lib/gcc/i686-linux-gnu/
LIBRARY_PATH=/usr/lib/gcc/i686-linux-gnu/7/:/usr/lib/gcc/i686-linux-gnu/7/../../../i386-linux-gnu/:/usr/lib/gcc/i686-linux-gnu/7/../../../../lib/:/lib/i386-linux-gnu/:/lib/../lib/:/usr/lib/i386-linux-gnu/:/usr/lib/../lib/:/usr/lib/gcc/i686-linux-gnu/7/../../../:/lib/:/usr/lib/
COLLECT_GCC_OPTIONS='-v' '-O1' '-c' '-S' '-save-temps' '-mtune=generic' '-march=i686'

% cat bar.i
# 1 "bar.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 31 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 32 "<command-line>" 2
# 1 "bar.c"


int BGl_testz00zzbugz002(double BgL_lhs1029z00_4) {
   double BgL_tmpz00_1191 = BgL_lhs1029z00_4 * ((double) 0.1);

   return (((double) 0.0) == BgL_tmpz00_1191);
}

Finally, the GCC version I use the is only currently available under Debian/Testing executed on a Linux x86 box.

% gcc --version
gcc (Debian 7.3.0-17) 7.3.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Please, let me know if this bug report is not sufficient for you to understand what's going on and if you need anything else.

Thanks in advance for your help.

--
Manuel

ps: You might object that running exact comparison of double is meaningless, this bug shows up when running the ECMAScript compliance
test with my JavaScript->C compiler. The original JavaScript test is:

% cat S11.5.1_A4_T7.js
/**
 * The result of a floating-point multiplication is governed by the rules of IEEE 754 double-precision arithmetics
 *
 * @path ch11/11.5/11.5.1/S11.5.1_A4_T7.js
 * @description If the magnitude is too small to represent, the result is then a zero of appropriate sign
 */

//CHECK#1
if (Number.MIN_VALUE * 0.1 !== 0) {
  $ERROR('#1: Number.MIN_VALUE * 0.1 === 0. Actual: ' + (Number.MIN_VALUE * 0.1));
}

//CHECK#2
if (-0.1 * Number.MIN_VALUE !== -0) {
  $ERROR('#2.1: -0.1 * Number.MIN_VALUE === -0. Actual: ' + (-0.1 * Number.MIN_VALUE));
} else {
  if (1 / (-0.1 * Number.MIN_VALUE) !== Number.NEGATIVE_INFINITY) {
    $ERROR('#2.2: -0.1 * Number.MIN_VALUE === -0. Actual: +0');
  }
}

...
Comment 1 Jakub Jelinek 2018-05-05 09:11:44 UTC
If this is on i?86 32-bit, then most likely you just aren't taking into account the extended precision and the default -fexcess-precision=fast mode.
So, either if you want excess precision, but require the C rules (and are using C), you need to use -fexcess-precision=standard (default in strict C std modes), or if you don't want excess precision, make sure you have SSE2 or later hw and use -msse2 -mfpmath=sse.  In any case, that is a user error rather than compiler bug.
Comment 2 manuel.serrano 2018-05-05 15:17:38 UTC
Dear Jakub,

Thank you very much for your useful answer. I was indeed ignorant of the -fexcess-precision=fast option and as you suggested "-msse2 -mfpmath=sse"
solves the problem. I'm still puzzled why inserting a dummy printf changes
the behaviour of the double equality test. Is that expected?
Comment 3 Andrew Pinski 2018-05-05 15:36:32 UTC
See bug 323.

*** This bug has been marked as a duplicate of bug 323 ***