[Bug sanitizer/67258] "invalid vptr" false positive from ubsan for virtual inheritance

[Bug sanitizer/67258] "invalid vptr" false positive from ubsan for virtual inheritance

[avi@cloudius-systems.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67258

Avi Kivity <avi@cloudius-systems.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |avi@cloudius-systems.com

--- Comment #1 from Avi Kivity <avi@cloudius-systems.com> ---
I bisected a similar problem (with boost unit-tests) to:

commit 6365c92707e013c1bc7c262bd762ac8bfacbdda3
Author: hubicka <hubicka@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Thu Jan 15 23:11:49 2015 +0000

        PR ipa/64612
        * ipa-inline-transform.c (can_remove_node_now_p): Fix handling
        of comdat locals.
        (inline_call): Fix removal of aliases.


    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@219696
138bc75d-0d04-0410-961f-82ee72b054a4

Strangely this is one commit after the commit that introduces -fsanitize=vptr,
so maybe it's unrelated and the previous commit is guilty.

[Bug sanitizer/67258] "invalid vptr" false positive from ubsan for virtual inheritance

[trippels at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67258

Markus Trippelsdorf <trippels at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-09-09
                 CC|                            |trippels at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #2 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
This was fixed upstream by:

commit 1d2477faafda9ad2cc19927b3c31efd22747f013
Author: Alexey Samsonov <vonosmas@gmail.com>
Date:   Wed Aug 5 19:35:46 2015 +0000

    [UBSan] Fix UBSan-vptr false positive.

    Offset from vptr to the start of most-derived object can actually
    be positive in some virtual base class vtables.

    Patch by Stephan Bergmann!

    git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@244101
91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/ubsan/ubsan_type_hash_itanium.cc
b/lib/ubsan/ubsan_type_hash_itanium.cc
index 5cd46df16a33..b84e88d4c71d 100644
--- a/lib/ubsan/ubsan_type_hash_itanium.cc
+++ b/lib/ubsan/ubsan_type_hash_itanium.cc
@@ -185,8 +185,8 @@ namespace {

 struct VtablePrefix {
   /// The offset from the vptr to the start of the most-derived object.
-  /// This should never be greater than zero, and will usually be exactly
-  /// zero.
+  /// This will only be greater than zero in some virtual base class vtables
+  /// used during object con-/destruction, and will usually be exactly zero.
   sptr Offset;
   /// The type_info object describing the most-derived class type.
   std::type_info *TypeInfo;
@@ -196,7 +196,7 @@ VtablePrefix *getVtablePrefix(void *Vtable) {
   if (!Vptr)
     return 0;
   VtablePrefix *Prefix = Vptr - 1;
-  if (Prefix->Offset > 0 || !Prefix->TypeInfo)
+  if (!Prefix->TypeInfo)
     // This can't possibly be a valid vtable.
     return 0;
   return Prefix;
diff --git a/test/ubsan/TestCases/TypeCheck/vptr-virtual-base-construction.cpp
b/test/ubsan/TestCases/TypeCheck/vptr-virtual-base-construction.cpp
new file mode 100644
index 000000000000..dc27d9f39ce3
--- /dev/null
+++ b/test/ubsan/TestCases/TypeCheck/vptr-virtual-base-construction.cpp
@@ -0,0 +1,13 @@
+// RUN: %clangxx -frtti -fsanitize=vptr -fno-sanitize-recover=vptr %s -o %t
+// RUN: %run %t
+
+// REQUIRES: cxxabi
+
+int volatile n;
+
+struct A { virtual ~A() {} };
+struct B: virtual A {};
+struct C: virtual A { ~C() { n = 0; } };
+struct D: virtual B, virtual C {};
+
+int main() { delete new D; }

Analogous gcc patch would be:

diff --git a/libsanitizer/ubsan/ubsan_type_hash.cc
b/libsanitizer/ubsan/ubsan_type_hash.cc
index d01009426db0..495ec5386cf3 100644
--- a/libsanitizer/ubsan/ubsan_type_hash.cc
+++ b/libsanitizer/ubsan/ubsan_type_hash.cc
@@ -197,7 +197,7 @@ VtablePrefix *getVtablePrefix(void *Object) {
   if (!*VptrPtr)
     return 0;
   VtablePrefix *Prefix = *VptrPtr - 1;
-  if (Prefix->Offset > 0 || !Prefix->TypeInfo)
+  if (!Prefix->TypeInfo)
     // This can't possibly be a valid vtable.
     return 0;
   return Prefix;

[Bug sanitizer/67258] "invalid vptr" false positive from ubsan for virtual inheritance

[trippels at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67258

--- Comment #3 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
Author: trippels
Date: Wed Sep  9 16:34:59 2015
New Revision: 227591

URL: https://gcc.gnu.org/viewcvs?rev=227591&root=gcc&view=rev
Log:
Fix sanitizer/67258 by cherry picking upstream patch

        PR sanitizer/67258
        * ubsan/ubsan_type_hash.cc: Cherry pick upstream r244101.

Upstraem patch:
commit 1d2477faafda9ad2cc19927b3c31efd22747f013
Author: Alexey Samsonov <vonosmas@gmail.com>
Date:   Wed Aug 5 19:35:46 2015 +0000

    [UBSan] Fix UBSan-vptr false positive.

    Offset from vptr to the start of most-derived object can actually
    be positive in some virtual base class vtables.

    Patch by Stephan Bergmann!

    git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@244101
91177308-0d34-0410-b5e6-96231b3b80d8

Added:
    trunk/gcc/testsuite/g++.dg/ubsan/vptr-10.C
Modified:
    trunk/libsanitizer/ChangeLog
    trunk/libsanitizer/ubsan/ubsan_type_hash.cc

[Bug sanitizer/67258] "invalid vptr" false positive from ubsan for virtual inheritance

[trippels at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67258

--- Comment #4 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
Author: trippels
Date: Wed Sep  9 16:39:32 2015
New Revision: 227592

URL: https://gcc.gnu.org/viewcvs?rev=227592&root=gcc&view=rev
Log:
Fix sanitizer/67258 by cherry picking upstream patch

        PR sanitizer/67258
        * ubsan/ubsan_type_hash.cc: Cherry pick upstream r244101.

Upstream patch:
commit 1d2477faafda9ad2cc19927b3c31efd22747f013
Author: Alexey Samsonov <vonosmas@gmail.com>
Date:   Wed Aug 5 19:35:46 2015 +0000

    [UBSan] Fix UBSan-vptr false positive.

    Offset from vptr to the start of most-derived object can actually
    be positive in some virtual base class vtables.

    Patch by Stephan Bergmann!

    git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@244101
91177308-0d34-0410-b5e6-96231b3b80d8

Added:
    branches/gcc-5-branch/gcc/testsuite/g++.dg/ubsan/vptr-10.C
Modified:
    branches/gcc-5-branch/libsanitizer/ChangeLog
    branches/gcc-5-branch/libsanitizer/ubsan/ubsan_type_hash.cc

[Bug sanitizer/67258] "invalid vptr" false positive from ubsan for virtual inheritance

[trippels at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67258

Markus Trippelsdorf <trippels at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #5 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
Fixed.