This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fix a tcb crash and a potential bug on mainline


On Tue, 2004-10-19 at 15:28, Steven Bosscher wrote:
> On Tuesday 19 October 2004 22:48, Jeffrey A Law wrote:
> > On Mon, 2004-10-18 at 15:03, Steven Bosscher wrote:
> > > 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.
> >
> > Hmm.  I wonder if this is the same bug Andrew Pinski was trying to
> > fix by not clearing TREE_TYPE.  Your description matches a lot of
> > the tidbits Andrew mentioned.
> 
> Yes, it is.
> 
> 
> > > 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.
> >
> > ???  Putting c_13 on the free list is the right thing to do.
> 
> I believe it is not.  It is *wrong* to put an SSA_NAME on the free list
> unless you're absolutely positively sure that the thing is not used
> anywhere in the code.
Have you read the code to manage the free list?  This is normal and
common precisely because we do not have immediate use information
to allow us to find and remove all references.

> >  I'm not
> > sure why you think setting SSA_NAME_DEF_STMT to NULL is particularly
> > interesting though.  Whether or not the name has any uses left in the
> > IL isn't something we use to determine whether or not to set
> > SSA_NAME_DEF_STMT to any particular value IIRC.
> 
> Not usually.  But this case is special.  Normally we don't just throw
> away a statement, we first update the code or we delete entire blocks
> of code.  In this particular case we're only deleting a PHI node and
> we don't know if the block we're deleting the PHI from is unreachable
> (we could, but it's not the business of remove_phi_arg_num() to remove
> unreachable blocks).  So conceptually the PHI_RESULT of the deleted
> PHI becomes undefined.  But it is not dead until all its uses are also
> removed.  We express "undefined-ness" by setting the SSA_NAME_DEF_STMT
> to the empty statement.
Err, if we remove the last argument to a PHI node then by definition
the block containing the PHI is unreachable as it has no incoming
edges.

There is no such thing as an "undefined" PHI node.


> 
> > >   This may be something Andrew may want to look into as part
> > > of his immediate uses work??
> >
> > It's something we've pondered and talked about a little.  Basically
> > with the immediate use work we would know where all the uses are
> > and we could probably arrange to delete those uses.
> 
> How would you delete those uses?  You don't know if those statements
> are dead, so you really can't do that.
If they are using the result of a deleted PHI or some other deleted
statement, then those statements must also be dead.  It's one of
the nice SSA properties.


> What I mean with this is that perhaps we should not put the SSA_NAME on
> the free list if there still are immediate uses.  On Andrew's branch
> that is very easy to do, if we decide to fix the bug this way.
No, you would delete all the immediate uses.

> I mean to fix it by making it set the SSA_NAME_DEF_STMT of PHIs that
> it is deleting to the empty statement...
No.  That is wrong.

Jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]