| Message ID | 20180828223952.GT2218@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | [C++] Fix -fsanitize=vptr -fno-sanitize-recover=vptr (PR c++/87095) | expand |
OK. On Tue, Aug 28, 2018 at 6:39 PM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > As mentioned in the PR, if some class has nearly empty virtual base as > primary base, it shares the vptr with that primary base; in that case, > we can't clear the vptr in the not in charge destructor of that class, > as the dtor for the virtual base will be invoked later on and needs to have > the virtual pointer non-NULL. > > This version differs from the originally pasted into the PR by the > convert_to_void, so that with -O0 we don't emit an unnecessary load from the > vptr after the store (+ testcase). > > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for > trunk and after a while release branches? > > 2018-08-28 Jakub Jelinek <jakub@redhat.com> > > PR c++/87095 > * decl.c (begin_destructor_body): If current_class_type has > virtual bases and the primary base is nearly empty virtual base, > voidify clearing of vptr and make it conditional on in-charge > argument. > > * g++.dg/ubsan/vptr-13.C: New test. > > --- gcc/cp/decl.c.jj 2018-08-27 17:50:43.781489594 +0200 > +++ gcc/cp/decl.c 2018-08-28 14:41:01.354365914 +0200 > @@ -15696,6 +15696,18 @@ begin_destructor_body (void) > tree stmt = cp_build_modify_expr (input_location, vtbl_ptr, > NOP_EXPR, vtbl, > tf_warning_or_error); > + /* If the vptr is shared with some virtual nearly empty base, > + don't clear it if not in charge, the dtor of the virtual > + nearly empty base will do that later. */ > + if (CLASSTYPE_VBASECLASSES (current_class_type) > + && CLASSTYPE_PRIMARY_BINFO (current_class_type) > + && BINFO_VIRTUAL_P > + (CLASSTYPE_PRIMARY_BINFO (current_class_type))) > + { > + stmt = convert_to_void (stmt, ICV_STATEMENT, > + tf_warning_or_error); > + stmt = build_if_in_charge (stmt); > + } > finish_decl_cleanup (NULL_TREE, stmt); > } > else > --- gcc/testsuite/g++.dg/ubsan/vptr-13.C.jj 2018-08-28 14:50:18.712112488 +0200 > +++ gcc/testsuite/g++.dg/ubsan/vptr-13.C 2018-08-28 14:49:17.895122080 +0200 > @@ -0,0 +1,19 @@ > +// PR c++/87095 > +// { dg-do run } > +// { dg-options "-fsanitize=vptr -fno-sanitize-recover=vptr" } > + > +struct A > +{ > + virtual ~A () {} > +}; > + > +struct B : virtual A {}; > + > +struct C : B {}; > + > +int > +main () > +{ > + C c; > + return 0; > +} > > Jakub
--- gcc/cp/decl.c.jj 2018-08-27 17:50:43.781489594 +0200 +++ gcc/cp/decl.c 2018-08-28 14:41:01.354365914 +0200 @@ -15696,6 +15696,18 @@ begin_destructor_body (void) tree stmt = cp_build_modify_expr (input_location, vtbl_ptr, NOP_EXPR, vtbl, tf_warning_or_error); + /* If the vptr is shared with some virtual nearly empty base, + don't clear it if not in charge, the dtor of the virtual + nearly empty base will do that later. */ + if (CLASSTYPE_VBASECLASSES (current_class_type) + && CLASSTYPE_PRIMARY_BINFO (current_class_type) + && BINFO_VIRTUAL_P + (CLASSTYPE_PRIMARY_BINFO (current_class_type))) + { + stmt = convert_to_void (stmt, ICV_STATEMENT, + tf_warning_or_error); + stmt = build_if_in_charge (stmt); + } finish_decl_cleanup (NULL_TREE, stmt); } else --- gcc/testsuite/g++.dg/ubsan/vptr-13.C.jj 2018-08-28 14:50:18.712112488 +0200 +++ gcc/testsuite/g++.dg/ubsan/vptr-13.C 2018-08-28 14:49:17.895122080 +0200 @@ -0,0 +1,19 @@ +// PR c++/87095 +// { dg-do run } +// { dg-options "-fsanitize=vptr -fno-sanitize-recover=vptr" } + +struct A +{ + virtual ~A () {} +}; + +struct B : virtual A {}; + +struct C : B {}; + +int +main () +{ + C c; + return 0; +}