public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Fix a tcb crash and a potential bug on mainline
@ 2004-10-18 21:09 Steven Bosscher
  2004-10-18 22:06 ` Andrew MacLeod
                   ` (2 more replies)
  0 siblings, 3 replies; 28+ messages in thread
From: Steven Bosscher @ 2004-10-18 21:09 UTC (permalink / raw)
  To: gcc-patches; +Cc: dnovillo, amacleod

Hi,


So here is an interesting tree CFG cleanup bug seen on the t-c-branch,
but it may well be that it also exists on mainline (haven't tried it).

Consider this function,

extern void abort (void) __attribute__ ((__noreturn__));
extern void foo (const char *s);

int
bar (const char **mangled)
{
  const char *c;
  int type_quals;

  switch (**mangled)
    {
    case 'C':
      type_quals = 1;
      break;
    case 'u':
      type_quals = 4;
      break;
    default:
      abort ();
    }

  switch (type_quals)
    {
    case 6:
      c = "volatile __restrict";
      break;
    case 4:
      c = "const volatile __restrict";
      break;
    default:
      abort ();
    }
  if (c != ((void *)0))
    foo (c);
}

After some constant propagating and jump threading, we eventually find
out that only the path with "case 4" matters and we end up with the two
following basic blocks:

;; basic block 2, loop depth 0, count 0
;; prev block 1, next block 3
;; pred:       0 [33.3%]  (exec)
;; succ:       3 [33.3%]  4 [33.3%]  7 [33.3%]  (exec)
# type_quals_8 = PHI <4(0)>;
<L3>:;
switch (4)
  {
    case 4: goto <L11>;
    case 6: goto <L7>;
    default : goto <L6>;
  }

;; basic block 4, loop depth 0, count 0
;; prev block 3, next block 5
;; pred:       2 [33.3%]
;; succ:       6 [10.0%]  (false) 5 [90.0%]  (true)
# c_13 = PHI <&"volatile __restrict"[0](2)>;
<L7>:;
if (c_13 != 0B) goto <L8>; else goto <L9>;

Then we try to do cleanup_tree_cfg() which calls cleanup_control_flow()
which iterates over all basic blocks with FOR_EACH_BB.
When we find that we'll always go to <L11> from block 2, we replace the
switch with an unconditional jump and block 4 becomes unreachable. We
remove the edge from block 2 to block 4, and we remove the matching PHI
arguments with remove_phi_arg_num().

Now it gets interesting.  remove_phi_arg_num() removes the PHI defining
c_13, and then releases the PHI_RESULT, so c_13 ends up on the SSA free
list, even though it is still used in the COND_EXPR in block 4.

Later on cleanup_control_flow() tries to do its thing on basic block 4.
It tries to fold "(c_13 != 0B)", but c_13 is on the free list, so it
does not have a TREE_TYPE, so fold crashes.

I think the root cause of this bug is that c_13 is put on the free list,
and I think its SSA_NAME_DEF_STMT should be set to the empty statement,
because remove_phi_arg_num() cannot know if there are any immediate
uses of it still left.  I don't know if making that change could have
some unforseen effects, so I decided to re-hide the the bug by just not
looking at unreachable blocks in cleanup_control_flow().  I've tried to
convince myself that this can not happen in other situations, but I'm
not sure.  This may be something Andrew may want to look into as part
of his immediate uses work??

This fix is completely untested at the moment, but it's quite obvious
and it fixes the ICE I was seeing.  Still, I am a bit surprised that we
have not been in trouble over this bug before - perhaps we should still
fix the remove_phi_arg_num() issue instead of papering it over this way?

Gr.
Steven


Index: tree-cfg.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-cfg.c,v
retrieving revision 2.55.2.5
diff -c -3 -p -r2.55.2.5 tree-cfg.c
*** tree-cfg.c  13 Oct 2004 16:04:10 -0000      2.55.2.5
--- tree-cfg.c  18 Oct 2004 21:00:37 -0000
*************** cleanup_control_flow (void)
*** 1853,1858 ****
--- 1852,1863 ----
  
    FOR_EACH_BB (bb)
      {
+       /* We can have unreachable blocks here if we run before
+        delete_unreachable_blocks or if a block visited before
+        this one had an edge to bb that we removed.  */
+       if (EDGE_COUNT (bb->preds) == 0)
+       continue;
+
        bsi = bsi_last (bb);
  
        if (bsi_end_p (bsi))

^ permalink raw reply	[flat|nested] 28+ messages in thread
end of thread, other threads:[~2004-10-27 16:22 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-10-18 21:09 Fix a tcb crash and a potential bug on mainline Steven Bosscher
2004-10-18 22:06 ` Andrew MacLeod
2004-10-18 22:18 ` Andrew Pinski
2004-10-19 20:58 ` Jeffrey A Law
2004-10-19 21:29   ` Steven Bosscher
2004-10-19 22:08     ` Jeffrey A Law
2004-10-19 22:27       ` Daniel Berlin
2004-10-19 22:35         ` Jeffrey A Law
2004-10-19 23:08           ` Daniel Berlin
2004-10-19 23:14             ` Daniel Berlin
2004-10-21  2:33               ` Jeffrey A Law
2004-10-19 22:36       ` Steven Bosscher
2004-10-19 22:41         ` Diego Novillo
2004-10-19 23:16         ` Jeffrey A Law
2004-10-20 13:17           ` Andrew MacLeod
2004-10-20 13:32             ` Steven Bosscher
2004-10-20 13:47               ` Andrew MacLeod
2004-10-20 14:30                 ` Steven Bosscher
2004-10-20 16:53                   ` Andrew MacLeod
2004-10-20 17:33                     ` Daniel Berlin
2004-10-22 19:15               ` Jeffrey A Law
2004-10-22 19:33                 ` Steven Bosscher
2004-10-22 19:37                   ` Jeffrey A Law
2004-10-22 19:40                     ` Steven Bosscher
2004-10-22 21:08                       ` Jeffrey A Law
2004-10-24 21:20                         ` Steven Bosscher
2004-10-27 16:27                           ` Jeffrey A Law
2004-10-22 19:10             ` Jeffrey A Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).