Bug 50741 - [4.7 Regression] remove_unused_locals causes seg fault
Summary: [4.7 Regression] remove_unused_locals causes seg fault
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.7.0
: P1 normal
Target Milestone: 4.7.0
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-16 10:15 UTC by David Binderman
Modified: 2011-11-17 16:08 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2011-10-17 00:00:00


Attachments
C++ source code (26.23 KB, text/x-c++src)
2011-10-16 10:15 UTC, David Binderman
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Binderman 2011-10-16 10:15:53 UTC
Created attachment 25512 [details]
C++ source code

I just tried to compile the package fusecompress_offline1-1.99.19-7
on latest trunk snapshot 20111015 on an AMD x86_64 box.

The compiler said

CompressedMagic.cpp: In constructor 'CompressedMagic::CompressedMagic()':
CompressedMagic.cpp:106:1: internal compiler error: Segmentation fault
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.

valgrind said

==8702== Invalid read of size 1
==8702==    at 0xB0B2B6: mark_all_vars_used_1(tree_node**, int*, void*) (tree-fl
ow-inline.h:562)
==8702==    by 0xC1098D: walk_tree_1(tree_node**, tree_node* (*)(tree_node**, in
t*, void*), void*, pointer_set_t*, tree_node* (*)(tree_node**, int*, tree_node*
(*)(tree_node**, int*, void*), void*, pointer_set_t*)) (tree.c:10455)
==8702==    by 0xC10F7A: walk_tree_1(tree_node**, tree_node* (*)(tree_node**, in
t*, void*), void*, pointer_set_t*, tree_node* (*)(tree_node**, int*, tree_node*
(*)(tree_node**, int*, void*), void*, pointer_set_t*)) (tree.c:10533)
==8702==    by 0xB0C049: remove_unused_locals() (tree-ssa-live.c:595)
==8702==    by 0x99165C: execute_function_todo(void*) (passes.c:1695)
==8702==    by 0x991E7C: execute_todo(unsigned int) (passes.c:1741)
==8702==    by 0x994FC9: execute_one_pass(opt_pass*) (passes.c:2087)
==8702==    by 0x9952C4: execute_pass_list(opt_pass*) (passes.c:2119)
==8702==    by 0x9944F3: do_per_function_toporder(void (*)(void*), void*) (passe
s.c:1606)
==8702==    by 0x995745: execute_ipa_pass_list(opt_pass*) (passes.c:2437)
==8702==    by 0x78A362: cgraph_optimize() (cgraphunit.c:2011)
==8702==    by 0x78A8D9: cgraph_finalize_compilation_unit() (cgraphunit.c:1312)
==8702==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==8702==

Preprocessed source code attached. Flag -O2 required.
Comment 1 Paolo Carlini 2011-10-16 15:04:32 UTC
Tree-optimization?
Comment 2 Richard Biener 2011-10-17 07:17:07 UTC
Probably a dup.
Comment 3 Richard Biener 2011-10-17 07:21:16 UTC
Confirmed.

(gdb) call debug_tree (t)
 <var_decl 0x101cde780 __FUNCTION__
    type <array_type 0x1424de5e8
        type <integer_type 0x14232cc78 char readonly sizes-gimplified string-flag type_6 QI
            size <integer_cst 0x14230db80 constant 8>
            unit size <integer_cst 0x14230dba0 constant 1>
            align 8 symtab 0 alias set -1 canonical type 0x14232cc78 precision 8 min <integer_cst 0x14230db40 -128> max <integer_cst 0x14230dc20 127>
            pointer_to_this <pointer_type 0x14232cd20> reference_to_this <reference_type 0x101bc67e0>>

while walking CONSTRUCTOR elements of DECL_INITIAL of

 <var_decl 0x101cde6e0 _rL_53
...
    addressable used static tree_1 tree_3 decl_5 decl_6 BLK file bug36.cc line 8706 col 80 size <integer_cst 0x14252d820 512> unit size <integer_cst 0x101a362e0 64>
    align 256 context <function_decl 0x101cd4200 CompressedMagic> attributes <tree_list 0x1424f5780> initial <constructor 0x101a38360>>

thus a function-local static.
Comment 4 Michael Matz 2011-10-17 15:18:08 UTC
Reducable to:
% cat x.cc
struct PublishLo
{
  const char *functionName;
  ~PublishLo();
};
struct A { A(); };
A::A()
{
  static PublishLo _rL_53 = {__FUNCTION__};
}

The problem doesn't happen when A::A is instead a normal top-level function,
which leads me to believe that somehow for member functions something forgets
to walk the initializers in add_referenced_var.
Comment 5 Michael Matz 2011-10-17 15:28:47 UTC
And of course, it's the ctor cloning:

DECL_CONTEXT of _rL_53 is <function_decl 0x7ffff53db000 A>,
but current_function_decl is <function_decl 0x7ffff53db200 __base_ctor>.

