Bug 13371 - [3.3.2 regression] infinite loop with packed struct and inlining
Summary: [3.3.2 regression] infinite loop with packed struct and inlining
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 3.3.2
: P2 critical
Target Milestone: 3.3.3
Assignee: Jason Merrill
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2003-12-10 07:32 UTC by dje
Modified: 2003-12-19 20:25 UTC (History)
3 users (show)

See Also:
Host: i686-pc-linux-gnu
Target: i686-pc-linux-gnu
Build: i686-pc-linux-gnu
Known to work:
Known to fail:
Last reconfirmed: 2003-12-10 08:27:57


Attachments
slightly smaller testcase (231 bytes, text/plain)
2003-12-11 08:12 UTC, Dara Hazeghi
Details

Note You need to log in before you can comment on or make changes to this bug.
Description dje 2003-12-10 07:32:50 UTC
cc1plus generates bad code for this testcase.
An infinite loop is generated.  I think it's because a scope statement is
emitted twice (or something like that).  you can see
the bug in the .00.rtl file.
I was able to recreate this with the gcc-ss-3_3-20031208 tag.

Compile with ./cc1plus -quiet -O2 foo.cc

enum state_e
{
    STATE_0 = 0,
    STATE_1 = 1,
};
                                                                                
struct packed_state_t
{
    state_e state : 8;
} __attribute__ ((packed));
                                                                                
class foo_c
{
 public:
                                                                                
    void set_state (unsigned p, state_e s);
};
                                                                                
class bar_c
{
 public:
                                                                                
  struct
  {
    unsigned a;
    unsigned b;
  } r;
                                                                                
  __inline__ unsigned get_a () const { return this->r.a; }
                                                                                
  __inline__ unsigned get_b () const { return this->r.b; }
                                                                                
  __inline__ unsigned
  get_u (unsigned a) const
    {
      return a + this->get_b ();
    }
                                                                                
  packed_state_t* state;
                                                                                
  foo_c* foo_if;
                                                                                
  void set_state (unsigned p, state_e s);
};
                                                                                
enum state_params_e
{
    STATE_LOG_CHUNK_SZ = 7,
    STATE_CHUNK_SZ = 1 << STATE_LOG_CHUNK_SZ
};
static __inline__ unsigned
get_state_index (unsigned a)
{
    return a >> STATE_LOG_CHUNK_SZ;
}
                                                                                
void
bar_c::set_state (unsigned p, state_e s)
{
    unsigned u = this->get_u (p);
    if (u < this->get_a ())
    {
        this->state [get_state_index (u)].state = s;
        return;
    }
                                                                                
    this->foo_if->set_state (p, s);
}

Study the resulting assembly for the `if' clause and you'll see this:

.L7:
        movl    %edx, %eax
        shrl    $7, %eax
        movb    %bl, (%eax,%ecx)
        jmp     .L7
Comment 1 Dara Hazeghi 2003-12-10 08:27:53 UTC
Same problem code is seen on 3.3 branch, but not on mainline or 3.3.1.
Comment 2 dje 2003-12-10 18:52:48 UTC
(In reply to comment #1)
> Same problem code is seen on 3.3 branch, but not on mainline or 3.3.1.

Thanks for checking 3.3.1.  Diffing the ChangeLog's and doing some
semi-intelligent guessing, I narrowed it down to this patch.

2003-09-07  Jason Merrill  <jason@redhat.com>

       PR c++/12181
       * typeck.c (build_modify_expr): Don't always stabilize the lhs and
       rhs.  Do stabilize the lhs of a MODIFY_EXPR used on the lhs.

Sorry Jason. :-)
Comment 3 Dara Hazeghi 2003-12-11 08:12:23 UTC
Created attachment 5311 [details]
slightly smaller testcase
Comment 4 Dara Hazeghi 2003-12-11 08:15:02 UTC
I'm not sure if the reason why this doesn't occur on mainline is luck, or a bug in backporting. Mark 
you did the backport. Any suggestions? Thanks.
Comment 5 Mark Mitchell 2003-12-15 06:39:25 UTC
I really have no idea what's wrong.  It probably makes sense to ask Jason, since
it looks like this was this was his bug originally?
Comment 6 Dara Hazeghi 2003-12-15 22:24:30 UTC
Jason, the backport of your fix to PR12181 causes this regression on the 3.3 branch. Any 
suggestions? Thanks.
Comment 7 GCC Commits 2003-12-19 20:23:46 UTC
Subject: Bug 13371

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_3-branch
Changes by:	jason@gcc.gnu.org	2003-12-19 20:23:40

Modified files:
	gcc/cp         : ChangeLog typeck.c 

Log message:
	PR c++/13371
	* typeck.c (build_modify_expr): Stabilize lhs if we're narrowing.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.3076.2.224&r2=1.3076.2.225
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/typeck.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.436.2.19&r2=1.436.2.20

Comment 8 GCC Commits 2003-12-19 20:25:08 UTC
Subject: Bug 13371

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	jason@gcc.gnu.org	2003-12-19 20:25:04

Modified files:
	gcc/cp         : ChangeLog typeck.c 

Log message:
	PR c++/13371
	* typeck.c (build_modify_expr): Stabilize lhs if we're narrowing.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&r1=1.3821&r2=1.3822
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/typeck.c.diff?cvsroot=gcc&r1=1.513&r2=1.514

Comment 9 GCC Commits 2003-12-19 20:25:27 UTC
Subject: Bug 13371

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	jason@gcc.gnu.org	2003-12-19 20:25:22

Added files:
	gcc/testsuite/g++.dg/init: bitfield2.C 

Log message:
	PR c++/13371
	* typeck.c (build_modify_expr): Stabilize lhs if we're narrowing.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/init/bitfield2.C.diff?cvsroot=gcc&r1=NONE&r2=1.1

Comment 10 Jason Merrill 2003-12-19 20:25:56 UTC
Fixed.