Hi, Kees, Thanks a lot for your testing on the linux kernel, I am so happy that this time it works well.
> On Apr 7, 2021, at 5:19 PM, Kees Cook <keesc...@chromium.org> wrote: > > On Wed, Mar 24, 2021 at 04:21:49PM -0500, Qing Zhao wrote: >> This is the 2nd version of the patch for the new security feature for GCC. >> >> Could you please take a look at it and let me know any comments and issues. > > This behaves perfectly as far as I'm able to test in the Linux kernel! > Thank you! > > For comparison to v1, here's the stack init test output for version 2 of the > patch: > > test_stackinit: u8_zero ok > test_stackinit: u16_zero ok > test_stackinit: u32_zero ok > test_stackinit: u64_zero ok > test_stackinit: char_array_zero ok > test_stackinit: small_hole_zero ok > test_stackinit: big_hole_zero ok > test_stackinit: trailing_hole_zero ok > test_stackinit: packed_zero ok > test_stackinit: small_hole_dynamic_partial ok > test_stackinit: big_hole_dynamic_partial ok > test_stackinit: trailing_hole_dynamic_partial ok > test_stackinit: packed_dynamic_partial ok > test_stackinit: small_hole_static_partial ok > test_stackinit: big_hole_static_partial ok > test_stackinit: trailing_hole_static_partial ok > test_stackinit: packed_static_partial ok > test_stackinit: small_hole_static_all ok > test_stackinit: big_hole_static_all ok > test_stackinit: trailing_hole_static_all ok > test_stackinit: packed_static_all ok > test_stackinit: small_hole_dynamic_all ok > test_stackinit: big_hole_dynamic_all ok > test_stackinit: trailing_hole_dynamic_all ok > test_stackinit: packed_dynamic_all ok > test_stackinit: small_hole_runtime_partial ok > test_stackinit: big_hole_runtime_partial ok > test_stackinit: trailing_hole_runtime_partial ok > test_stackinit: packed_runtime_partial ok > test_stackinit: small_hole_runtime_all ok > test_stackinit: big_hole_runtime_all ok > test_stackinit: trailing_hole_runtime_all ok > test_stackinit: packed_runtime_all ok > test_stackinit: u8_none ok > test_stackinit: u16_none ok > test_stackinit: u32_none ok > test_stackinit: u64_none ok > test_stackinit: char_array_none ok > test_stackinit: switch_1_none XFAIL (uninit bytes: 8) > test_stackinit: switch_2_none XFAIL (uninit bytes: 8) > test_stackinit: small_hole_none ok > test_stackinit: big_hole_none ok > test_stackinit: trailing_hole_none ok > test_stackinit: packed_none ok > test_stackinit: user ok > test_stackinit: all tests passed! > > The switch cases are an "expected fail" still, and that's totally fine > for now[1]. Those were all purged from real kernel code anyway. ;) > > So that's a big "Ack" from me. :) > > What are next steps for this patch? My understanding is, it needs to be reviewed and get approved by the global reviewers first, Currently, I am waiting for any comments from global reviewers in order to be able to push this patch into gcc12 as early as possible. > I know a lot of people are looking > forward to it. (And then long-open bug[2] for auto-init can be closed.) > > Thanks again! Thank you! Qing > > -Kees > > [1] Clang doesn't handle this either > https://bugs.llvm.org/show_bug.cgi?id=44916 > <https://bugs.llvm.org/show_bug.cgi?id=44916> > [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87210 > <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87210> > >> >> Thanks. >> >> Qing >> >> ******compared to Version 1, this version added the following new features >> to address Kees’s comments: >> >> 1. correctly handle VLA inside a structure for pattern initialization. >> In tree.c (build_pattern_cst): >> >> + /* if the field is a variable length array, it should be the last >> + field of the record, and no need to initialize. */ >> + if (TREE_CODE (TREE_TYPE (field)) == ARRAY_TYPE >> + && TYPE_SIZE (TREE_TYPE (field)) == NULL_TREE >> + && ((TYPE_DOMAIN (TREE_TYPE (field)) != NULL_TREE >> + && TYPE_MAX_VALUE (TYPE_DOMAIN (TREE_TYPE (field))) >> + == NULL_TREE) >> + || TYPE_DOMAIN (TREE_TYPE (field)) == NULL_TREE)) >> + continue; >> >> 2. initialize all paddings to zero when -ftrivial-auto-var-init is present. >> In expr.c (store_constructor): >> >> Clear the whole structure when >> -ftrivial-auto-var-init and the structure has paddings. >> >> In gimplify.c (gimplify_init_constructor): >> >> Clear the whole structure when >> -ftrivial-auto-var-init and the structure has paddings. >> >> As agreed with Kees, treat the issue related to auto variables outside of >> the cases and inside the switch as a low priority one. >> >> 3. Add the following new testing cases for the above 1 and 2: >> >> * c-c++-common/auto-init-13.c: New test. >> * c-c++-common/auto-init-14.c: New test. >> >> * gcc.target/aarch64/auto-init-9.c: New test. >> * gcc.target/aarch64/auto-init-10.c: New test. >> * gcc.target/aarch64/auto-init-11.c: New test. >> * gcc.target/aarch64/auto-init-12.c: New test. >> * gcc.target/aarch64/auto-init-13.c: New test. >> * gcc.target/aarch64/auto-init-14.c: New test. >> * gcc.target/aarch64/auto-init-15.c: New test. >> * gcc.target/aarch64/auto-init-16.c: New test. >> * gcc.target/aarch64/auto-init-17.c: New test. >> * gcc.target/aarch64/auto-init-18.c: New test. >> * gcc.target/aarch64/auto-init-19.c: New test. >> * gcc.target/aarch64/auto-init-20.c: New test. >> >> * gcc.target/i386/auto-init-9.c: New test. >> * gcc.target/i386/auto-init-10.c: New test. >> * gcc.target/i386/auto-init-11.c: New test. >> * gcc.target/i386/auto-init-12.c: New test. >> * gcc.target/i386/auto-init-13.c: New test. >> * gcc.target/i386/auto-init-14.c: New test. >> * gcc.target/i386/auto-init-15.c: New test. >> * gcc.target/i386/auto-init-16.c: New test. >> * gcc.target/i386/auto-init-17.c: New test. >> * gcc.target/i386/auto-init-18.c: New test. >> * gcc.target/i386/auto-init-19.c: New test. >> * gcc.target/i386/auto-init-20.c: New test. >> >> 4. Update the default approach as D, then when specify >> -ftrivial-auto-var-init, the default approach is the “.DEFERRED_INIT” >> Approach. No need to add -fauto-var-init-approach=D anymore. >> >> If we need to compare approach A and D, we can add >> -fauto-var-init-approach=A to get that implementation. >> >> 5. Delete all -fauto-var-init-approach=D in the testing cases. >> >> ****** others are the same as Version 1, please see the version 1 >> description at: >> >> https://gcc.gnu.org/pipermail/gcc-patches/2021-February/565581.html >> >> >> *******Changelog: >> >> gcc/: >> >> 2021-03-24 qing zhao <qing.z...@oracle.com> >> >> * common.opt (ftrivial-auto-var-init=): New. >> (fauto-var-init-approach=): Likewise. >> * doc/extend.texi: Document the uninitialized attribute. >> * doc/invoke.texi: Document -ftrivial-auto-var-init. >> * expr.c (store_constructor): Clear the whole structure when >> -ftrivial-auto-var-init and the structure has paddings. >> * flag-types.h (enum auto_init_type): New enumerated type >> auto_init_type. >> (enum auto_init_approach): New enumerated type auto_init_approach. >> * gimple.h (enum gf_mask): Add GF_CALL_MEMSET_FOR_UNINIT case. >> (gimple_call_set_memset_for_uninit): New function. >> (gimple_call_memset_for_uninit_p): Likewise. >> * gimplify.c (gimplify_vla_decl): Add initialization to vla per users' >> requests. >> (build_deferred_init): New function. >> (gimple_add_init_for_auto_var): Likewise. >> (gimplify_decl_expr): Add initialization to automatic variables per >> users' requests. >> (gimplify_init_constructor): Clear the whole structure when >> -ftrivial-auto-var-init and the structure has paddings. >> * internal-fn.c (expand_DEFERRED_INIT): New function. >> * internal-fn.def (DEFERRED_INIT): New internal function. >> * tree-cfg.c (verify_gimple_call): Skip calls to DEFERRED_INIT. >> * tree-core.h (tree_decl_with_vis): Add uninitialized field. >> * tree-sra.c (sra_stats): Add two new fields deferred_init and >> subtree_deferred_init. >> (generate_subtree_deferred_init): New function. >> (sra_modify_deferred_init): Likewise. >> (sra_modify_function_body): Handle calls to DEFERRED_INIT specially. >> * tree-ssa-structalias.c (find_func_aliases_for_deferred_init): New >> function. >> (find_func_aliases_for_call): Handle calls to DEFERRED_INIT specially. >> * tree-ssa-uninit.c (warn_uninit): Handle calls to DEFERRED_INIT >> specially. >> (check_defs): Handle calls to DEFERRED_INIT and MEMSET for uninitialized >> variable specially. >> (warn_uninitialized_vars): Handle calls to DEFERRED_INIT specially. >> * tree-ssa.c (ssa_undefined_value_p): Handle calls to DEFERRED_INIT >> specially. >> * tree.c (build_pattern_cst): New function. >> (type_has_padding): Likewise. >> * tree.h (DECL_UNINITIALIZED): New macro. >> (build_pattern_cst): New declaration. >> (type_has_padding): Likewise. >> >> gcc/c-family/: >> >> 2021-03-24 qing zhao <qing.z...@oracle.com> >> >> * c-attribs.c (handle_uninitialized_attribute): New function. >> (c_common_attribute_table): Add "uninitialized" attribute. >> >> gcc/testsuite/: >> >> 2021-03-24 qing zhao <qing.z...@oracle.com> >> >> * c-c++-common/auto-init-1.c: New test. >> * c-c++-common/auto-init-10.c: New test. >> * c-c++-common/auto-init-11.c: New test. >> * c-c++-common/auto-init-12.c: New test. >> * c-c++-common/auto-init-13.c: New test. >> * c-c++-common/auto-init-14.c: New test. >> * c-c++-common/auto-init-2.c: New test. >> * c-c++-common/auto-init-3.c: New test. >> * c-c++-common/auto-init-4.c: New test. >> * c-c++-common/auto-init-5.c: New test. >> * c-c++-common/auto-init-6.c: New test. >> * c-c++-common/auto-init-7.c: New test. >> * c-c++-common/auto-init-8.c: New test. >> * c-c++-common/auto-init-9.c: New test. >> * c-c++-common/auto-init-esra.c: New test. >> * g++.dg/auto-init-uninit-pred-1_a.C: New test. >> * g++.dg/auto-init-uninit-pred-1_b.C: New test. >> * g++.dg/auto-init-uninit-pred-2_a.C: New test. >> * g++.dg/auto-init-uninit-pred-2_b.C: New test. >> * g++.dg/auto-init-uninit-pred-3_a.C: New test. >> * g++.dg/auto-init-uninit-pred-3_b.C: New test. >> * g++.dg/auto-init-uninit-pred-4.C: New test. >> * g++.dg/auto-init-uninit-pred-loop-1_a.cc: New test. >> * g++.dg/auto-init-uninit-pred-loop-1_b.cc: New test. >> * g++.dg/auto-init-uninit-pred-loop-1_c.cc: New test. >> * g++.dg/auto-init-uninit-pred-loop_1.cc: New test. >> * gcc.dg/auto-init-uninit-1.c: New test. >> * gcc.dg/auto-init-uninit-11.c: New test. >> * gcc.dg/auto-init-uninit-12.c: New test. >> * gcc.dg/auto-init-uninit-13.c: New test. >> * gcc.dg/auto-init-uninit-14.c: New test. >> * gcc.dg/auto-init-uninit-15.c: New test. >> * gcc.dg/auto-init-uninit-16.c: New test. >> * gcc.dg/auto-init-uninit-17.c: New test. >> * gcc.dg/auto-init-uninit-18.c: New test. >> * gcc.dg/auto-init-uninit-19.c: New test. >> * gcc.dg/auto-init-uninit-2.c: New test. >> * gcc.dg/auto-init-uninit-20.c: New test. >> * gcc.dg/auto-init-uninit-21.c: New test. >> * gcc.dg/auto-init-uninit-22.c: New test. >> * gcc.dg/auto-init-uninit-23.c: New test. >> * gcc.dg/auto-init-uninit-24.c: New test. >> * gcc.dg/auto-init-uninit-25.c: New test. >> * gcc.dg/auto-init-uninit-26.c: New test. >> * gcc.dg/auto-init-uninit-3.c: New test. >> * gcc.dg/auto-init-uninit-34.c: New test. >> * gcc.dg/auto-init-uninit-36.c: New test. >> * gcc.dg/auto-init-uninit-37.c: New test. >> * gcc.dg/auto-init-uninit-4.c: New test. >> * gcc.dg/auto-init-uninit-5.c: New test. >> * gcc.dg/auto-init-uninit-6.c: New test. >> * gcc.dg/auto-init-uninit-8.c: New test. >> * gcc.dg/auto-init-uninit-9.c: New test. >> * gcc.dg/auto-init-uninit-A.c: New test. >> * gcc.dg/auto-init-uninit-B.c: New test. >> * gcc.dg/auto-init-uninit-C.c: New test. >> * gcc.dg/auto-init-uninit-H.c: New test. >> * gcc.dg/auto-init-uninit-I.c: New test. >> * gcc.target/aarch64/auto-init-1.c: New test. >> * gcc.target/aarch64/auto-init-10.c: New test. >> * gcc.target/aarch64/auto-init-11.c: New test. >> * gcc.target/aarch64/auto-init-12.c: New test. >> * gcc.target/aarch64/auto-init-13.c: New test. >> * gcc.target/aarch64/auto-init-14.c: New test. >> * gcc.target/aarch64/auto-init-15.c: New test. >> * gcc.target/aarch64/auto-init-16.c: New test. >> * gcc.target/aarch64/auto-init-17.c: New test. >> * gcc.target/aarch64/auto-init-18.c: New test. >> * gcc.target/aarch64/auto-init-19.c: New test. >> * gcc.target/aarch64/auto-init-2.c: New test. >> * gcc.target/aarch64/auto-init-20.c: New test. >> * gcc.target/aarch64/auto-init-3.c: New test. >> * gcc.target/aarch64/auto-init-4.c: New test. >> * gcc.target/aarch64/auto-init-5.c: New test. >> * gcc.target/aarch64/auto-init-6.c: New test. >> * gcc.target/aarch64/auto-init-7.c: New test. >> * gcc.target/aarch64/auto-init-8.c: New test. >> * gcc.target/aarch64/auto-init-9.c: New test. >> * gcc.target/i386/auto-init-1.c: New test. >> * gcc.target/i386/auto-init-10.c: New test. >> * gcc.target/i386/auto-init-11.c: New test. >> * gcc.target/i386/auto-init-12.c: New test. >> * gcc.target/i386/auto-init-13.c: New test. >> * gcc.target/i386/auto-init-14.c: New test. >> * gcc.target/i386/auto-init-15.c: New test. >> * gcc.target/i386/auto-init-16.c: New test. >> * gcc.target/i386/auto-init-17.c: New test. >> * gcc.target/i386/auto-init-18.c: New test. >> * gcc.target/i386/auto-init-19.c: New test. >> * gcc.target/i386/auto-init-2.c: New test. >> * gcc.target/i386/auto-init-20.c: New test. >> * gcc.target/i386/auto-init-3.c: New test. >> * gcc.target/i386/auto-init-4.c: New test. >> * gcc.target/i386/auto-init-5.c: New test. >> * gcc.target/i386/auto-init-6.c: New test. >> * gcc.target/i386/auto-init-7.c: New test. >> * gcc.target/i386/auto-init-8.c: New test. >> * gcc.target/i386/auto-init-9.c: New test. >> >> ******The complete patch is attached as following: >> > > >> >> > > > -- > Kees Cook