Bug 13239 - [3.3/3.4 regression] Assertion does not seem to work correctly anymore
Summary: [3.3/3.4 regression] Assertion does not seem to work correctly anymore
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 3.3.3
: P1 critical
Target Milestone: 3.3.3
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2003-11-30 16:34 UTC by merkert
Modified: 2004-01-17 04:22 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2003-12-02 23:38:51


Attachments
Assembly code (800 bytes, text/plain)
2003-12-02 03:13 UTC, merkert
Details

Note You need to log in before you can comment on or make changes to this bug.
Description merkert 2003-11-30 16:34:47 UTC
The following code fails the assertion, but I don't understand why. I does work
with 2.95.3. I think it fails in the builtin_expect function.

cat > test.cpp <<EOF
#include <assert.h>

struct Y {
  Y () : _y(0) {}
  int _y;
  int y () const  { return _y; }

};

struct X {
  X () {}

  Y _y;

  inline int x() const { return 0; }
  inline Y y() const { return _y; }

};


int main()
{
  X x;
  assert (x.y().y()==0);

  assert ( ((x.x()==0) && (x.y().y()==0)) );
  return 0;
}
EOF

g++ -DDEBUG -o test test.cpp && ./test

If I replace Y::y() with "return 0;" then it works.

Maybe I'm doing something wrong?? I just installed the new SuSe 9.0 and it also
fails with their compiler.

Here's also the preprocessed source in case my system is not setup right.

# 1 "test.cpp"
# 1 "<built-in>"
# 1 "<command line>"
# 1 "test.cpp"
# 1 "/usr/include/assert.h" 1 3 4
# 36 "/usr/include/assert.h" 3 4
# 1 "/usr/include/features.h" 1 3 4
# 295 "/usr/include/features.h" 3 4
# 1 "/usr/include/sys/cdefs.h" 1 3 4
# 296 "/usr/include/features.h" 2 3 4
# 318 "/usr/include/features.h" 3 4
# 1 "/usr/include/gnu/stubs.h" 1 3 4
# 319 "/usr/include/features.h" 2 3 4
# 37 "/usr/include/assert.h" 2 3 4
# 65 "/usr/include/assert.h" 3 4
extern "C" {


extern void __assert_fail (__const char *__assertion, __const char *__file,
                           unsigned int __line, __const char *__function)
     throw () __attribute__ ((__noreturn__));


extern void __assert_perror_fail (int __errnum, __const char *__file,
                                  unsigned int __line,
                                  __const char *__function)
     throw () __attribute__ ((__noreturn__));




extern void __assert (const char *__assertion, const char *__file, int __line)
     throw () __attribute__ ((__noreturn__));


}
# 2 "test.cpp" 2

struct Y {
  Y () : _y(0) {}
  int _y;
  int y () const { return _y; }

};

struct X {
  X () {}

  Y _y;

  inline int x() const { return 0; }
  inline Y y() const { return _y; }

};


int main()
{
  X x;
  (static_cast<void> (__builtin_expect (!!(x.y().y()==0), 1) ? 0 :
(__assert_fail ("x.y().y()==0", "test.cpp", 24, __PRETTY_FUNCTION__), 0)));

  (static_cast<void> (__builtin_expect (!!(((x.x()==0) && (x.y().y()==0))), 1) ?
0 : (__assert_fail ("((x.x()==0) && (x.y().y()==0))", "test.cpp", 26,
__PRETTY_FUNCTION__), 0)));
  return 0;
}
Comment 1 Wolfgang Bangerth 2003-12-01 15:54:01 UTC
This works with my installation of 3.3.2 (release). Maybe someone else
can double-check?

W.
Comment 2 merkert 2003-12-02 03:13:07 UTC
Created attachment 5262 [details]
Assembly code

This assembly code was generated from this example:

#include <assert.h>

struct Y {
  Y () : _y(0) {}
  int _y;
  int y () const  { return _y; }

};

struct X {
  X () {}

  Y _y;

  int x() const { return 1; }
  Y z() const { return _y; }

};


int main()
{
  X x;
  assert (x.z().y()==0);
  assert (((x.x()==1) && (x.z().y()==0)) );
  return 0;
}
Comment 3 merkert 2003-12-02 03:16:32 UTC
I've modified the code slightly and I'm attaching the generated assembly code
from :  /local/gcc-3.3.3/bin/g++ -o test.S test.cpp -S -march=i386


You can see where the problem is :
there are three functions : 
_ZNK1X1xEv
_ZNK1X1zEv
_ZNK1Y1yEv

You can see that at labels .LCFI3 and .LCFI4 there are calls to  and _ZNK1X1zEv
and _ZNK1Y1yEv, respectively (i.e. X::x, and X::z, and Y::y). This corresponds
to the first assertion.

Around label .L3, there are only calls to _ZNK1X1xEv and _ZNK1Y1yEv, which means
the call to X::z is omitted in the second assertion.


Any ideas what this problem might be? I did build the compiler with gcc from
SuSe's 9.0 distribution. Could this have caused this problem? 
I have to say that with the old SuSe8.2 this worked for me, so I suspect suse to
be the root cause. 
Comment 4 merkert 2003-12-02 22:32:24 UTC
Here's another, slightly simplified, example: 

#include <assert.h>

struct Y {
  Y () : _y(0) {}
  //Y (const Y& y) : _y(y._y) {}
  int _y;
};

bool foo() { return true; }
Y bar() { return Y(); }