So it's similar to PR50640, in that the initializers of statics declared
in different functions aren't walked.  That's reasonable assuming that
such initializers are walked when the declaring function is handled (which
is reasonable to expect, as otherwise the initializer couldn't have been
known).  But here the cloning and rewriting gets into our way.
Comment 6 Richard Biener 2011-10-18 09:17:55 UTC
(In reply to comment #5)
> And of course, it's the ctor cloning:
> 
> DECL_CONTEXT of _rL_53 is <function_decl 0x7ffff53db000 A>,
> but current_function_decl is <function_decl 0x7ffff53db200 __base_ctor>.
> 
> So it's similar to PR50640, in that the initializers of statics declared
> in different functions aren't walked.  That's reasonable assuming that
> such initializers are walked when the declaring function is handled (which
> is reasonable to expect, as otherwise the initializer couldn't have been
> known).  But here the cloning and rewriting gets into our way.

So maybe we should, in remove_unused_locals, not walk initializers of
non-locals either (their elements will never be in a BLOCK local to
our function).  Even the decl itself I suppose.  So,

Index: gcc/tree-ssa-live.c
===================================================================
--- gcc/tree-ssa-live.c (revision 180126)
+++ gcc/tree-ssa-live.c (working copy)
@@ -372,7 +372,8 @@ mark_all_vars_used_1 (tree *tp, int *wal
 
   /* Only need to mark VAR_DECLS; parameters and return results are not
      eliminated as unused.  */
-  if (TREE_CODE (t) == VAR_DECL)
+  if (TREE_CODE (t) == VAR_DECL
+      && decl_function_context (t) == current_function_decl)
     {
       if (data != NULL && bitmap_clear_bit ((bitmap) data, DECL_UID (t)))
        mark_all_vars_used (&DECL_INITIAL (t), data);

?  At least I can't see how this would break any of its functionality,
and it fixes the ICE.  Bootstrapping/testing now.
Comment 7 Richard Biener 2011-10-18 11:00:49 UTC
Bootstraps and tests ok.  But I suppose with BLOCK_NONLOCAL_VARS we could
refer to an inlined functions static vars, and their initializer could refer
to static vars from the inlined function that are otherwise unused.

int foo()
{
  static volatile int i;
  static volatile int * volatile p = &i;
  return *p;
}

int bar()
{
  return foo();
}

when foo is inlined into bar.  And indeed with the patch we get

{ Scope block 0x7ffff5b3ac30#0

  { Scope block 0x7ffff5b950a0#2 t.c:10 Originating from :  static intD.6 fooD.1604 ();

    { Scope block 0x7ffff5b950f0#3 Originating from : 0x7ffff5b3ab90#0
      static volatile intD.6 i.1606D.1606; (nonlocalized)
      static volatile intD.6 * volatile p.1607D.1607 = &i.1606D.1606; (nonlocalized)

    }

  }

}
bar ()
{
  volatile intD.6 * volatile p.0D.2736;
  intD.6 D.2735;

  # BLOCK 2 freq:10000
  # PRED: ENTRY [100.0%]  (fallthru,exec)
  # VUSE <.MEMD.2734_2(D)>
  # PT = nonlocal escaped
  p.0D.2736_4 ={v} p.1607D.1607;
  # VUSE <.MEMD.2734_2(D)>
  D.2735_5 ={v} *p.0D.2736_4;
  # VUSE <.MEMD.2734_2(D)>
  return D.2735_5;

while without we get

Scope blocks after cleanups:

{ Scope block 0x7ffff5b3ac30#0

  { Scope block 0x7ffff5b950a0#2 t.c:10 Originating from :  static intD.6 fooD.1604 ();

    { Scope block 0x7ffff5b950f0#3 Originating from : 0x7ffff5b3ab90#0
      static volatile intD.6 i.1606D.1606; (nonlocalized)
      static volatile intD.6 * volatile p.1607D.1607 = &i.1606D.1606; (nonlocalized)

    }

  }

bar ()
{
  volatile intD.6 * volatile p.0D.2736;
  static volatile intD.6 iD.1606;
  static volatile intD.6 * volatile p.1607D.1607 = &iD.1606;
  intD.6 D.2735;
  static volatile intD.6 * volatile p.1607D.1607 = &iD.1606;
  static volatile intD.6 iD.1606;

  # BLOCK 2 freq:10000
  # PRED: ENTRY [100.0%]  (fallthru,exec)
  # VUSE <.MEMD.2734_2(D)>
  # PT = nonlocal escaped
  p.0D.2736_4 ={v} p.1607D.1607;
  # VUSE <.MEMD.2734_2(D)>
  D.2735_5 ={v} *p.0D.2736_4;
  # VUSE <.MEMD.2734_2(D)>
  return D.2735_5;
  # SUCC: EXIT [100.0%]

so there is a difference in what local_decls contains.  Not sure if that
is important though.
Comment 8 Michael Matz 2011-11-17 16:04:04 UTC
Author: matz
Date: Thu Nov 17 16:03:56 2011
New Revision: 181443

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181443
Log:
	PR middle-end/50644
	PR middle-end/50741

	* tree-ssa-live.c (mark_all_vars_used_1): Recurse only for decls of
	current function.
	(remove_unused_locals): Ditto.

testsuite/

	* g++.dg/tree-ssa/pr50741.C: New.

Added:
    trunk/gcc/testsuite/g++.dg/tree-ssa/pr50741.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-live.c
Comment 9 Michael Matz 2011-11-17 16:08:38 UTC
Fixed.