Created attachment 48111 [details] reproducer code See attached: ubsan-gcc.ii Program ubsan-gcc compiled with options: g++ -fsanitize=undefined -fno-sanitize-recover=all ubsan-gcc.ii exits with following false positive: ubsan-gcc.cpp:12:7: runtime error: member call on address 0x7ffcdd8c9800 which does not point to an object of type 'DE' 0x7ffcdd8c9800: note: object has invalid vptr 17 56 00 00 00 00 00 00 00 00 00 00 00 76 43 93 7b 4b c2 bf 30 71 eb 2f 17 56 00 00 e3 41 18 cc ^~~~~~~~~~~~~~~~~~~~~~~ invalid vptr May be related to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87095 This error doesn't manifest without -fno-sanitize-recover=all gcc -v Using built-in specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/9/lto-wrapper OFFLOAD_TARGET_NAMES=nvptx-none:hsa OFFLOAD_TARGET_DEFAULT=1 Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Ubuntu 9.2.1-9ubuntu2' --with-bugurl=file:///usr/share/doc/gcc-9/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++,gm2 --prefix=/usr --with-gcc-major-version-only --program-suffix=-9 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-plugin --enable-default-pie --with-system-zlib --with-target-system-zlib=auto --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none,hsa --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix gcc version 9.2.1 20191008 (Ubuntu 9.2.1-9ubuntu2)
Confirmed, I'm bisecting that.
Started with my r8-4602-g896f6b3dfa6fb337109f97bed8d74c1e030c965e. Anyway, a help from C++ maintainer is more than welcomed.
sizeof (MD) == sizeof (void *), so the clearing of the vptr in DD::~DD() when DE::~DE() is invoked later on looks wrong. But, without -fsanitize=vptr this isn't done. I've added that guard in PR87095, see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87095#c3 for more details. So, either we can just do what build_clobber_this does, which would be just: --- gcc/cp/decl.c 2020-03-27 09:59:26.407083563 +0100 +++ gcc/cp/decl.c 2020-04-06 13:14:15.814200485 +0200 @@ -16662,10 +16662,7 @@ begin_destructor_body (void) /* If the vptr is shared with some virtual nearly empty base, don't clear it if not in charge, the dtor of the virtual nearly empty base will do that later. */ - if (CLASSTYPE_VBASECLASSES (current_class_type) - && CLASSTYPE_PRIMARY_BINFO (current_class_type) - && BINFO_VIRTUAL_P - (CLASSTYPE_PRIMARY_BINFO (current_class_type))) + if (CLASSTYPE_VBASECLASSES (current_class_type)) { stmt = convert_to_void (stmt, ICV_STATEMENT, tf_warning_or_error); or try to narrow down the condition to cover this problematic case too, though I'm afraid I have no idea what that would be. Nathan/Jason, can I ask for help here? A wild shot in the dark could be: --- gcc/cp/decl.c.jj 2020-03-27 09:59:26.407083563 +0100 +++ gcc/cp/decl.c 2020-04-06 13:25:03.321511554 +0200 @@ -16662,14 +16662,20 @@ begin_destructor_body (void) /* If the vptr is shared with some virtual nearly empty base, don't clear it if not in charge, the dtor of the virtual nearly empty base will do that later. */ - if (CLASSTYPE_VBASECLASSES (current_class_type) - && CLASSTYPE_PRIMARY_BINFO (current_class_type) - && BINFO_VIRTUAL_P - (CLASSTYPE_PRIMARY_BINFO (current_class_type))) + if (CLASSTYPE_VBASECLASSES (current_class_type)) { - stmt = convert_to_void (stmt, ICV_STATEMENT, - tf_warning_or_error); - stmt = build_if_in_charge (stmt); + tree c = current_class_type; + while (CLASSTYPE_PRIMARY_BINFO (c)) + { + if (BINFO_VIRTUAL_P (CLASSTYPE_PRIMARY_BINFO (c))) + { + stmt = convert_to_void (stmt, ICV_STATEMENT, + tf_warning_or_error); + stmt = build_if_in_charge (stmt); + break; + } + c = BINFO_TYPE (CLASSTYPE_PRIMARY_BINFO (c)); + } } finish_decl_cleanup (NULL_TREE, stmt); } just from the fact that the existing code seemed to DTRT for D::~D() where CLASSTYPE_VBASECLASSES is non-NULL, CLASSTYPE_PRIMARY_BINFO too and BINFO_VIRTUAL_P on that is set as well. But in DD::~DD() the last condition doesn't hold, but BINFO_TYPE of the CLASSTYPE_PRIMARY_BINFO is D which has this property.
Created attachment 48210 [details] gcc10-pr94325.patch The shot in the dark in whole patch form.
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:4cf6b06cb5b02c053738e2975e3b7a4b3c577401 commit r10-7620-g4cf6b06cb5b02c053738e2975e3b7a4b3c577401 Author: Jakub Jelinek <jakub@redhat.com> Date: Wed Apr 8 15:30:16 2020 +0200 c++: Further fix for -fsanitize=vptr [PR94325] For -fsanitize=vptr, we insert a NULL store into the vptr instead of just adding a CLOBBER of this. build_clobber_this makes the CLOBBER conditional on in_charge (implicit) parameter whenever CLASSTYPE_VBASECLASSES, but when adding this conditionalization to the -fsanitize=vptr code in PR87095, I wanted it to catch some more cases when the class has CLASSTYPE_VBASECLASSES, but the vptr is still not shared with something else, otherwise the sanitization would be less effective. The following testcase shows that the chosen test that CLASSTYPE_PRIMARY_BINFO is non-NULL and has BINFO_VIRTUAL_P set wasn't sufficient, the D class has still sizeof(D) == sizeof(void*) and thus contains just a single vptr, but while in B::~B() this results in the vptr not being cleared, in C::~C() this condition isn't true, as CLASSTYPE_PRIMARY_BINFO in that case is B and is not BINFO_VIRTUAL_P, so it clears the vptr, but the D::~D() dtor after invoking C::~C() invokes A::~A() with an already cleared vptr, which is then reported. The following patch is just a shot in the dark, keep looking through CLASSTYPE_PRIMARY_BINFO until we find BINFO_VIRTUAL_P, but it works on the existing testcase as well as this new one. 2020-04-08 Jakub Jelinek <jakub@redhat.com> PR c++/94325 * decl.c (begin_destructor_body): For CLASSTYPE_VBASECLASSES class dtors, if CLASSTYPE_PRIMARY_BINFO is non-NULL, but not BINFO_VIRTUAL_P, look at CLASSTYPE_PRIMARY_BINFO of its BINFO_TYPE if it is not BINFO_VIRTUAL_P, and so on. * g++.dg/ubsan/vptr-15.C: New test.
Fixed on the trunk so far.
The releases/gcc-9 branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:175f052446556d32e887e1658a5a92c3c2f3a6f5 commit r9-8875-g175f052446556d32e887e1658a5a92c3c2f3a6f5 Author: Jakub Jelinek <jakub@redhat.com> Date: Wed Apr 8 15:30:16 2020 +0200 c++: Further fix for -fsanitize=vptr [PR94325] For -fsanitize=vptr, we insert a NULL store into the vptr instead of just adding a CLOBBER of this. build_clobber_this makes the CLOBBER conditional on in_charge (implicit) parameter whenever CLASSTYPE_VBASECLASSES, but when adding this conditionalization to the -fsanitize=vptr code in PR87095, I wanted it to catch some more cases when the class has CLASSTYPE_VBASECLASSES, but the vptr is still not shared with something else, otherwise the sanitization would be less effective. The following testcase shows that the chosen test that CLASSTYPE_PRIMARY_BINFO is non-NULL and has BINFO_VIRTUAL_P set wasn't sufficient, the D class has still sizeof(D) == sizeof(void*) and thus contains just a single vptr, but while in B::~B() this results in the vptr not being cleared, in C::~C() this condition isn't true, as CLASSTYPE_PRIMARY_BINFO in that case is B and is not BINFO_VIRTUAL_P, so it clears the vptr, but the D::~D() dtor after invoking C::~C() invokes A::~A() with an already cleared vptr, which is then reported. The following patch is just a shot in the dark, keep looking through CLASSTYPE_PRIMARY_BINFO until we find BINFO_VIRTUAL_P, but it works on the existing testcase as well as this new one. 2020-04-08 Jakub Jelinek <jakub@redhat.com> PR c++/94325 * decl.c (begin_destructor_body): For CLASSTYPE_VBASECLASSES class dtors, if CLASSTYPE_PRIMARY_BINFO is non-NULL, but not BINFO_VIRTUAL_P, look at CLASSTYPE_PRIMARY_BINFO of its BINFO_TYPE if it is not BINFO_VIRTUAL_P, and so on. * g++.dg/ubsan/vptr-15.C: New test. (cherry picked from commit 4cf6b06cb5b02c053738e2975e3b7a4b3c577401)
Fixed for 8.5 in r8-10484-g1298b488c37c44abf33cca6932e760ef69dd7815 and by the above commit for 9.4+ too.