On Fri, Oct 27, 2017 at 01:16:08PM +0200, Martin Liška wrote:
> On 10/27/2017 12:52 PM, Jakub Jelinek wrote:
> > The decl.c change seems to be only incremental change from a not publicly
> > posted patch rather than the full diff against trunk.
>
> Sorry for that. Sending full patch.
Thanks.
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -15280,7 +15280,19 @@ begin_destructor_body (void)
> if (flag_lifetime_dse
> /* Clobbering an empty base is harmful if it overlays real data. */
> && !is_empty_class (current_class_type))
> - finish_decl_cleanup (NULL_TREE, build_clobber_this ());
> + {
> + if (sanitize_flags_p (SANITIZE_VPTR)
> + && (flag_sanitize_recover & SANITIZE_VPTR) == 0)
> + {
> + tree fndecl = builtin_decl_explicit (BUILT_IN_MEMSET);
> + tree call = build_call_expr (fndecl, 3,
> + current_class_ptr, integer_zero_node,
> + TYPE_SIZE_UNIT (current_class_type));
I wonder if it wouldn't be cheaper to just use thisref = {}; rather than
memset, pretty much the same thing as build_clobber_this () emits, except
for the TREE_VOLATILE. Also, build_clobber_this has:
if (vbases)
exprstmt = build_if_in_charge (exprstmt);
so it doesn't clobber if not in charge, not sure if it applies here too.
So maybe easiest would be add a bool argument to build_clobber_this which
would say whether it is a clobber or real clearing?
> + finish_decl_cleanup (NULL_TREE, call);
> + }
> + else
> + finish_decl_cleanup (NULL_TREE, build_clobber_this ());
> + }
>
> /* And insert cleanups for our bases and members so that they
> will be properly destroyed if we throw. */
> diff --git a/gcc/testsuite/g++.dg/ubsan/vptr-12.C
> b/gcc/testsuite/g++.dg/ubsan/vptr-12.C
> new file mode 100644
> index 00000000000..96c8473d757
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ubsan/vptr-12.C
> @@ -0,0 +1,26 @@
> +// { dg-do run }
> +// { dg-shouldfail "ubsan" }
> +// { dg-options "-fsanitize=vptr -fno-sanitize-recover=vptr" }
> +
> +struct MyClass
> +{
> + virtual ~MyClass () {}
> + virtual void
> + Doit ()
> + {
> + }
Why not put all the above 4 lines into a single one, the dtor already uses
that kind of formatting.
> +};
> +
> +int
> +main ()
> +{
> + MyClass *c = new MyClass;
> + c->~MyClass ();
> + c->Doit ();
> +
> + return 0;
> +}
> +
> +// { dg-output "\[^\n\r]*vptr-12.C:19:\[0-9]*: runtime error: member call on
> address 0x\[0-9a-fA-F]* which does not point to an object of type
> 'MyClass'(\n|\r\n|\r)" }
> +// { dg-output "0x\[0-9a-fA-F]*: note: object has invalid vptr(\n|\r\n|\r)" }
> +
Unnecessary empty line at end.
Jakub
