From: Jakub Jelinek Date: Fri, 22 Jan 2021 18:03:23 +0000 (+0100) Subject: c++: Fix up ubsan false positives on references [PR95693] X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=a9ed18295bfc6d69d40af197e059e16622cd94c6;p=gcc.git c++: Fix up ubsan false positives on references [PR95693] 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. 2021-01-22 Jakub Jelinek 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. --- diff --git a/gcc/cp/init.c b/gcc/cp/init.c index e6de2e10dc3..3b37c970dfa 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -288,10 +288,7 @@ build_zero_init_1 (tree type, tree nelts, bool static_storage_p, 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) diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c index d9362500f06..f4be72fc514 100644 --- a/gcc/cp/typeck2.c +++ b/gcc/cp/typeck2.c @@ -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 diff --git a/gcc/testsuite/g++.dg/ubsan/pr95693.C b/gcc/testsuite/g++.dg/ubsan/pr95693.C new file mode 100644 index 00000000000..13f688e8ec0 --- /dev/null +++ b/gcc/testsuite/g++.dg/ubsan/pr95693.C @@ -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; +}