rjmccall added a comment. In https://reviews.llvm.org/D25448#572245, @vsk wrote:
> Patch update: Pass along the type info of the derived class to the ubsan > runtime when we devirtualize a method call. This squashes the FP. I tested > this with 'check-ubsan' in addition to adding a lit test. > > > A pointer to the vtable pointer for Base1 is a pointer to a Derived. You > > have a multiple inheritance bug, or really a general inheritance bug. It's > > being covered up in the case of single, non-virtual inheritance because > > that's the case in which a pointer to a base-class object is the same as a > > pointer to the derived class object. > > I imagine that it would be difficult to extend the runtime to handle this > case. I.e, given a pointer to vtable pointer for Base1 and the type info for > Base2, recognize that we may _actually_ be looking at an instance of Derived, > and therefore claim that the types match. I don't know how the runtime parts of this check are implemented. It is certainly true that an object that is a Base1 might also be a Base2 due to multiple inheritance. But given a pointer that's statically typed to be a Base2, I believe you want to be checking not "am I pointing at an object whose dynamic type derives from Base2" but "am I pointing *at the Base2 subobject* of an object whose dynamic type derives from Base1", which is a very different check and doesn't necessarily require the same runtime complexity (or might need more, depending on what you're doing). But anyway, the pointer you're passing does not point to the Base2 subobject anymore, which is clearly a frontend bug, not a runtime bug. > I wonder if that would result in a false negative in this case: > > Base1 b1; > reinterpret_cast<Base2 *>(b1)->method_from_base2_only() > > > We are currently able to diagnose this. I can't imagine what on earth you would do in the runtime that would have a false-negative problem here without basically disabling the check, but let's leave that aside. >> ... it should also be changing its notion of what class the pointer points >> to. > > I'm taking this to mean that we should pass along the type information for > 'Derived'. This is also what @rsmith suggests. Yes, this is what you should do. Your runtime logic is, somehow, checking whether the memory contains an object of some type that you know statically. Devirt is potentially changing where your pointer points and thus also changing the expected static type of the memory you're pointing to. Properly responding to that change will not only fix the logic here, it will strengthen your check so that you also detect the bug here: Base1 p; Derived *badp = static_cast<Derived*>(&p); static_cast<Base1*>(badp)->method_that_is_final_in_derived(); https://reviews.llvm.org/D25448 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
