[PR middle-end/78548] fix latent double free in tree-ssa-uninit.c

[Richard Biener]

On Thu, Dec 1, 2016 at 12:03 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
> This looks like a latent problem in the -Wmaybe-uninitialized code unrelated
> to my work.
>
> The problem here is a sequence of simplify_preds() followed by
> normalize_preds() that I added, but is based on prior art all over the file:
>
> +      simplify_preds (&uninit_preds, NULL, false);
> +      uninit_preds = normalize_preds (uninit_preds, NULL, false);
>
> The problem is that in this particular testcase we trigger simplify_preds_4
> which makes a copy of a chain, frees the chain, and then tries to use the
> chain later (in normalize_preds).  The normalize_preds() function tries to
> free again the chain and we blow up:
>
> This is the main problem in simplify_preds_4:
>
>   /* Now clean up the chain.  */
>   if (simplified)
>     {
>       for (i = 0; i < n; i++)
>         {
>           if ((*preds)[i].is_empty ())
>             continue;
>           s_preds.safe_push ((*preds)[i]);
> // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> // Makes a copy of the pred_chain.
>         }
>
>       destroy_predicate_vecs (preds);
> // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> // free() all the pred_chain's.
>
>       (*preds) = s_preds;
> // ^^^^^^^^^^^^^^^^^^^^^^
> // Wait a minute, we still keep a copy of the pred_chains.
>       s_preds = vNULL;
>     }
>
> I have no idea how this worked even before my patch.  Perhaps we never had a
> simplify_preds() followed by a normalize_preds() where the simplification
> involved a call to simplify_preds_4.
>
> Interestingly enough, simplify_preds_2() has the exact same code, but with
> the fix I am proposing. So this seems like an oversight.  Also, the fact
> that the simplification in simplify_preds_2 is more common than the
> simplification performed in simplify_preds_4 would suggest that
> simplify_preds_4 was uncommon enough and probably wasn't been used in a
> simplify_preds/normalize_preds combo.
>
> Anyways... I've made some other cleanups to the code, but the main gist of
> the entire patch is:
>
> -      destroy_predicate_vecs (preds);
> +      preds->release ();
>
> That is, release preds, but don't free the associated memory with the
> pred_chain's therein.
>
> This patch is on top of my pending patch here:
>
>         https://gcc.gnu.org/ml/gcc-patches/2016-11/msg02900.html
>
> Tested on x86-64 Linux.
>
> OK for trunk?

Ok.

Richard.

> Aldy