Re: [RFC PATCH] Fix crash in linearize_compound_statement()

["Christopher Li" <sparse@chrisli.org>]

On Mon, Apr 7, 2008 at 10:14 PM, Pavel Roskin <proski@gnu.org> wrote:
>
>  I should have been more clear.  Last time I was carried away by some
>  strange messages about "return" and labels, which seemed really weird to
>  me, but less so after your explanation.
>
>  This example is made from the same original source, but this time I
>  concentrated on the crash, because that's the thing that cannot be
>  justified by any bad code.

I understand that you have your source code which sparse choked on
fixed. Good.

I am actually complaining your fix to sparse itself. It is not good enough.
You fix it in the linearize stage. That is already too late.

You should fix it in the parse and type evaluate stage, before it even reach
to the linearizer.

>
>
>  > I agree sparse should not assert on it, but not like this.
>
>  I don't quite understand why sparse is getting in this situation, but

Exactly. That is because you try to linearize the exact same function
twice due to the "typeof(bar)". Linearize the same function twice is just
plain wrong. It should never happen. Your patch just covers up the real problem
instead of fixing it.

>  That's fixed, of course.  And many other problems are fixed too, some of
>  which are user visible.  I really appreciate the usefulness of sparse.

I mean the fix in the sparse source code, not the source code which sparse
is checking. You should change sparse, when it do "typeof(function) varname",
give it a warning and skip this "varname" node all together. Later in the
linearize stage, it will not linearize the same function twice.

What your patch does is actually allowing sparse linearized a function twice.
That is going to mess up the user-define chain very badly (e.g. the PHI node
usage problem you are seeing.)

Chris