Skip to content

Commit

Permalink
-fsanitize=vptr warnings on bad static types in dynamic_cast and typeid
Browse files Browse the repository at this point in the history
...when such an operation is done on an object during con-/destruction.

This is the cfe part of a patch covering both cfe and compiler-rt.

Differential Revision: https://reviews.llvm.org/D40295

llvm-svn: 321519
  • Loading branch information
stbergmann committed Dec 28, 2017
1 parent 703478a commit d71ad17
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 8 deletions.
4 changes: 2 additions & 2 deletions clang/lib/CodeGen/CGExpr.cpp
Expand Up @@ -570,15 +570,15 @@ static llvm::Value *emitHash16Bytes(CGBuilderTy &Builder, llvm::Value *Low,

bool CodeGenFunction::isNullPointerAllowed(TypeCheckKind TCK) {
return TCK == TCK_DowncastPointer || TCK == TCK_Upcast ||
TCK == TCK_UpcastToVirtualBase;
TCK == TCK_UpcastToVirtualBase || TCK == TCK_DynamicOperation;
}

bool CodeGenFunction::isVptrCheckRequired(TypeCheckKind TCK, QualType Ty) {
CXXRecordDecl *RD = Ty->getAsCXXRecordDecl();
return (RD && RD->hasDefinition() && RD->isDynamicClass()) &&
(TCK == TCK_MemberAccess || TCK == TCK_MemberCall ||
TCK == TCK_DowncastPointer || TCK == TCK_DowncastReference ||
TCK == TCK_UpcastToVirtualBase);
TCK == TCK_UpcastToVirtualBase || TCK == TCK_DynamicOperation);
}

bool CodeGenFunction::sanitizePerformTypeCheck() const {
Expand Down
26 changes: 21 additions & 5 deletions clang/lib/CodeGen/CGExprCXX.cpp
Expand Up @@ -2056,6 +2056,15 @@ static llvm::Value *EmitTypeidFromVTable(CodeGenFunction &CGF, const Expr *E,
// Get the vtable pointer.
Address ThisPtr = CGF.EmitLValue(E).getAddress();

QualType SrcRecordTy = E->getType();

// C++ [class.cdtor]p4:
// If the operand of typeid refers to the object under construction or
// destruction and the static type of the operand is neither the constructor
// or destructor’s class nor one of its bases, the behavior is undefined.
CGF.EmitTypeCheck(CodeGenFunction::TCK_DynamicOperation, E->getExprLoc(),
ThisPtr.getPointer(), SrcRecordTy);

// C++ [expr.typeid]p2:
// If the glvalue expression is obtained by applying the unary * operator to
// a pointer and the pointer is a null pointer value, the typeid expression
Expand All @@ -2064,7 +2073,6 @@ static llvm::Value *EmitTypeidFromVTable(CodeGenFunction &CGF, const Expr *E,
// However, this paragraph's intent is not clear. We choose a very generous
// interpretation which implores us to consider comma operators, conditional
// operators, parentheses and other such constructs.
QualType SrcRecordTy = E->getType();
if (CGF.CGM.getCXXABI().shouldTypeidBeNullChecked(
isGLValueFromPointerDeref(E), SrcRecordTy)) {
llvm::BasicBlock *BadTypeidBlock =
Expand Down Expand Up @@ -2127,10 +2135,6 @@ llvm::Value *CodeGenFunction::EmitDynamicCast(Address ThisAddr,
CGM.EmitExplicitCastExprType(DCE, this);
QualType DestTy = DCE->getTypeAsWritten();

if (DCE->isAlwaysNull())
if (llvm::Value *T = EmitDynamicCastToNull(*this, DestTy))
return T;

QualType SrcTy = DCE->getSubExpr()->getType();

// C++ [expr.dynamic.cast]p7:
Expand All @@ -2151,6 +2155,18 @@ llvm::Value *CodeGenFunction::EmitDynamicCast(Address ThisAddr,
DestRecordTy = DestTy->castAs<ReferenceType>()->getPointeeType();
}

// C++ [class.cdtor]p5:
// If the operand of the dynamic_cast refers to the object under
// construction or destruction and the static type of the operand is not a
// pointer to or object of the constructor or destructor’s own class or one
// of its bases, the dynamic_cast results in undefined behavior.
EmitTypeCheck(TCK_DynamicOperation, DCE->getExprLoc(), ThisAddr.getPointer(),
SrcRecordTy);

if (DCE->isAlwaysNull())
if (llvm::Value *T = EmitDynamicCastToNull(*this, DestTy))
return T;

assert(SrcRecordTy->isRecordType() && "source type must be a record type!");

// C++ [expr.dynamic.cast]p4:
Expand Down
5 changes: 4 additions & 1 deletion clang/lib/CodeGen/CodeGenFunction.h
Expand Up @@ -2371,7 +2371,10 @@ class CodeGenFunction : public CodeGenTypeCache {
/// object within its lifetime.
TCK_UpcastToVirtualBase,
/// Checking the value assigned to a _Nonnull pointer. Must not be null.
TCK_NonnullAssign
TCK_NonnullAssign,
/// Checking the operand of a dynamic_cast or a typeid expression. Must be
/// null or an object within its lifetime.
TCK_DynamicOperation
};

/// Determine whether the pointer type check \p TCK permits null pointers.
Expand Down
12 changes: 12 additions & 0 deletions clang/test/CodeGenCXX/ubsan-vtable-checks.cpp
Expand Up @@ -38,3 +38,15 @@ void delete_it(T *t) {
// CHECK-VPTR: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
delete t;
}

// ITANIUM: define %struct.U* @_Z7dyncastP1T
// MSABI: define %struct.U* @"\01?dyncast
U* dyncast(T *t) {
// First, we check that dynamic_cast is not called before a type check.
// CHECK-VPTR-NOT: call i8* @__{{dynamic_cast|RTDynamicCast}}
// CHECK-VPTR: br i1 {{.*}} label %{{.*}}
// CHECK-VPTR: call void @__ubsan_handle_dynamic_type_cache_miss_abort
// Second, we check that dynamic_cast is actually called once the type check is done.
// CHECK-VPTR: call i8* @__{{dynamic_cast|RTDynamicCast}}
return dynamic_cast<U*>(t);
}

0 comments on commit d71ad17

Please sign in to comment.