[PATCH] Fix ICE with __builtin_stack_save (PR c/90898)

[Richard Biener]

On Wed, 20 Nov 2019, Jakub Jelinek wrote:

> Hi!
> 
> I agree that we shouldn't have made __builtin_stack_{save,restore} public,
> but I'd fear it is too late to remove it now (and replace by internal fn),
> I'd think some code might use it to control when e.g. alloca will be
> released.  As the comment on insert_clobber* says, the addition of the
> clobbers is a best effort, we could end up not finding the right spot and
> not adding the CLOBBER in there even without user __builtin_* calls, so
> this patch just removes an unnecessary assertion and just will not find
> __builtin_stack_restore if something weird is seen, such as in the testcase
> storing of the __builtin_stack_save result into memory, or could be
> a cast or whatever else too.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2019-11-19  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c/90898
> 	* tree-ssa-ccp.c (insert_clobber_before_stack_restore): Remove
> 	assertion.
> 	(insert_clobbers_for_var): Fix a typo in function comment.
> 
> 	* gcc.dg/pr90898.c: New test.
> 
> --- gcc/tree-ssa-ccp.c.jj	2019-11-13 10:54:47.093020613 +0100
> +++ gcc/tree-ssa-ccp.c	2019-11-19 19:13:35.332705130 +0100
> @@ -2114,8 +2114,6 @@ insert_clobber_before_stack_restore (tre
>      else if (gimple_assign_ssa_name_copy_p (stmt))
>        insert_clobber_before_stack_restore (gimple_assign_lhs (stmt), var,
>  					   visited);
> -    else
> -      gcc_assert (is_gimple_debug (stmt));
>  }
>  
>  /* Advance the iterator to the previous non-debug gimple statement in the same
> @@ -2140,9 +2138,9 @@ gsi_prev_dom_bb_nondebug (gimple_stmt_it
>  /* Find a BUILT_IN_STACK_SAVE dominating gsi_stmt (I), and insert
>     a clobber of VAR before each matching BUILT_IN_STACK_RESTORE.
>  
> -   It is possible that BUILT_IN_STACK_SAVE cannot be find in a dominator when a
> -   previous pass (such as DOM) duplicated it along multiple paths to a BB.  In
> -   that case the function gives up without inserting the clobbers.  */
> +   It is possible that BUILT_IN_STACK_SAVE cannot be found in a dominator when
> +   a previous pass (such as DOM) duplicated it along multiple paths to a BB.
> +   In that case the function gives up without inserting the clobbers.  */
>  
>  static void
>  insert_clobbers_for_var (gimple_stmt_iterator i, tree var)
> --- gcc/testsuite/gcc.dg/pr90898.c.jj	2019-11-19 19:18:02.277712801 +0100
> +++ gcc/testsuite/gcc.dg/pr90898.c	2019-11-19 19:18:52.613959787 +0100
> @@ -0,0 +1,16 @@
> +/* PR c/90898 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +void *p;
> +int bar (void);
> +void baz (int *);
> +
> +void
> +foo (void)
> +{
> +  p = __builtin_stack_save ();
> +  int a[(bar (), 2)];
> +  baz (a);
> +  __builtin_stack_restore (p);
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)