diff mbox

[C++] Fix -fsanitize=vptr (PR c++/70147)

Message ID 20160315162428.GY3017@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek March 15, 2016, 4:24 p.m. UTC 
Hi!

Bernd E. mentioned in the PR the problem that if some subobject ctor throws,
if for -fsanitize=vptr we clear again the vtable pointers even for virtual
bases then they won't be properly destructed.

So, here is an incremental patch to the earlier patch, which will clear
the virtual base vtbl pointers only in the in-charge ctor.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Though, this brings a non-sanitizer issue, for -flifetime-dse=2
we emit a clobber of the whole subobject even in a ctor with _vtt_parm
argument, and the virtual bases at that point might live inside of the
area that is clobbered by the ctor {CLOBBER}
(both data and vtable pointers).  I'm afraid that e.g. with inlining this
could result into wrong code, e.g. DSE removing the earlier stores from the
virtual base ctor stores, because we {CLOBBER} them later on.
Shouldn't we conditionalize the -flifetime-dse=2 clobbers on __in_chrg
(if that parm is present)?

2016-03-15  Jakub Jelinek  <jakub@redhat.com>

	PR c++/70147
	* cp-ubsan.c (cp_ubsan_dfs_initialize_vtbl_ptrs): Conditionalize
	BINFO_VIRTUAL_P vtable clearing on current_in_charge_parm.

	* g++.dg/ubsan/pr70147-2.C (C::C): Initialize A base with invalid
	method call to i () as argument.  Adjust expected output.



	Jakub

Comments

Jason Merrill March 16, 2016, 4:11 a.m. UTC | #1 
On 03/15/2016 12:24 PM, Jakub Jelinek wrote:
> Bernd E. mentioned in the PR the problem that if some subobject ctor throws,
> if for -fsanitize=vptr we clear again the vtable pointers even for virtual
> bases then they won't be properly destructed.

> So, here is an incremental patch to the earlier patch, which will clear
> the virtual base vtbl pointers only in the in-charge ctor.

Right. If we aren't in charge of constructing the base, we shouldn't 
mess with its vptr either.  Both patches are OK.

> Though, this brings a non-sanitizer issue, for -flifetime-dse=2
> we emit a clobber of the whole subobject even in a ctor with _vtt_parm
> argument, and the virtual bases at that point might live inside of the
> area that is clobbered by the ctor {CLOBBER}
> (both data and vtable pointers).

See my comment in the PR.

Jason
diff mbox

Patch

--- gcc/cp/cp-ubsan.c.jj	2016-03-15 09:25:19.000000000 +0100
+++ gcc/cp/cp-ubsan.c	2016-03-15 09:40:51.209005916 +0100
@@ -299,8 +299,14 @@  cp_ubsan_dfs_initialize_vtbl_ptrs (tree
 
       /* Assign NULL to the vptr.  */
       tree vtbl = build_zero_cst (TREE_TYPE (vtbl_ptr));
-      finish_expr_stmt (cp_build_modify_expr (vtbl_ptr, NOP_EXPR, vtbl,
-					      tf_warning_or_error));
+      tree stmt = cp_build_modify_expr (vtbl_ptr, NOP_EXPR, vtbl,
+					tf_warning_or_error);
+      if (BINFO_VIRTUAL_P (binfo))
+	stmt = build3 (COND_EXPR, void_type_node,
+		       build2 (NE_EXPR, boolean_type_node,
+			       current_in_charge_parm, integer_zero_node),
+		       stmt, void_node);
+      finish_expr_stmt (stmt);
     }
 
   return NULL_TREE;
--- gcc/testsuite/g++.dg/ubsan/pr70147-2.C.jj	2016-03-15 09:46:24.000000000 +0100
+++ gcc/testsuite/g++.dg/ubsan/pr70147-2.C	2016-03-15 09:48:04.622051166 +0100
@@ -46,7 +46,7 @@  struct B : virtual A, public E, public F
 };
 struct C : B, virtual A
 {
-  C () {}
+  C () : A (i ()) {}
 };
 
 int
@@ -55,28 +55,22 @@  main ()
   C c;
 }
 