int main()
{
  assert (bar()._y==0);
  assert ( foo() && (bar()._y)==0 );
  return 0;
}

If the copy constructor is uncommented, then it works. It almost looks as if it
does not generate the temporary Y object and just pretends it has got it. I've
tried this on a solaris and it worked and I also tried it on a cygwin (3.1 ?
maybe) which worked as well.

Can someone verify that their system is using builtin_expect rather than general
assert?
Comment 5 Wolfgang Bangerth 2003-12-02 23:00:46 UTC
I can confirm that this indeed fails with the gcc 3.3.1 SuSE ships. 
I don't have a 3.3.x installed (yet) on this system, so can't presently 
check that it is a problem tied to the OS or with gcc. 
 
W. 
Comment 6 Wolfgang Bangerth 2003-12-02 23:38:50 UTC
Here's something self-contained: 
----------------------------- 
extern "C" void abort(void); 
 
struct Y { 
  int i; 
}; 
 
bool foo() { return true; } 
Y bar() { Y y = {0}; return y; } 
 
int main() 
{ 
  __builtin_expect (foo() && (bar().i)==0, 0) ? 0 : (abort(),1); 
} 
------------------------ 
This aborts when compiled with SuSE's 3.3.1. It also aborts with our 
present mainline, but not with 3.2.3. So the bug is for real. I'll 
try to figure out whether it is a mainline bug in FSF sources that SuSE 
happened to import into their tree, or if we also have this problem in 
3.3 CVS. 
 
W. 
 
W. 
Comment 7 merkert 2003-12-02 23:40:54 UTC
I think it's the __builtin_expect that does not work. This example is reduced to
use of __builtin_expect only and it should be possible to replicate it on any
system that uses builtin_expect.

The key to this problem is the use the expression 
"__builtin_expect (... ,1) ? 0 : -1"  (replace -1 with any number other than 1)
and the example fails.  Use 1 and it works! 

Looks like an optimization problem.



#include <cstdio>

struct Y {
  Y () : _y(0) {}
  //Y (const Y& y) : _y(y._y) {}
  int _y;
};
extern Y bar();
extern bool foo();


int main()
{
  int x = __builtin_expect(foo() && (bar()._y)==0,1) ? 0 : -1;
  ::std::printf("%d\n",x);
  return x;
}

bool foo() { return true; }
Y bar() { return Y(); }


Comment 8 Wolfgang Bangerth 2003-12-03 00:04:42 UTC
I just built a 3.3 CVS here, and it fails, too.  
 
The bug is an interaction between __builtin_expect and the function 
call. I tried to either replace the __builtin_expect with just 
the conditional expression, or to do something like 
  return (__builtin_expect(...) ? 0 : 1); 
to avoid the call to abort(), but the bug went away in both cases. 
 
W. 
Comment 9 Andrew Pinski 2003-12-05 23:09:18 UTC
This works correctly the tree-ssa branch, why? (is the C++ to gimple doing something which 
causes this to work?).
Comment 10 Wolfgang Bangerth 2003-12-06 20:59:48 UTC
I remember that changing small bits in the testcase made the bug go away. It may be 
that the intermediate transformations done in tree-ssa just don't trigger the bug, i.e. it 
is not fixed there but only hidden. 
 
W. 
Comment 11 janis187 2003-12-10 17:40:43 UTC
The regression in PR 13239 was introduced or exposed with this patch:

2003-03-24  Jakub Jelinek  <jakub@redhat.com>

        * dojump.c (do_jump): Handle UNSAVE_EXPR specially.

The regression hunt took place with mainline on i686-pc-linux-gnu using
the test case from comment #6.  The reports of where the bug has and has
not been seen made it sound as if this might be an intermittent failure;
I built g++ from sources for every two weeks between 2003-01-01 and
2003-07-02, and the results are consistent with the patch named above
changing the behavior of the test.
Comment 12 GCC Commits 2003-12-19 13:39:09 UTC
Subject: Bug 13239

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	jakub@gcc.gnu.org	2003-12-19 13:39:05

Modified files:
	gcc            : ChangeLog builtins.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/opt: expect1.C 

Log message:
	PR c++/13239
	* builtins.c (expand_builtin_expect_jump): Update
	TREE_VALUE (arglist) if unsave_expr_now langhook
	created a new tree.
	
	* g++.dg/opt/expect1.C: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.2044&r2=2.2045
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/builtins.c.diff?cvsroot=gcc&r1=1.265&r2=1.266
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.3286&r2=1.3287
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/opt/expect1.C.diff?cvsroot=gcc&r1=NONE&r2=1.1

Comment 13 GCC Commits 2003-12-19 13:54:39 UTC
Subject: Bug 13239

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_3-branch
Changes by:	jakub@gcc.gnu.org	2003-12-19 13:54:37

Modified files:
	gcc            : ChangeLog builtins.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/opt: expect1.C 

Log message:
	PR c++/13239
	* builtins.c (expand_builtin_expect_jump): Update
	TREE_VALUE (arglist) if unsave_expr_now langhook
	created a new tree.
	
	* g++.dg/opt/expect1.C: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.16114.2.849&r2=1.16114.2.850
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/builtins.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.165.2.8&r2=1.165.2.9
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.2261.2.338&r2=1.2261.2.339
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/opt/expect1.C.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=NONE&r2=1.1.2.1

Comment 14 Andrew Pinski 2003-12-21 22:21:09 UTC
Fixed for 3.3.3 and 3.4.