Hi, Kees,
Just noticed that you didn’t add -fauto-var-init-approach=D to the command line.
[qinzhao@localhost uninit]$ cat t8.c
a() { char b[1]; }
[qinzhao@localhost uninit]$ sh t
/home/qinzhao/Install/latest/bin/gcc -ftrivial-auto-var-init=pattern
-fauto-var-init-approach=D t8.c -S
t8.c:1:1: warning: return type defaults to ‘int’ [-Wimplicit-int]
1 | a() { char b[1]; }
| ^
Without -fauto-var-init-approach=D, I have the same error as yours.
(This option is just temporary, its purpose is to compare two different
implementations for “zero” initialization,
Since “pattern” initialization is added later after the comparison, “pattern”
initialization does not support the
Default “A” approach. I plan to make the default as “D” in the final version of
the patch).
So, please add “-fauto-var-init-approach=D” along with
“-ftrivial-auto-var-init=pattern/zero” for the testing.
Sorry for the confusion.
> On Feb 25, 2021, at 2:00 PM, Kees Cook <keesc...@chromium.org> wrote:
>
> On Thu, Feb 25, 2021 at 12:15:01PM -0600, Qing Zhao wrote:
>>> On Feb 24, 2021, at 10:41 PM, Kees Cook <keesc...@chromium.org> wrote:
>>> [...]
>>> test_stackinit: trailing_hole_none ok
>>> test_stackinit: packed_none ok
>>> test_stackinit: user ok
>>> test_stackinit: failures: 8
>>
>> Does the above testing include “pattern initialization” in addition to “zero
>> initialization”?
>
> This was from the zero-init case. I've just tested pattern-init now and
> it actually crashes GCC. I minimized the test case to this:
>
> $ cat main.i
> a() { char b[1]; }
> $ gcc -ftrivial-auto-var-init=pattern -c /dev/null main.i
> main.i:1:1: warning: return type defaults to ‘int’ [-Wimplicit-int]
> 1 | a() { char b[1]; }
> | ^
> main.i: In function ‘a’:
> main.i:1:12: internal compiler error: in gimplify_init_ctor_eval, at
> gimplify.c:4873
> 1 | a() { char b[1]; }
> | ^
> 0x69740d gimplify_init_ctor_eval
> ../../../gcc/gcc/gimplify.c:4873
> 0xb5ac8f gimplify_init_constructor
> ../../../gcc/gcc/gimplify.c:5320
> 0xb6b68a gimplify_modify_expr
> ../../../gcc/gcc/gimplify.c:5952
> 0xb533ba gimplify_expr(tree_node**, gimple**, gimple**, bool (*)(tree_node*),
> int)
> ../../../gcc/gcc/gimplify.c:14262
> 0xb56b26 gimplify_stmt(tree_node**, gimple**)
> ../../../gcc/gcc/gimplify.c:7056
> 0xb68e6e gimplify_and_add(tree_node*, gimple**)
> ../../../gcc/gcc/gimplify.c:489
> 0xb68e6e gimple_add_init_for_auto_var
> ../../../gcc/gcc/gimplify.c:1892
> 0xb68e6e gimplify_decl_expr
> ../../../gcc/gcc/gimplify.c:2010
> 0xb53bd6 gimplify_expr(tree_node**, gimple**, gimple**, bool (*)(tree_node*),
> int)
> ../../../gcc/gcc/gimplify.c:14459
> 0xb56b26 gimplify_stmt(tree_node**, gimple**)
> ../../../gcc/gcc/gimplify.c:7056
> 0xb5727d gimplify_bind_expr
> ../../../gcc/gcc/gimplify.c:1421
> 0xb536f0 gimplify_expr(tree_node**, gimple**, gimple**, bool (*)(tree_node*),
> int)
> ../../../gcc/gcc/gimplify.c:14463
> 0xb6ccc9 gimplify_stmt(tree_node**, gimple**)
> ../../../gcc/gcc/gimplify.c:7056
> 0xb6ccc9 gimplify_body(tree_node*, bool)
> ../../../gcc/gcc/gimplify.c:15498
> 0xb6d0ed gimplify_function_tree(tree_node*)
> ../../../gcc/gcc/gimplify.c:15652
> 0x9ae8d7 cgraph_node::analyze()
> ../../../gcc/gcc/cgraphunit.c:670
> 0x9b13a7 analyze_functions
> ../../../gcc/gcc/cgraphunit.c:1233
> 0x9b1f9d symbol_table::finalize_compilation_unit()
> ../../../gcc/gcc/cgraphunit.c:2511
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <https://gcc.gnu.org/bugs/> for instructions.
>
>
>>> struct test_..._hole instance = { .two = ..., };
>>>
>>> or
>>>
>>> struct test_..._hole instance = { .one = ...,
>>> .two = ...,
>>> .three = ...,
>>> .four = ...,
>>>
>>> };
>>
>> So, when the structure variables are not statically initialized, all the
>> paddings are initialized correctly by the compiler?
>
> For the zero case, yes. (Usually such happen via copies into stack from from
> .rodata
> sections, or accidentally from already-initialized copies.)
>
>> In the current implementation, when the auto variable is explicitly
>> initialized, compiler will do nothing.
>> Looks like for structure variables we need some special handling to
>> initialize the paddings.
>> Need to study this a little bit and see how to fix it.
>
> Deterministic padding init is a big part of this defense since that's
> where the bulk of the really hard-to-find problems have existed (i.e. in
> padding holes in structures that got copied around).
Okay, I will try to fix this.
Clang doesn’t have this issue, right?
>
>>> The last case is for switch variables outside of case statements, like
>>> "var" here:
>>>
>>> switch (path) {
>>> unsigned long var;
>>>
>>> case ..:
>>> ...
>>> case ..:
>>> ...
>>> ...
>>> }
>>>
>>
>> Will study this case more.
>
> For the kernel, this is a low priority, since I purged the code of all
> variable declarations outside of an execution path[1]. For reference,
> here's the Clang bug for the same problem[2] (though latest Clang git
> appears to no longer exhibit this issue, so they may have fixed it).
Okay, thanks for your info.
Qing
>
> -Kees
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?qt=grep&q=Distribute+switch+variables+for+initialization
> [2] https://bugs.llvm.org/show_bug.cgi?id=44916
>
> --
> Kees Cook
