From cda80833090be644d6a44823e22d39f2fbe53a39 Mon Sep 17 00:00:00 2001 From: John McCall Date: Fri, 22 Mar 2013 02:58:14 +0000 Subject: [PATCH] Warn about attempts to reinterpret_cast between two types that are hierarchy-related at a possibly nonzero offset. Patch by Alexander Zinenko! llvm-svn: 177698 --- clang/include/clang/Basic/DiagnosticGroups.td | 1 + clang/include/clang/Basic/DiagnosticSemaKinds.td | 8 + clang/lib/Sema/SemaCast.cpp | 89 +++++++- clang/test/SemaCXX/warn-reinterpret-base-class.cpp | 234 +++++++++++++++++++++ 4 files changed, 330 insertions(+), 2 deletions(-) create mode 100644 clang/test/SemaCXX/warn-reinterpret-base-class.cpp diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 0ff0460..1238e4b 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -266,6 +266,7 @@ def Trigraphs : DiagGroup<"trigraphs">; def : DiagGroup<"type-limits">; def UndefinedReinterpretCast : DiagGroup<"undefined-reinterpret-cast">; +def ReinterpretBaseClass : DiagGroup<"reinterpret-base-class">; def Unicode : DiagGroup<"unicode">; def UninitializedMaybe : DiagGroup<"conditional-uninitialized">; def UninitializedSometimes : DiagGroup<"sometimes-uninitialized">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 3c067cd..ec06e34 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -4491,6 +4491,14 @@ def note_parameter_here : Note< def err_bad_reinterpret_cast_overload : Error< "reinterpret_cast cannot resolve overloaded function %0 to type %1">; +def warn_reinterpret_different_from_static : Warning< + "'reinterpret_cast' %select{from|to}3 class %0 %select{to|from}3 its " + "%select{virtual base|base at non-zero offset}2 %1 behaves differently from " + "'static_cast'">, InGroup; +def note_reinterpret_updowncast_use_static: Note< + "use 'static_cast' to adjust the pointer correctly while " + "%select{upcasting|downcasting}0">; + def err_bad_static_cast_overload : Error< "address of overloaded function %0 cannot be static_cast to type %1">; diff --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp index 3f46cd4..1dbc9aa 100644 --- a/clang/lib/Sema/SemaCast.cpp +++ b/clang/lib/Sema/SemaCast.cpp @@ -19,6 +19,7 @@ #include "clang/AST/CXXInheritance.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/ExprObjC.h" +#include "clang/AST/RecordLayout.h" #include "clang/Basic/PartialDiagnostic.h" #include "clang/Sema/Initialization.h" #include "llvm/ADT/SmallVector.h" @@ -682,6 +683,88 @@ void CastOperation::CheckConstCast() { << SrcExpr.get()->getType() << DestType << OpRange; } +/// Check that a reinterpret_cast\(SrcExpr) is not used as upcast +/// or downcast between respective pointers or references. +static void DiagnoseReinterpretUpDownCast(Sema &Self, const Expr *SrcExpr, + QualType DestType, + SourceRange OpRange) { + QualType SrcType = SrcExpr->getType(); + // When casting from pointer or reference, get pointee type; use original + // type otherwise. + const CXXRecordDecl *SrcPointeeRD = SrcType->getPointeeCXXRecordDecl(); + const CXXRecordDecl *SrcRD = + SrcPointeeRD ? SrcPointeeRD : SrcType->getAsCXXRecordDecl(); + + // Examining subobjects for records is only possible if the complete + // definition is available. Also, template instantiation is not allowed here. + if(!SrcRD || !SrcRD->isCompleteDefinition()) + return; + + const CXXRecordDecl *DestRD = DestType->getPointeeCXXRecordDecl(); + + if(!DestRD || !DestRD->isCompleteDefinition()) + return; + + enum { + ReinterpretUpcast, + ReinterpretDowncast + } ReinterpretKind; + + CXXBasePaths BasePaths; + + if (SrcRD->isDerivedFrom(DestRD, BasePaths)) + ReinterpretKind = ReinterpretUpcast; + else if (DestRD->isDerivedFrom(SrcRD, BasePaths)) + ReinterpretKind = ReinterpretDowncast; + else + return; + + bool VirtualBase = true; + bool NonZeroOffset = false; + for (CXXBasePaths::const_paths_iterator I = BasePaths.begin(), + E = BasePaths.end(); + I != E; ++I) { + const CXXBasePath &Path = *I; + CharUnits Offset = CharUnits::Zero(); + bool IsVirtual = false; + for (CXXBasePath::const_iterator IElem = Path.begin(), EElem = Path.end(); + IElem != EElem; ++IElem) { + IsVirtual = IElem->Base->isVirtual(); + if (IsVirtual) + break; + const CXXRecordDecl *BaseRD = IElem->Base->getType()->getAsCXXRecordDecl(); + assert(BaseRD && "Base type should be a valid unqualified class type"); + const ASTRecordLayout &DerivedLayout = + Self.Context.getASTRecordLayout(IElem->Class); + Offset += DerivedLayout.getBaseClassOffset(BaseRD); + } + if (!IsVirtual) { + // Don't warn if any path is a non-virtually derived base at offset zero. + if (Offset.isZero()) + return; + // Offset makes sense only for non-virtual bases. + else + NonZeroOffset = true; + } + VirtualBase = VirtualBase && IsVirtual; + } + + assert((VirtualBase || NonZeroOffset) && + "Should have returned if has non-virtual base with zero offset"); + + QualType BaseType = + ReinterpretKind == ReinterpretUpcast? DestType : SrcType; + QualType DerivedType = + ReinterpretKind == ReinterpretUpcast? SrcType : DestType; + + Self.Diag(OpRange.getBegin(), diag::warn_reinterpret_different_from_static) + << DerivedType << BaseType << !VirtualBase << ReinterpretKind; + Self.Diag(OpRange.getBegin(), diag::note_reinterpret_updowncast_use_static) + << ReinterpretKind; + + // TODO: emit fixits. This requires passing operator SourceRange from Parser. +} + /// CheckReinterpretCast - Check that a reinterpret_cast\(SrcExpr) is /// valid. /// Refer to C++ 5.2.10 for details. reinterpret_cast is typically used in code @@ -714,8 +797,10 @@ void CastOperation::CheckReinterpretCast() { diagnoseBadCast(Self, msg, CT_Reinterpret, OpRange, SrcExpr.get(), DestType, /*listInitialization=*/false); } - } else if (tcr == TC_Success && Self.getLangOpts().ObjCAutoRefCount) { - checkObjCARCConversion(Sema::CCK_OtherCast); + } else if (tcr == TC_Success) { + if (Self.getLangOpts().ObjCAutoRefCount) + checkObjCARCConversion(Sema::CCK_OtherCast); + DiagnoseReinterpretUpDownCast(Self, SrcExpr.get(), DestType, OpRange); } } diff --git a/clang/test/SemaCXX/warn-reinterpret-base-class.cpp b/clang/test/SemaCXX/warn-reinterpret-base-class.cpp new file mode 100644 index 0000000..fc7d15c --- /dev/null +++ b/clang/test/SemaCXX/warn-reinterpret-base-class.cpp @@ -0,0 +1,234 @@ +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -Wreinterpret-base-class -Wno-unused-volatile-lvalue %s + +// PR 13824 +class A { +}; +class DA : public A { +}; +class DDA : public DA { +}; +class DAo : protected A { +}; +class DAi : private A { +}; + +class DVA : public virtual A { +}; +class DDVA : public virtual DA { +}; +class DMA : public virtual A, public virtual DA { +}; + +class B; + +struct C { + // Do not fail on incompletely-defined classes. + decltype(reinterpret_cast(0)) foo; + decltype(reinterpret_cast((C *) 0)) bar; + decltype(reinterpret_cast((A *) 0)) baz; +}; + +void reinterpret_not_defined_class(B *b, C *c) { + // Should not fail if class has no definition. + (void)*reinterpret_cast(b); + (void)*reinterpret_cast(c); + + (void)reinterpret_cast(*b); + (void)reinterpret_cast(*c); +} + +void reinterpret_not_updowncast(A *pa, const A *pca, A &a, const A &ca) { + (void)*reinterpret_cast(pa); + (void)*reinterpret_cast(pa); + (void)*reinterpret_cast(pa); + (void)*reinterpret_cast(pa); + + (void)*reinterpret_cast(pca); + (void)*reinterpret_cast(pca); + + (void)reinterpret_cast(a); + (void)reinterpret_cast(a); + (void)reinterpret_cast(a); + (void)reinterpret_cast(a); + + (void)reinterpret_cast(ca); + (void)reinterpret_cast(ca); +} + +void reinterpret_pointer_downcast(A *a, const A *ca) { + (void)*reinterpret_cast(a); + (void)*reinterpret_cast(a); + (void)*reinterpret_cast(a); + (void)*reinterpret_cast(a); + + (void)*reinterpret_cast(ca); + (void)*reinterpret_cast(ca); + + (void)*reinterpret_cast(a); + (void)*reinterpret_cast(a); + (void)*reinterpret_cast(a); + // expected-warning@+2 {{'reinterpret_cast' to class 'DVA *' from its virtual base 'A *' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while downcasting}} + (void)*reinterpret_cast(a); + // expected-warning@+2 {{'reinterpret_cast' to class 'DDVA *' from its virtual base 'A *' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while downcasting}} + (void)*reinterpret_cast(a); + // expected-warning@+2 {{'reinterpret_cast' to class 'DMA *' from its virtual base 'A *' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while downcasting}} + (void)*reinterpret_cast(a); +} + +void reinterpret_reference_downcast(A a, A &ra, const A &cra) { + (void)reinterpret_cast(a); + (void)reinterpret_cast(a); + (void)reinterpret_cast(a); + (void)reinterpret_cast(a); + + (void)reinterpret_cast(ra); + (void)reinterpret_cast(ra); + (void)reinterpret_cast(ra); + (void)reinterpret_cast(ra); + + (void)reinterpret_cast(cra); + (void)reinterpret_cast(cra); + + (void)reinterpret_cast(a); + (void)reinterpret_cast(a); + (void)reinterpret_cast(a); + // expected-warning@+2 {{'reinterpret_cast' to class 'DVA &' from its virtual base 'A' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while downcasting}} + (void)reinterpret_cast(a); + // expected-warning@+2 {{'reinterpret_cast' to class 'DDVA &' from its virtual base 'A' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while downcasting}} + (void)reinterpret_cast(a); + // expected-warning@+2 {{'reinterpret_cast' to class 'DMA &' from its virtual base 'A' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while downcasting}} + (void)reinterpret_cast(a); +} + +void reinterpret_pointer_upcast(DA *da, const DA *cda, DDA *dda, DAo *dao, + DAi *dai, DVA *dva, DDVA *ddva, DMA *dma) { + (void)*reinterpret_cast(da); + (void)*reinterpret_cast(da); + (void)*reinterpret_cast(da); + (void)*reinterpret_cast(da); + + (void)*reinterpret_cast(cda); + (void)*reinterpret_cast(cda); + + (void)*reinterpret_cast(dda); + (void)*reinterpret_cast(dda); + (void)*reinterpret_cast(dao); + (void)*reinterpret_cast(dai); + // expected-warning@+2 {{'reinterpret_cast' from class 'DVA *' to its virtual base 'A *' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while upcasting}} + (void)*reinterpret_cast(dva); + // expected-warning@+2 {{'reinterpret_cast' from class 'DDVA *' to its virtual base 'A *' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while upcasting}} + (void)*reinterpret_cast(ddva); + // expected-warning@+2 {{'reinterpret_cast' from class 'DDVA *' to its virtual base 'DA *' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while upcasting}} + (void)*reinterpret_cast(ddva); + // expected-warning@+2 {{'reinterpret_cast' from class 'DMA *' to its virtual base 'A *' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while upcasting}} + (void)*reinterpret_cast(dma); + // expected-warning@+2 {{'reinterpret_cast' from class 'DMA *' to its virtual base 'DA *' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while upcasting}} + (void)*reinterpret_cast(dma); +} + +void reinterpret_reference_upcast(DA &da, const DA &cda, DDA &dda, DAo &dao, + DAi &dai, DVA &dva, DDVA &ddva, DMA &dma) { + (void)reinterpret_cast(da); + (void)reinterpret_cast(da); + (void)reinterpret_cast(da); + (void)reinterpret_cast(da); + + (void)reinterpret_cast(cda); + (void)reinterpret_cast(cda); + + (void)reinterpret_cast(dda); + (void)reinterpret_cast(dda); + (void)reinterpret_cast(dao); + (void)reinterpret_cast(dai); + // expected-warning@+2 {{'reinterpret_cast' from class 'DVA' to its virtual base 'A &' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while upcasting}} + (void)reinterpret_cast(dva); + // expected-warning@+2 {{'reinterpret_cast' from class 'DDVA' to its virtual base 'A &' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while upcasting}} + (void)reinterpret_cast(ddva); + // expected-warning@+2 {{'reinterpret_cast' from class 'DDVA' to its virtual base 'DA &' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while upcasting}} + (void)reinterpret_cast(ddva); + // expected-warning@+2 {{'reinterpret_cast' from class 'DMA' to its virtual base 'A &' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while upcasting}} + (void)reinterpret_cast(dma); + // expected-warning@+2 {{'reinterpret_cast' from class 'DMA' to its virtual base 'DA &' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while upcasting}} + (void)reinterpret_cast(dma); +} + +struct E { + int x; +}; + +class F : public E { + virtual int foo() { return x; } +}; + +class G : public F { +}; + +class H : public E, public A { +}; + +class I : virtual public F { +}; + +typedef const F * K; +typedef volatile K L; + +void different_subobject_downcast(E *e, F *f, A *a) { + // expected-warning@+2 {{'reinterpret_cast' to class 'F *' from its base at non-zero offset 'E *' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while downcasting}} + (void)reinterpret_cast(e); + // expected-warning@+2 {{'reinterpret_cast' to class 'G *' from its base at non-zero offset 'E *' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while downcasting}} + (void)reinterpret_cast(e); + (void)reinterpret_cast(e); + // expected-warning@+2 {{'reinterpret_cast' to class 'I *' from its virtual base 'E *' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while downcasting}} + (void)reinterpret_cast(e); + + (void)reinterpret_cast(f); + // expected-warning@+2 {{'reinterpret_cast' to class 'I *' from its virtual base 'F *' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while downcasting}} + (void)reinterpret_cast(f); + + (void)reinterpret_cast(a); + + // expected-warning@+2 {{'reinterpret_cast' to class 'L' (aka 'const F *volatile') from its base at non-zero offset 'E *' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while downcasting}} + (void)reinterpret_cast(e); +} + +void different_subobject_upcast(F *f, G *g, H *h, I *i) { + // expected-warning@+2 {{'reinterpret_cast' from class 'F *' to its base at non-zero offset 'E *' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while upcasting}} + (void)reinterpret_cast(f); + + (void)reinterpret_cast(g); + // expected-warning@+2 {{'reinterpret_cast' from class 'G *' to its base at non-zero offset 'E *' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while upcasting}} + (void)reinterpret_cast(g); + + (void)reinterpret_cast(h); + (void)reinterpret_cast(h); + + // expected-warning@+2 {{'reinterpret_cast' from class 'I *' to its virtual base 'F *' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while upcasting}} + (void)reinterpret_cast(i); + // expected-warning@+2 {{'reinterpret_cast' from class 'I *' to its virtual base 'E *' behaves differently from 'static_cast'}} + // expected-note@+1 {{use 'static_cast' to adjust the pointer correctly while upcasting}} + (void)reinterpret_cast(i); +} -- 2.7.4