Bug List: (This bug is not in your last search results)   Show last search results      Search page      Enter new bug
Bug#: 31806
Product:  
Component:  
Status: RESOLVED
Resolution: FIXED
Assigned To: Not yet assigned to anyone <unassigned@gcc.gnu.org>
Host:
Reported against  
Priority:  
Severity:  
Target Milestone:  
 
 
Target:
Reporter: Dirk Mueller <mueller@gcc.gnu.org>
Add CC:
CC:
Remove selected CCs
Build:
URL:
Summary:
Keywords:
Known to work:
Known to fail:

Attachment Description Type Created Size Actions
fix-pr31806.patch Patch patch 2007-05-30 15:06 432 bytes Edit | Diff
fix-pr31806.patch Patch patch 2007-05-30 15:22 533 bytes Edit | Diff
Create a New Attachment (proposed patch, testcase, etc.) View All

Bug 31806 depends on: 31809 Show dependency tree
Show dependency graph
Bug 31806 blocks:

Additional Comments:






View Bug Activity   |   Format For Printing   |   Clone This Bug


Description:   Last confirmed: 2007-05-30 14:30 Opened: 2007-05-03 22:29
Hi, 

the testcase below aborts with gcc 4.1.3 compiled with:
g++ -O2 -fno-threadsafe-statics

=== Cut ===
extern "C" void abort(void);

struct A
{
    void *d;
};

static const A& staticA()
{
    static A s_static;
    return s_static;
}

void assert_failed()
{
    abort();
}

A testMethod()
{
    static const A& s = staticA( );
    if (&s == 0)
        assert_failed();
    return s;
}

int main()
{
    testMethod();
    return 0;
}

// g++ -O2 -fno-inline -fno-threadsafe-statics -o kglobaltest kglobaltest.cpp
=== Cut ===

The testcase was extracted from a KDE miscompilation. -fno-schedule-insns2
seems to work around it. 

the bug is visible in assembler:

·       call·   _Z7staticAv·    #
·       movl·   _ZZ10testMethodvE1s, %ebx·      # s, s.1
·       movb·   $1, _ZGVZ10testMethodvE1s·      #,
·       testl·  %ebx, %ebx·     # s.1
·       movl·   %eax, _ZZ10testMethodvE1s·      # tmp61, s

the movl %eax,_ZZ10testMethodvE1s is moved below the movl·  
_ZZ10testMethodvE1s, %ebx, which causes the bug.

------- Comment #1 From Maxim Kuvyrkov 2007-05-30 14:30 -------
This is an aliasing issue.

true_dependence () returns '0' on mems from insns 15 and 58.  This is due to
MEM_READONLY_P () flag set on them.

(insn:HI 15 13 16 3 (set (mem/u/f/c/i:DI (symbol_ref:DI ("_ZZ10testMethodvE1s")
[flags 0x2] <var_decl 0x2aaaae171c60 s>) [5 s+0 S8 A64])
        (reg:DI 0 ax [62])) 81 {*movdi_1_rex64} (insn_list:REG_DEP_TRUE 14
(nil))
    (nil))

(insn 58 16 59 3 (set (reg/f:DI 3 bx [orig:59 s.1 ] [59])
        (mem/u/f/c/i:DI (symbol_ref:DI ("_ZZ10testMethodvE1s") [flags 0x2]
<var_decl 0x2aaaae171c60 s>) [5 s+0 S8 A64])) 81 {*movdi_1_rex64} (nil)
    (expr_list:REG_EQUIV (mem/u/f/c/i:DI (symbol_ref:DI ("_ZZ10testMethodvE1s")
[flags 0x2] <var_decl 0x2aaaae171c60 s>) [5 s+0 S8 A64])
        (nil)))

I believe that comment in true_dependence ():
  /* Read-only memory is by definition never modified, and therefore can't
     conflict with anything.  We don't expect to find read-only set on MEM,
     but stupid user tricks can produce them, so don't die.  */
conflicts with comment describing MEM_READONLY_P flag in rtl.h:
  /* 1 in a REG, MEM, or CONCAT if the value is set at most once, anywhere.  */

In mainline this bug is latent because code that initialize static variable and
code that use it are in the separate basic blocks.  As long as scheduling works
on individual basic blocks the bug is hidden.

Confirm with fedora 6 system compiler.

------- Comment #2 From Maxim Kuvyrkov 2007-05-30 15:06 -------
Created an attachment (id=13634) [edit]
Patch

This patch fixes the issue but wasn't yet bootstrapped or tested in any other
way.

------- Comment #3 From Andrew Pinski 2007-05-30 15:12 -------
I don't think MEM_READONLY_P should be set.  I think this is related to PR
31809.

------- Comment #4 From Maxim Kuvyrkov 2007-05-30 15:22 -------
Created an attachment (id=13635) [edit]
Patch

