https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70147
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |redi at gcc dot gnu.org
--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Untested patch:
--- 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 ()
+{
+}
Though, I'm still not really sure.
Consider e.g.:
// PR c++/70147
// { dg-do run }
// { dg-options "-fsanitize=vptr" }
struct A { A () {} A (int x) : a (x) {} virtual void f () {} virtual int i () {
return 0; } int a; };
struct E { E () {} E (int x) : e (x) {} virtual void f () {} virtual int g () {
return 0; } int e; };
struct F { F () {} F (int x) : f (x) {} virtual int h () { return 0; } int f;
};
struct B : virtual A, public E, public F { B () : E (g () + i ()), F (h ()) {}
virtual void f () {} int b; };
struct D { D () {} D (int x) : d (x) {} virtual void f () {} int d; };
struct C : B, virtual A { C () {} } c;
int
main ()
{
}
With the patch, I get 3 UB calls reported (g (), i () and h ()). While I
believe g () and h () are clearly invalid, I'm not 100% sure about the i ()
call.
This is in B::B() ctor with __vtt_parm argument, not-in-charge, and I believe
this ctor is called already after the A() has been constructed in the C::C()
ctor.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70035#c3
mentions a reference to the standard, and it is true that at that point not all
the bases have been constructed yet, though some clearly already were.
In B::B with the above patch we have:
MEM[(struct &)this] = {CLOBBER};
// Start of ubsan vptr clearing
this->D.2689._vptr.E = 0B;
D.3012 = MEM[(int (*__vtbl_ptr_type) () * *)__vtt_parm];
D.3013 = D.3012 + 18446744073709551592;
D.3014 = MEM[(long int *)D.3013];
D.3015 = (sizetype) D.3014;
D.3016 = this + D.3015;
D.3016->_vptr.A = 0B;
this->D.2690._vptr.F = 0B;
// End of that. _vptr.E and _vptr.F stores are certainly desirable, the
question is about the _vptr.A store.
// That pointer has been already initialized to non-NULL by the caller
(C::C()), but on the other side seems to be within
// the range of the object clobbered by the above {CLOBBER}. Note that _vptr.A
store could be in theory
// outside of the range of the object clobbered by the above {CLOBBER}.
{
if (0 != 0) goto <D.3017>; else goto <D.3018>;
<D.3017>:
D.3019 = &this->D.2694;
A::A (D.3019);
goto <D.3020>;
<D.3018>:
<D.3020>:
D.3021 = &this->D.2689;
D.3022 = D.3021->_vptr.E;
D.3023 = (long unsigned int) D.3022;
UBSAN_VPTR (D.3021, D.3023, 18446725470975354828, &_ZTI1E, 4B);
// We certainly want to complain here.
D.3024 = E::g (D.3021);
D.3012 = MEM[(int (*__vtbl_ptr_type) () * *)__vtt_parm];
D.3013 = D.3012 + 18446744073709551592;
D.3014 = MEM[(long int *)D.3013];
D.3015 = (sizetype) D.3014;
D.3025 = this + D.3015;
D.3026 = D.3025->_vptr.A;
D.3027 = (long unsigned int) D.3026;
UBSAN_VPTR (D.3025, D.3027, 18446725454765570473, &_ZTI1A, 4B);
// And the question is about this one.
D.3028 = A::i (D.3025);
...
D.3036 = *__vtt_parm;
this->D.2689._vptr.E = D.3036;
D.3037 = this->D.2689._vptr.E;
D.3038 = D.3037 + 18446744073709551592;
D.3039 = MEM[(long int *)D.3038];
D.3040 = (sizetype) D.3039;
D.3041 = this + D.3040;
D.3042 = MEM[(const void * *)__vtt_parm + 8B];
D.3041->_vptr.A = D.3042;
D.3043 = &_ZTV1B + 56;
this->D.2690._vptr.F = D.3043;
// And finally, the above is initialization of the base classes. Note that we
overwrite the _vptr.A, no matter that it has been initialized before already,
and no matter if it lives within the
// current B subobject or outside of it.