-// { dg-output "\[^\n\r]*pr70147-2.C:33:\[0-9]*: runtime error: member call on address 0x\[0-9a-fA-F]* which does not point to an object of type 'E'(\n|\r\n|\r)" }
-// { dg-output "0x\[0-9a-fA-F]*: note: object has invalid vptr(\n|\r\n|\r)" }
-// { dg-output "  ?.. .. .. ..  ?.. .. .. ..  ?.. .. .. .. \[^\n\r]*(\n|\r\n|\r)" }
-// { dg-output "              ?\\^~~~~~~~~~~\[^\n\r]*(\n|\r\n|\r)" }
-// { dg-output "              ?invalid vptr(\n|\r\n|\r)" }
-// { dg-output "\[^\n\r]*pr70147-2.C:34:\[0-9]*: runtime error: member call on address 0x\[0-9a-fA-F]* which does not point to an object of type 'F'(\n|\r\n|\r)" }
+// { dg-output "\[^\n\r]*pr70147-2.C:49:\[0-9]*: runtime error: member call on address 0x\[0-9a-fA-F]* which does not point to an object of type 'A'(\n|\r\n|\r)" }
 // { dg-output "0x\[0-9a-fA-F]*: note: object has invalid vptr(\n|\r\n|\r)" }
 // { dg-output "  ?.. .. .. ..  ?.. .. .. ..  ?.. .. .. .. \[^\n\r]*(\n|\r\n|\r)" }
 // { dg-output "              ?\\^~~~~~~~~~~\[^\n\r]*(\n|\r\n|\r)" }
 // { dg-output "              ?invalid vptr\[^\n\r]*(\n|\r\n|\r)" }
-// { dg-output "\[^\n\r]*pr70147-2.C:35:\[0-9]*: runtime error: member call on address 0x\[0-9a-fA-F]* which does not point to an object of type 'A'(\n|\r\n|\r)" }
+// { dg-output "\[^\n\r]*pr70147-2.C:33:\[0-9]*: runtime error: member call on address 0x\[0-9a-fA-F]* which does not point to an object of type 'E'(\n|\r\n|\r)" }
 // { dg-output "0x\[0-9a-fA-F]*: note: object has invalid vptr(\n|\r\n|\r)" }
 // { dg-output "  ?.. .. .. ..  ?.. .. .. ..  ?.. .. .. .. \[^\n\r]*(\n|\r\n|\r)" }
 // { dg-output "              ?\\^~~~~~~~~~~\[^\n\r]*(\n|\r\n|\r)" }
-// { dg-output "              ?invalid vptr\[^\n\r]*(\n|\r\n|\r)" }
-// Note we don't catch the UB of calling g () on line 36.
-// { dg-output "\[^\n\r]*pr70147-2.C:38:\[0-9]*: runtime error: member call on address 0x\[0-9a-fA-F]* which does not point to an object of type 'F'(\n|\r\n|\r)" }
+// { dg-output "              ?invalid vptr(\n|\r\n|\r)" }
+// { dg-output "\[^\n\r]*pr70147-2.C:34:\[0-9]*: runtime error: member call on address 0x\[0-9a-fA-F]* which does not point to an object of type 'F'(\n|\r\n|\r)" }
 // { dg-output "0x\[0-9a-fA-F]*: note: object has invalid vptr(\n|\r\n|\r)" }
 // { dg-output "  ?.. .. .. ..  ?.. .. .. ..  ?.. .. .. .. \[^\n\r]*(\n|\r\n|\r)" }
 // { dg-output "              ?\\^~~~~~~~~~~\[^\n\r]*(\n|\r\n|\r)" }
 // { dg-output "              ?invalid vptr\[^\n\r]*(\n|\r\n|\r)" }
-// { dg-output "\[^\n\r]*pr70147-2.C:39:\[0-9]*: runtime error: member call on address 0x\[0-9a-fA-F]* which does not point to an object of type 'A'(\n|\r\n|\r)" }
+// { dg-output "\[^\n\r]*pr70147-2.C:38:\[0-9]*: runtime error: member call on address 0x\[0-9a-fA-F]* which does not point to an object of type 'F'(\n|\r\n|\r)" }
 // { dg-output "0x\[0-9a-fA-F]*: note: object has invalid vptr(\n|\r\n|\r)" }
 // { dg-output "  ?.. .. .. ..  ?.. .. .. ..  ?.. .. .. .. \[^\n\r]*(\n|\r\n|\r)" }
 // { dg-output "              ?\\^~~~~~~~~~~\[^\n\r]*(\n|\r\n|\r)" }