Updated patch.

------- Comment #5 From Maxim Kuvyrkov 2007-05-30 15:27 -------
(In reply to comment #3)
> I don't think MEM_READONLY_P should be set.  I think this is related to PR
> 31809.
>

Memory location in question is a constant static variable in a function.  When
the function is run for the first time it will be initialized and after that
will behave like a normal MEM_READONLY_P ().

------- Comment #6 From Andrew Pinski 2007-05-30 15:56 -------
Here is the comment from rtl.h about MEM_READONLY_P:
/* 1 if RTX is a mem that is statically allocated in read-only memory.  */
And not:
  /* 1 in a REG, MEM, or CONCAT if the value is set at most once, anywhere.  */

Oh someone forgot to update that part of rtl.h (the first one is the true
definition).  The second comment for unchanging is the old definition pre
4.0.x.  This was changed because the exact reasons here.  It was hard to define
what was the initialize setting.

So this is a front-end bug.

------- Comment #7 From Jakub Jelinek 2007-05-31 16:40 -------
Subject: Bug 31806

Author: jakub
Date: Thu May 31 16:40:12 2007
New Revision: 125229

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=125229
Log:
        PR c++/31806
        * decl.c (cp_finish_decl): Also clear was_readonly if a static var
        needs runtime initialization.

Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/decl.c

------- Comment #8 From Dirk Mueller 2007-06-06 15:02 -------
testcase works with gcc 3.4 and gcc 3.3

------- Comment #9 From Dirk Mueller 2007-06-08 21:48 -------
Subject: Bug 31806

Author: mueller
Date: Fri Jun  8 21:48:34 2007
New Revision: 125580

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=125580
Log:
2007-06-08  Dirk Mueller  <dmueller@suse.de>

       PR c++/31809
       PR c++/31806
       Backport from mainline:
       2007-05-31  Jakub Jelinek  <jakub@redhat.com>

       * decl.c (cp_finish_decl): Also clear was_readonly if a static var
       needs runtime initialization.

       2007-05-30  Jakub Jelinek  <jakub@redhat.com>

       * decl.c (cp_finish_decl): Clear TREE_READONLY flag on TREE_STATIC
       variables that need runtime initialization.

       PR c++/31809
       Backport from mainline:
       2007-05-30  Jakub Jelinek  <jakub@redhat.com>

       * g++.dg/opt/static5.C: New test.

Added:
    branches/gcc-4_2-branch/gcc/testsuite/g++.dg/opt/static5.C
      - copied unchanged from r125201, trunk/gcc/testsuite/g++.dg/opt/static5.C
Modified:
    branches/gcc-4_2-branch/gcc/cp/ChangeLog
    branches/gcc-4_2-branch/gcc/cp/decl.c
    branches/gcc-4_2-branch/gcc/testsuite/ChangeLog

------- Comment #10 From Dirk Mueller 2007-06-14 23:12 -------
Subject: Bug 31806

Author: mueller
Date: Thu Jun 14 23:12:25 2007
New Revision: 125726

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=125726
Log:
2007-06-14  Dirk Mueller  <dmueller@suse.de>

       PR c++/31806
       * g++.dg/opt/static6.C: New testcase.

Added:
    trunk/gcc/testsuite/g++.dg/opt/static6.C
Modified:
    trunk/gcc/testsuite/ChangeLog

------- Comment #11 From Dirk Mueller 2007-06-20 16:27 -------
Subject: Bug 31806

Author: mueller
Date: Wed Jun 20 16:27:23 2007
New Revision: 125887

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=125887
Log:
2007-06-20  Dirk Mueller  <dmueller@suse.de>

       PR c++/31809
       PR c++/31806
       Backport from mainline:
       2007-05-31  Jakub Jelinek  <jakub@redhat.com>

       * decl.c (cp_finish_decl): Also clear was_readonly if a static var
       needs runtime initialization.

       2007-05-30  Jakub Jelinek  <jakub@redhat.com>

       * decl.c (cp_finish_decl): Clear TREE_READONLY flag on TREE_STATIC
       variables that need runtime initialization.

       * g++.dg/opt/static5.C: New testcase
       * g++.dg/opt/static6.C: Likewise


Added:
    branches/gcc-4_1-branch/gcc/testsuite/g++.dg/opt/static5.C
    branches/gcc-4_1-branch/gcc/testsuite/g++.dg/opt/static6.C
Modified:
    branches/gcc-4_1-branch/gcc/cp/ChangeLog
    branches/gcc-4_1-branch/gcc/cp/decl.c
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog

------- Comment #12 From Dirk Mueller 2007-06-20 16:28 -------
Fixed

Bug List: (This bug is not in your last search results)   Show last search results      Search page      Enter new bug