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? 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! -Kees [1] Clang doesn't handle this either https://bugs.llvm.org/show_bug.cgi?id=44916 [2] 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
