Hi!

My recent patch for PR70035 broke -fsanitize=vptr, the early clearing
of _vptr.* pointers in the objects can crash in constructors with _vtt_parm
parameter.

This patch arranges to use in them *_vtt_parm instead the base vptr (which
we clear, instead of initialize).

The first testcase is just to make sure we don't crash, the second one
is an attempt to find out what is and what is not valid C++ and what can we
(easily) diagnose.

According to Jonathan, in the second testcase in B::B() all the calls
to g(), h() and i() methods before the E and F base classes are constructed
are invalid, while in the non-static data member mem initializers and
within body they are ok.  With the patch we diagnose all but the
g() call in F() base constructor argument (because the E() construction
initializes the vptr and we don't clear it once again afterwards).
clang doesn't instrument any of these, but they warn at compile time about
the g() call in E(g() + ...) and both h() calls (in E() and F()), but e.g.
doesn't warn about either of the calls to i().  At the point where they are
called, A() is already constructed, but other bases aren't constructed yet.
If we should instrument just what clang complains about at compile time,
we could e.g. in addition to this patch conditionalize the clearing of virtual
base vptr pointers by test on the __in_chrg argument, thus only clear those
in the comp ctor.

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

2016-03-14  Jakub Jelinek  <ja...@redhat.com>

        PR c++/70147
        * cp-ubsan.c (cp_ubsan_maybe_initialize_vtbl_ptrs): Temporarily
        set in_base_initializer.

        * g++.dg/ubsan/pr70147-1.C: New test.
        * g++.dg/ubsan/pr70147-2.C: New test.

--- gcc/cp/cp-ubsan.c.jj        2016-03-04 23:09:53.000000000 +0100
+++ gcc/cp/cp-ubsan.c   2016-03-14 15:08:18.107919371 +0100
@@ -318,9 +318,15 @@ cp_ubsan_maybe_initialize_vtbl_ptrs (tre
 
   tree type = TREE_TYPE (TREE_TYPE (addr));
   tree list = build_tree_list (type, addr);
+  /* We cannot rely on the vtable being set up.  We have to indirect via the
+     vtt_parm.  */
+  int save_in_base_initializer = in_base_initializer;
+  in_base_initializer = 1;
 
   /* Walk through the hierarchy, initializing the vptr in each base
      class to NULL.  */
   dfs_walk_once (TYPE_BINFO (type), cp_ubsan_dfs_initialize_vtbl_ptrs,
                 NULL, list);
+
+  in_base_initializer = save_in_base_initializer;
 }
--- gcc/testsuite/g++.dg/ubsan/pr70147-1.C.jj   2016-03-14 15:20:03.648332034 
+0100
+++ gcc/testsuite/g++.dg/ubsan/pr70147-1.C      2016-03-14 15:18:52.000000000 
+0100
@@ -0,0 +1,12 @@
+// PR c++/70147
+// { dg-do run }
+// { dg-options "-fsanitize=vptr" }
+
+struct A { A () {} virtual void f () {} };
+struct B : virtual A { B () {} virtual void f () {} };
+struct C : B, virtual A { C () {} } c;
+
+int
+main ()
+{
+}
--- gcc/testsuite/g++.dg/ubsan/pr70147-2.C.jj   2016-03-14 17:07:19.638824667 
+0100
+++ gcc/testsuite/g++.dg/ubsan/pr70147-2.C      2016-03-14 18:09:45.934170053 
+0100
@@ -0,0 +1,83 @@
+// PR c++/70147
+// { dg-do run }
+// { dg-skip-if "" { *-*-* }  { "*" } { "-O0" } }
+// { dg-options "-fsanitize=vptr" }
+
+struct A
+{
+  A () : a (0) {}
+  A (int x) : a (x) {}
+  virtual void f () {}
+  virtual int i () { int r = 0; __asm ("" : "+r" (r)); return r; }
+  int a;
+};
+struct E
+{
+  E () : e (0) {}
+  E (int x) : e (x) {}
+  virtual void f () {}
+  virtual int g () { int r = 0; __asm ("" : "+r" (r)); return r; }
+  int e;
+};
+struct F
+{
+  F () : f (0) {}
+  F (int x) : f (x) {}
+  virtual int h () { int r = 0; __asm ("" : "+r" (r)); return r; }
+  int f;
+};
+struct B : virtual A, public E, public F
+{
+  B ()
+    : E (
+         g ()
+         + h ()
+         + i ()
+        ),
+      F (g ()
+         + h ()
+         + i ()),
+      b (g () + h () + i ())   // It is ok to call the methods here.
+  {
+    b += g () + h () + i ();   // And here too.
+  }
+  virtual void f () {}
+  int b;
+};
+struct C : B, virtual A
+{
+  C () {}
+};
+
+int
+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 "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 "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 "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 "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" }

        Jakub

Reply via email to