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

[Kees Cook]

On Wed, Jul 14, 2021 at 02:09:50PM +0000, Qing Zhao wrote:
> Hi, Richard,
> 
> > On Jul 14, 2021, at 2:14 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> > 
> > On Wed, Jul 14, 2021 at 1:17 AM Qing Zhao <qing.zhao@oracle.com> wrote:
> >> 
> >> Hi, Kees,
> >> 
> >> I took a look at the kernel testing case you attached in the previous email, and found the testing failed with the following case:
> >> 
> >> #define INIT_STRUCT_static_all          = { .one = arg->one,            \
> >>                                            .two = arg->two,            \
> >>                                            .three = arg->three,        \
> >>                                            .four = arg->four,          \
> >>                                        }
> >> 
> >> i.e, when the structure type auto variable has been explicitly initialized in the source code.  -ftrivial-auto-var-init in the 4th version
> >> does not initialize the paddings for such variables.
> >> 
> >> But in the previous version of the patches ( 2 or 3), -ftrivial-auto-var-init initializes the paddings for such variables.
> >> 
> >> I intended to remove this part of the code from the 4th version of the patch since the implementation for initializing such paddings is completely different from
> >> the initializing of the whole structure as a whole with memset in this version of the implementation.
> >> 
> >> If we really need this functionality, I will add another separate patch for this additional functionality, but not with this patch.
> >> 
> >> Richard, what’s your comment and suggestions on this?
> > 
> > I think this can be addressed in the gimplifier by adjusting
> > gimplify_init_constructor to clear
> > the object before the initialization (if it's not done via aggregate
> > copying).  
> 
> I did this in the previous versions of the patch like the following:
> 
> @@ -5001,6 +5185,17 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
>  	  /* If a single access to the target must be ensured and all elements
>  	     are zero, then it's optimal to clear whatever their number.  */
>  	  cleared = true;
> +	else if (flag_trivial_auto_var_init > AUTO_INIT_UNINITIALIZED
> +		 && !TREE_STATIC (object)
> +		 && type_has_padding (type))
> +	  /* If the user requests to initialize automatic variables with
> +	     paddings inside the type, we should initialize the paddings too.
> +	     C guarantees that brace-init with fewer initializers than members
> +	     aggregate will initialize the rest of the aggregate as-if it were
> +	     static initialization.  In turn static initialization guarantees
> +	     that pad is initialized to zero bits.
> +	     So, it's better to clear the whole record under such situation.  */
> +	  cleared = true;
>  	else
>  	  cleared = false;
> 
> Then the paddings are also initialized to zeroes with this option. (Even for -ftrivial-auto-var-init=pattern).

Thanks! I've tested with the attached patch to v4 and it passes all my
tests again.

> Is the above change Okay? (With this change, when -ftrivial-auto-var-init=pattern, the paddings for the
> structure variables that have explicit initializer will be ZEROed, not 0xFE)

Padding zeroing in the face of pattern-init is correct (and matches what
Clang does).

-Kees

-- 
Kees Cook
-------------- next part --------------
A non-text attachment was scrubbed...
Name: padding.patch
Type: text/x-diff
Size: 4165 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20210714/59f74a98/attachment.bin>