On 1/22/21 3:30 AM, Jakub Jelinek wrote:
Hi!Alex' 2 years old change to build_zero_init_1 to return NULL pointer with reference type for references breaks the sanitizers, the assignment of NULL to a reference typed member is then instrumented before it is overwritten with a non-NULL address later on. That change has been done to fix error recovery ICE during process_init_constructor_record, where we: if (TYPE_REF_P (fldtype)) { if (complain & tf_error) error ("member %qD is uninitialized reference", field); else return PICFLAG_ERRONEOUS; } a few lines earlier, but then continue and ICE when build_zero_init returns NULL. The following patch reverts the build_zero_init_1 change and instead creates the NULL with reference type constants during the error recovery. The pr84593.C testcase Alex' change was fixing still works as before. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
2021-01-22 Jakub Jelinek <ja...@redhat.com> PR sanitizer/95693 * init.c (build_zero_init_1): Revert the 2018-03-06 change to return build_zero_cst for reference types. * typeck2.c (process_init_constructor_record): Instead call build_zero_cst here during error recovery instead of build_zero_init. * g++.dg/ubsan/pr95693.C: New test. --- gcc/cp/init.c.jj 2021-01-04 10:25:48.384125675 +0100 +++ gcc/cp/init.c 2021-01-21 13:59:26.863840340 +0100 @@ -288,10 +288,7 @@ build_zero_init_1 (tree type, tree nelts else if (VECTOR_TYPE_P (type)) init = build_zero_cst (type); else - { - gcc_assert (TYPE_REF_P (type)); - init = build_zero_cst (type); - } + gcc_assert (TYPE_REF_P (type));/* In all cases, the initializer is a constant. */if (init) --- gcc/cp/typeck2.c.jj 2021-01-21 14:22:45.000000000 +0100 +++ gcc/cp/typeck2.c 2021-01-21 14:24:25.119772897 +0100 @@ -1609,10 +1609,14 @@ process_init_constructor_record (tree type, tree init, int nested, int flags, warning (OPT_Wmissing_field_initializers, "missing initializer for member %qD", field);- if (!zero_init_p (fldtype)- || skipped < 0) - next = build_zero_init (fldtype, /*nelts=*/NULL_TREE, - /*static_storage_p=*/false); + if (!zero_init_p (fldtype) || skipped < 0) + { + if (TYPE_REF_P (fldtype)) + next = build_zero_cst (fldtype); + else + next = build_zero_init (fldtype, /*nelts=*/NULL_TREE, + /*static_storage_p=*/false); + } else { /* The default zero-initialization is fine for us; don't --- gcc/testsuite/g++.dg/ubsan/pr95693.C.jj 2021-01-21 14:31:31.536921423 +0100 +++ gcc/testsuite/g++.dg/ubsan/pr95693.C 2021-01-21 14:32:42.590114353 +0100 @@ -0,0 +1,26 @@ +// PR sanitizer/95693 +// { dg-do run } +// { dg-options "-O2 -fsanitize=undefined -fno-sanitize-recover=undefined" } + +int g = 9; + +struct A { + A () : a(g) {} +private: + int &a; +}; + +struct B { + A payload; +}; + +struct C : public B { + C () : B () {} + A p; +}; + +int +main () +{ + C t; +} Jakub
