From: John McCall Date: Mon, 1 Feb 2010 03:16:54 +0000 (+0000) Subject: Access checking for implicit user-defined conversions. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b13b737a2450167c82e148590e8019b839ce6b98;p=clang Access checking for implicit user-defined conversions. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@94971 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/UnresolvedSet.h b/include/clang/AST/UnresolvedSet.h index 8bf7c11e6f..9c59229e31 100644 --- a/include/clang/AST/UnresolvedSet.h +++ b/include/clang/AST/UnresolvedSet.h @@ -16,7 +16,6 @@ #define LLVM_CLANG_AST_UNRESOLVEDSET_H #include -#include "llvm/ADT/PointerIntPair.h" #include "llvm/ADT/SmallVector.h" #include "clang/Basic/Specifiers.h" @@ -24,12 +23,58 @@ namespace clang { class NamedDecl; +/// A POD class for pairing a NamedDecl* with an access specifier. +/// Can be put into unions. +class DeclAccessPair { + NamedDecl *Ptr; // we'd use llvm::PointerUnion, but it isn't trivial + + enum { Mask = 0x3 }; + +public: + static DeclAccessPair make(NamedDecl *D, AccessSpecifier AS) { + DeclAccessPair p; + p.set(D, AS); + return p; + } + + NamedDecl *getDecl() const { + return (NamedDecl*) (~Mask & (uintptr_t) Ptr); + } + AccessSpecifier getAccess() const { + return AccessSpecifier(Mask & (uintptr_t) Ptr); + } + + void setDecl(NamedDecl *D) { + set(D, getAccess()); + } + void setAccess(AccessSpecifier AS) { + set(getDecl(), AS); + } + void set(NamedDecl *D, AccessSpecifier AS) { + Ptr = reinterpret_cast(uintptr_t(AS) | + reinterpret_cast(D)); + } + + operator NamedDecl*() const { return getDecl(); } + NamedDecl *operator->() const { return getDecl(); } +}; +} + +// Take a moment to tell SmallVector that this is POD. +namespace llvm { +template struct isPodLike; +template<> struct isPodLike { + static const bool value = true; +}; +} + +namespace clang { + /// The iterator over UnresolvedSets. Serves as both the const and /// non-const iterator. class UnresolvedSetIterator { - - typedef llvm::PointerIntPair DeclEntry; - typedef llvm::SmallVectorImpl DeclsTy; +private: + typedef llvm::SmallVectorImpl DeclsTy; typedef DeclsTy::iterator IteratorTy; IteratorTy ir; @@ -47,8 +92,8 @@ public: typedef NamedDecl *reference; typedef std::iterator_traits::iterator_category iterator_category; - NamedDecl *getDecl() const { return ir->getPointer(); } - AccessSpecifier getAccess() const { return AccessSpecifier(ir->getInt()); } + NamedDecl *getDecl() const { return ir->getDecl(); } + AccessSpecifier getAccess() const { return ir->getAccess(); } NamedDecl *operator*() const { return getDecl(); } @@ -87,7 +132,6 @@ public: /// in a lot of places, but isn't really worth breaking into its own /// header right now. class UnresolvedSetImpl { - typedef UnresolvedSetIterator::DeclEntry DeclEntry; typedef UnresolvedSetIterator::DeclsTy DeclsTy; // Don't allow direct construction, and only permit subclassing by @@ -114,7 +158,7 @@ public: } void addDecl(NamedDecl *D, AccessSpecifier AS) { - decls().push_back(DeclEntry(D, AS)); + decls().push_back(DeclAccessPair::make(D, AS)); } /// Replaces the given declaration with the new one, once. @@ -122,19 +166,19 @@ public: /// \return true if the set changed bool replace(const NamedDecl* Old, NamedDecl *New) { for (DeclsTy::iterator I = decls().begin(), E = decls().end(); I != E; ++I) - if (I->getPointer() == Old) - return (I->setPointer(New), true); + if (I->getDecl() == Old) + return (I->setDecl(New), true); return false; } /// Replaces the declaration at the given iterator with the new one, /// preserving the original access bits. void replace(iterator I, NamedDecl *New) { - I.ir->setPointer(New); + I.ir->setDecl(New); } void replace(iterator I, NamedDecl *New, AccessSpecifier AS) { - *I.ir = DeclEntry(New, AS); + I.ir->set(New, AS); } void erase(unsigned I) { @@ -148,7 +192,7 @@ public: } void setAccess(iterator I, AccessSpecifier AS) { - I.ir->setInt(AS); + I.ir->setAccess(AS); } void clear() { decls().clear(); } @@ -161,41 +205,8 @@ public: decls().append(I.ir, E.ir); } - /// A proxy reference for implementing operator[]. - class Proxy { - DeclEntry &Ref; - - friend class UnresolvedSetImpl; - Proxy(DeclEntry &Ref) : Ref(Ref) {} - - public: - NamedDecl *getDecl() const { return Ref.getPointer(); } - void setDecl(NamedDecl *D) { Ref.setPointer(D); } - - AccessSpecifier getAccess() const { return AccessSpecifier(Ref.getInt()); } - void setAccess(AccessSpecifier AS) const { Ref.setInt(AS); } - - NamedDecl* operator->() const { return getDecl(); } - operator NamedDecl*() const { return getDecl(); } - Proxy &operator=(const Proxy &D) { Ref = D.Ref; return *this; } - }; - Proxy operator[](unsigned I) { return Proxy(decls()[I]); } - - /// A proxy reference for implementing operator[] const. - class ConstProxy { - const DeclEntry &Ref; - - friend class UnresolvedSetImpl; - ConstProxy(const DeclEntry &Ref) : Ref(Ref) {} - - public: - NamedDecl *getDecl() const { return Ref.getPointer(); } - AccessSpecifier getAccess() const { return AccessSpecifier(Ref.getInt()); } - - NamedDecl *operator->() const { return getDecl(); } - operator NamedDecl*() const { return getDecl(); } - }; - ConstProxy operator[](unsigned I) const { return ConstProxy(decls()[I]); } + DeclAccessPair &operator[](unsigned I) { return decls()[I]; } + const DeclAccessPair &operator[](unsigned I) const { return decls()[I]; } private: // These work because the only permitted subclass is UnresolvedSetImpl @@ -211,7 +222,7 @@ private: /// A set of unresolved declarations template class UnresolvedSet : public UnresolvedSetImpl { - llvm::SmallVector Decls; + llvm::SmallVector Decls; }; diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h index 54d36357be..1595941651 100644 --- a/lib/Sema/Sema.h +++ b/lib/Sema/Sema.h @@ -2415,6 +2415,8 @@ public: bool CheckUnresolvedLookupAccess(UnresolvedLookupExpr *E, NamedDecl *D, AccessSpecifier Access); + bool CheckConstructorAccess(SourceLocation Loc, CXXConstructorDecl *D, + AccessSpecifier Access); bool CheckMemberOperatorAccess(SourceLocation Loc, Expr *ObjectExpr, NamedDecl *D, AccessSpecifier Access); bool CheckAccess(const LookupResult &R, NamedDecl *D, AccessSpecifier Access); diff --git a/lib/Sema/SemaAccess.cpp b/lib/Sema/SemaAccess.cpp index 34c9718014..ceee61df23 100644 --- a/lib/Sema/SemaAccess.cpp +++ b/lib/Sema/SemaAccess.cpp @@ -306,7 +306,24 @@ bool Sema::CheckUnresolvedMemberAccess(UnresolvedMemberExpr *E, return false; } -/// Checks access to an overloaded member operator. +/// Checks access to a constructor. +bool Sema::CheckConstructorAccess(SourceLocation UseLoc, + CXXConstructorDecl *Constructor, + AccessSpecifier Access) { + if (!getLangOptions().AccessControl) + return false; + + CXXRecordDecl *NamingClass = cast(Constructor->getParent()); + + LookupResult R(*this, Constructor->getDeclName(), UseLoc, LookupOrdinaryName); + R.suppressDiagnostics(); + + R.setNamingClass(NamingClass); + return CheckAccess(R, Constructor, Access); +} + +/// Checks access to an overloaded member operator, including +/// conversion operators. bool Sema::CheckMemberOperatorAccess(SourceLocation OpLoc, Expr *ObjectExpr, NamedDecl *MemberOperator, diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp index db888a62a4..4f23751e73 100644 --- a/lib/Sema/SemaInit.cpp +++ b/lib/Sema/SemaInit.cpp @@ -1971,7 +1971,8 @@ void InitializationSequence::AddAddressOverloadResolutionStep( Step S; S.Kind = SK_ResolveAddressOfOverloadedFunction; S.Type = Function->getType(); - S.Function = Function; + // Access is currently ignored for these. + S.Function = DeclAccessPair::make(Function, AccessSpecifier(0)); Steps.push_back(S); } @@ -1992,11 +1993,12 @@ void InitializationSequence::AddReferenceBindingStep(QualType T, } void InitializationSequence::AddUserConversionStep(FunctionDecl *Function, + AccessSpecifier Access, QualType T) { Step S; S.Kind = SK_UserConversion; S.Type = T; - S.Function = Function; + S.Function = DeclAccessPair::make(Function, Access); Steps.push_back(S); } @@ -2029,11 +2031,12 @@ void InitializationSequence::AddListInitializationStep(QualType T) { void InitializationSequence::AddConstructorInitializationStep( CXXConstructorDecl *Constructor, + AccessSpecifier Access, QualType T) { Step S; S.Kind = SK_ConstructorInitialization; S.Type = T; - S.Function = Constructor; + S.Function = DeclAccessPair::make(Constructor, Access); Steps.push_back(S); } @@ -2246,7 +2249,8 @@ static OverloadingResult TryRefInitWithConversionFunction(Sema &S, T2 = cv1T1; // Add the user-defined conversion step. - Sequence.AddUserConversionStep(Function, T2.getNonReferenceType()); + Sequence.AddUserConversionStep(Function, Best->getAccess(), + T2.getNonReferenceType()); // Determine whether we need to perform derived-to-base or // cv-qualification adjustments. @@ -2577,10 +2581,11 @@ static void TryConstructorInitialization(Sema &S, // Add the constructor initialization step. Any cv-qualification conversion is // subsumed by the initialization. if (Kind.getKind() == InitializationKind::IK_Copy) { - Sequence.AddUserConversionStep(Best->Function, DestType); + Sequence.AddUserConversionStep(Best->Function, Best->getAccess(), DestType); } else { Sequence.AddConstructorInitializationStep( cast(Best->Function), + Best->getAccess(), DestType); } } @@ -2778,13 +2783,13 @@ static void TryUserDefinedConversion(Sema &S, if (isa(Function)) { // Add the user-defined conversion step. Any cv-qualification conversion is // subsumed by the initialization. - Sequence.AddUserConversionStep(Function, DestType); + Sequence.AddUserConversionStep(Function, Best->getAccess(), DestType); return; } // Add the user-defined conversion step that calls the conversion function. QualType ConvType = Function->getResultType().getNonReferenceType(); - Sequence.AddUserConversionStep(Function, ConvType); + Sequence.AddUserConversionStep(Function, Best->getAccess(), ConvType); // If the conversion following the call to the conversion function is // interesting, add it as a separate step. @@ -3250,7 +3255,9 @@ InitializationSequence::Perform(Sema &S, case SK_ResolveAddressOfOverloadedFunction: // Overload resolution determined which function invoke; update the // initializer to reflect that choice. - CurInit = S.FixOverloadedFunctionReference(move(CurInit), Step->Function); + // Access control was done in overload resolution. + CurInit = S.FixOverloadedFunctionReference(move(CurInit), + cast(Step->Function.getDecl())); break; case SK_CastDerivedToBaseRValue: @@ -3329,13 +3336,14 @@ InitializationSequence::Perform(Sema &S, // or a conversion function. CastExpr::CastKind CastKind = CastExpr::CK_Unknown; bool IsCopy = false; - if (CXXConstructorDecl *Constructor - = dyn_cast(Step->Function)) { + FunctionDecl *Fn = cast(Step->Function.getDecl()); + AccessSpecifier FnAccess = Step->Function.getAccess(); + if (CXXConstructorDecl *Constructor = dyn_cast(Fn)) { // Build a call to the selected constructor. ASTOwningVector<&ActionBase::DeleteExpr> ConstructorArgs(S); SourceLocation Loc = CurInitExpr->getLocStart(); CurInit.release(); // Ownership transferred into MultiExprArg, below. - + // Determine the arguments required to actually perform the constructor // call. if (S.CompleteConstructorCall(Constructor, @@ -3350,6 +3358,8 @@ InitializationSequence::Perform(Sema &S, move_arg(ConstructorArgs)); if (CurInit.isInvalid()) return S.ExprError(); + + S.CheckConstructorAccess(Kind.getLocation(), Constructor, FnAccess); CastKind = CastExpr::CK_ConstructorConversion; QualType Class = S.Context.getTypeDeclType(Constructor->getParent()); @@ -3358,8 +3368,11 @@ InitializationSequence::Perform(Sema &S, IsCopy = true; } else { // Build a call to the conversion function. - CXXConversionDecl *Conversion = cast(Step->Function); + CXXConversionDecl *Conversion = cast(Fn); + S.CheckMemberOperatorAccess(Kind.getLocation(), CurInitExpr, + Conversion, FnAccess); + // FIXME: Should we move this initialization into a separate // derived-to-base conversion? I believe the answer is "no", because // we don't want to turn off access control here for c-style casts. @@ -3424,8 +3437,8 @@ InitializationSequence::Perform(Sema &S, case SK_ConstructorInitialization: { CXXConstructorDecl *Constructor - = cast(Step->Function); - + = cast(Step->Function.getDecl()); + // Build a call to the selected constructor. ASTOwningVector<&ActionBase::DeleteExpr> ConstructorArgs(S); SourceLocation Loc = Kind.getLocation(); @@ -3444,6 +3457,9 @@ InitializationSequence::Perform(Sema &S, Entity.getKind() == InitializedEntity::EK_Base); if (CurInit.isInvalid()) return S.ExprError(); + + // Only check access if all of that succeeded. + S.CheckConstructorAccess(Loc, Constructor, Step->Function.getAccess()); bool Elidable = cast((Expr *)CurInit.get())->isElidable(); diff --git a/lib/Sema/SemaInit.h b/lib/Sema/SemaInit.h index 001ba91d54..2b49df28fe 100644 --- a/lib/Sema/SemaInit.h +++ b/lib/Sema/SemaInit.h @@ -15,6 +15,7 @@ #include "SemaOverload.h" #include "clang/AST/Type.h" +#include "clang/AST/UnresolvedSet.h" #include "clang/Parse/Action.h" #include "clang/Basic/SourceLocation.h" #include "llvm/ADT/PointerIntPair.h" @@ -449,7 +450,11 @@ public: /// \brief When Kind == SK_ResolvedOverloadedFunction or Kind == /// SK_UserConversion, the function that the expression should be /// resolved to or the conversion function to call, respectively. - FunctionDecl *Function; + /// + /// Always a FunctionDecl. + /// For conversion decls, the naming class is the source type. + /// For construct decls, the naming class is the target type. + DeclAccessPair Function; /// \brief When Kind = SK_ConversionSequence, the implicit conversion /// sequence @@ -616,7 +621,9 @@ public: /// \brief Add a new step invoking a conversion function, which is either /// a constructor or a conversion function. - void AddUserConversionStep(FunctionDecl *Function, QualType T); + void AddUserConversionStep(FunctionDecl *Function, + AccessSpecifier Access, + QualType T); /// \brief Add a new step that performs a qualification conversion to the /// given type. @@ -631,6 +638,7 @@ public: /// \brief Add a constructor-initialization step. void AddConstructorInitializationStep(CXXConstructorDecl *Constructor, + AccessSpecifier Access, QualType T); /// \brief Add a zero-initialization step. diff --git a/test/CXX/class.access/p6.cpp b/test/CXX/class.access/p6.cpp index 5c0dcf34d2..ce60b3bf04 100644 --- a/test/CXX/class.access/p6.cpp +++ b/test/CXX/class.access/p6.cpp @@ -15,6 +15,8 @@ // to create and destroy a static data member is performed as if // these calls appeared in the scope of the member's class. +struct Public {}; struct Protected {}; struct Private {}; + namespace test0 { class A { typedef int type; // expected-note {{declared private here}} @@ -24,3 +26,29 @@ namespace test0 { A::type foo() { } // expected-error {{access to private member}} A::type A::foo() { } } + +// conversion decls +namespace test1 { + class A { + public: + A(); + operator Public (); + A(Public); + protected: + operator Protected (); // expected-note {{declared protected here}} + A(Protected); // expected-note {{declared protected here}} + private: + operator Private (); // expected-note {{declared private here}} + A(Private); // expected-note {{declared private here}} + }; + + void test() { + A a; + Public pub = a; + Protected prot = a; // expected-error {{access to protected member}} + Private priv = a; // expected-error {{access to private member}} + A apub = pub; + A aprot = prot; // expected-error {{access to protected member}} + A apriv = priv; // expected-error {{access to private member}} + } +}