[patch][version 4]add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc

[Martin Jambor]

Hi,

On Wed, Jul 07 2021, Qing Zhao via Gcc-patches wrote:
> Hi, 
>
> This is the 4th version of the patch for the new security feature for GCC.

I have been following the threads about this feature only very lightly,
so please accept my apologies if my comments are about something which
has been already discussed, but...

[...]

> diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
> index c05d22f3e8f1..35051d7c6b96 100644
> --- a/gcc/tree-sra.c
> +++ b/gcc/tree-sra.c
> @@ -384,6 +384,13 @@ static struct
>  
>    /* Numbber of components created when splitting aggregate parameters.  */
>    int param_reductions_created;
> +
> +  /* Number of deferred_init calls that are modified.  */
> +  int deferred_init;
> +
> +  /* Number of deferred_init calls that are created by
> +     generate_subtree_deferred_init.  */
> +  int subtree_deferred_init;
>  } sra_stats;
>  
>  static void
> @@ -4096,6 +4103,110 @@ get_repl_default_def_ssa_name (struct access *racc, tree reg_type)
>    return get_or_create_ssa_default_def (cfun, racc->replacement_decl);
>  }
>  
> +
> +/* Generate statements to call .DEFERRED_INIT to initialize scalar replacements
> +   of accesses within a subtree ACCESS; all its children, siblings and their
> +   children are to be processed.
> +   GSI is a statement iterator used to place the new statements.  */
> +static void
> +generate_subtree_deferred_init (struct access *access,
> +				tree init_type,
> +				tree is_vla,
> +				gimple_stmt_iterator *gsi,
> +				location_t loc)
> +{
> +  do
> +    {
> +      if (access->grp_to_be_replaced)
> +	{
> +	  tree repl = get_access_replacement (access);
> +	  gimple *call
> +	    = gimple_build_call_internal (IFN_DEFERRED_INIT, 3,
> +					  TYPE_SIZE_UNIT (TREE_TYPE (repl)),
> +					  init_type, is_vla);
> +	  gimple_call_set_lhs (call, repl);
> +	  gsi_insert_before (gsi, call, GSI_SAME_STMT);
> +	  update_stmt (call);
> +	  gimple_set_location (call, loc);
> +	  sra_stats.subtree_deferred_init++;
> +	}
> +      else if (access->grp_to_be_debug_replaced)
> +	{
> +	  tree drepl = get_access_replacement (access);
> +	  tree call = build_call_expr_internal_loc
> +		     (UNKNOWN_LOCATION, IFN_DEFERRED_INIT,
> +		      TREE_TYPE (drepl), 3,
> +		      TYPE_SIZE_UNIT (TREE_TYPE (drepl)),
> +		      init_type, is_vla);
> +	  gdebug *ds = gimple_build_debug_bind (drepl, call,
> +						gsi_stmt (*gsi));
> +	  gsi_insert_before (gsi, ds, GSI_SAME_STMT);

Is handling of grp_to_be_debug_replaced accesses necessary here?  If so,
why?  grp_to_be_debug_replaced accesses are there only to facilitate
debug information about a part of an aggregate decl is that is likely
going to be entirely removed - so that debuggers can sometimes show to
users information about what they would contain had they not removed.
It seems strange you need to mark them as uninitialized because they
should not have any consumers.  (But perhaps it is also harmless.)

On a related note, if the intent of the feature is for optimizers to
behave (almost?) as if it was not taking place, I believe you need to
handle specially, and probably just ignore, calls to IFN_DEFERRED_INIT
in scan_function in tree-sra.c.  Otherwise the generated SRA access
structures will have extra write flags turned on in them and that will
lead to different behavior of the pass.

Martin



> +	}
> +      if (access->first_child)
> +	generate_subtree_deferred_init (access->first_child, init_type,
> +					is_vla, gsi, loc);
> +
> +      access = access ->next_sibling;
> +    }
> +  while (access);
> +}
> +
> +/* For a call to .DEFERRED_INIT:
> +   var = .DEFERRED_INIT (size_of_var, init_type, is_vla);
> +   examine the LHS variable VAR and replace it with a scalar replacement if
> +   there is one, also replace the RHS call to a call to .DEFERRED_INIT of
> +   the corresponding scalar relacement variable.  Examine the subtree and
> +   do the scalar replacements in the subtree too.  STMT is the call, GSI is
> +   the statment iterator to place newly created statement.  */
> +
> +static enum assignment_mod_result
> +sra_modify_deferred_init (gimple *stmt, gimple_stmt_iterator *gsi)
> +{
> +  tree lhs = gimple_call_lhs (stmt);
> +  tree init_type = gimple_call_arg (stmt, 1);
> +  tree is_vla = gimple_call_arg (stmt, 2);
> +
> +  struct access *lhs_access = get_access_for_expr (lhs);
> +  if (!lhs_access)
> +    return SRA_AM_NONE;
> +
> +  location_t loc = gimple_location (stmt);
> +
> +  if (lhs_access->grp_to_be_replaced)
> +    {
> +      tree lhs_repl = get_access_replacement (lhs_access);
> +      gimple_call_set_lhs (stmt, lhs_repl);
> +      tree arg0_repl = TYPE_SIZE_UNIT (TREE_TYPE (lhs_repl));
> +      gimple_call_set_arg (stmt, 0, arg0_repl);
> +      sra_stats.deferred_init++;
> +    }
> +  else if (lhs_access->grp_to_be_debug_replaced)
> +    {
> +      tree lhs_drepl = get_access_replacement (lhs_access);
> +      tree call = build_call_expr_internal_loc
> +		  (UNKNOWN_LOCATION, IFN_DEFERRED_INIT,
> +		  TREE_TYPE (lhs_drepl), 3,
> +		  TYPE_SIZE_UNIT (TREE_TYPE (lhs_drepl)),
> +		  init_type, is_vla);
> +      gdebug *ds = gimple_build_debug_bind (lhs_drepl, call,
> +					    gsi_stmt (*gsi));
> +      gsi_insert_before (gsi, ds, GSI_SAME_STMT);
> +    }
> +
> +  if (lhs_access->first_child)
> +    generate_subtree_deferred_init (lhs_access->first_child,
> +				    init_type, is_vla, gsi, loc);
> +  if (lhs_access->grp_covered)
> +    {
> +      unlink_stmt_vdef (stmt);
> +      gsi_remove (gsi, true);
> +      release_defs (stmt);
> +      return SRA_AM_REMOVED;
> +    }
> +
> +  return SRA_AM_MODIFIED;
> +}
> +
>  /* Examine both sides of the assignment statement pointed to by STMT, replace
>     them with a scalare replacement if there is one and generate copying of
>     replacements if scalarized aggregates have been used in the assignment.  GSI
> @@ -4460,17 +4571,27 @@ sra_modify_function_body (void)
>  	      break;
>  
>  	    case GIMPLE_CALL:
> -	      /* Operands must be processed before the lhs.  */
> -	      for (i = 0; i < gimple_call_num_args (stmt); i++)
> +	      /* Handle calls to .DEFERRED_INIT specially.  */
> +	      if (gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
>  		{
> -		  t = gimple_call_arg_ptr (stmt, i);
> -		  modified |= sra_modify_expr (t, &gsi, false);
> +		  assign_result = sra_modify_deferred_init (stmt, &gsi);
> +		  modified |= assign_result == SRA_AM_MODIFIED;
> +		  deleted = assign_result == SRA_AM_REMOVED;
>  		}
> -
> -	      if (gimple_call_lhs (stmt))
> +	      else
>  		{
> -		  t = gimple_call_lhs_ptr (stmt);
> -		  modified |= sra_modify_expr (t, &gsi, true);
> +		  /* Operands must be processed before the lhs.  */
> +		  for (i = 0; i < gimple_call_num_args (stmt); i++)
> +		    {
> +		      t = gimple_call_arg_ptr (stmt, i);
> +		      modified |= sra_modify_expr (t, &gsi, false);
> +		    }
> +
> +	      	  if (gimple_call_lhs (stmt))
> +		    {
> +		      t = gimple_call_lhs_ptr (stmt);
> +		      modified |= sra_modify_expr (t, &gsi, true);
> +		    }
>  		}
>  	      break;
>