r176146 - Don't crash when diagnosing path-constrained protected
John McCall
rjmccall at apple.com
Tue Feb 26 16:08:20 PST 2013
Author: rjmccall
Date: Tue Feb 26 18:08:19 2013
New Revision: 176146
URL: http://llvm.org/viewvc/llvm-project?rev=176146&view=rev
Log:
Don't crash when diagnosing path-constrained protected
access to a private member to which we have special access.
rdar://12926092
Modified:
cfe/trunk/lib/Sema/SemaAccess.cpp
cfe/trunk/test/CXX/class.access/class.access.base/p5.cpp
Modified: cfe/trunk/lib/Sema/SemaAccess.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaAccess.cpp?rev=176146&r1=176145&r2=176146&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaAccess.cpp (original)
+++ cfe/trunk/lib/Sema/SemaAccess.cpp Tue Feb 26 18:08:19 2013
@@ -217,6 +217,15 @@ struct AccessTarget : public AccessedEnt
return DeclaringClass;
}
+ /// The "effective" naming class is the canonical non-anonymous
+ /// class containing the actual naming class.
+ const CXXRecordDecl *getEffectiveNamingClass() const {
+ const CXXRecordDecl *namingClass = getNamingClass();
+ while (namingClass->isAnonymousStructOrUnion())
+ namingClass = cast<CXXRecordDecl>(namingClass->getParent());
+ return namingClass->getCanonicalDecl();
+ }
+
private:
void initialize() {
HasInstanceContext = (isMemberAccess() &&
@@ -1023,8 +1032,7 @@ static bool TryDiagnoseProtectedAccess(S
assert(Target.isMemberAccess());
- const CXXRecordDecl *NamingClass = Target.getNamingClass();
- NamingClass = NamingClass->getCanonicalDecl();
+ const CXXRecordDecl *NamingClass = Target.getEffectiveNamingClass();
for (EffectiveContext::record_iterator
I = EC.Records.begin(), E = EC.Records.end(); I != E; ++I) {
@@ -1089,129 +1097,173 @@ static bool TryDiagnoseProtectedAccess(S
return false;
}
-/// Diagnose the path which caused the given declaration or base class
-/// to become inaccessible.
-static void DiagnoseAccessPath(Sema &S,
- const EffectiveContext &EC,
- AccessTarget &Entity) {
- AccessSpecifier Access = Entity.getAccess();
+/// We are unable to access a given declaration due to its direct
+/// access control; diagnose that.
+static void diagnoseBadDirectAccess(Sema &S,
+ const EffectiveContext &EC,
+ AccessTarget &entity) {
+ assert(entity.isMemberAccess());
+ NamedDecl *D = entity.getTargetDecl();
+
+ if (D->getAccess() == AS_protected &&
+ TryDiagnoseProtectedAccess(S, EC, entity))
+ return;
+
+ // Find an original declaration.
+ while (D->isOutOfLine()) {
+ NamedDecl *PrevDecl = 0;
+ if (VarDecl *VD = dyn_cast<VarDecl>(D))
+ PrevDecl = VD->getPreviousDecl();
+ else if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
+ PrevDecl = FD->getPreviousDecl();
+ else if (TypedefNameDecl *TND = dyn_cast<TypedefNameDecl>(D))
+ PrevDecl = TND->getPreviousDecl();
+ else if (TagDecl *TD = dyn_cast<TagDecl>(D)) {
+ if (isa<RecordDecl>(D) && cast<RecordDecl>(D)->isInjectedClassName())
+ break;
+ PrevDecl = TD->getPreviousDecl();
+ }
+ if (!PrevDecl) break;
+ D = PrevDecl;
+ }
- NamedDecl *D = (Entity.isMemberAccess() ? Entity.getTargetDecl() : 0);
- const CXXRecordDecl *DeclaringClass = Entity.getDeclaringClass();
+ CXXRecordDecl *DeclaringClass = FindDeclaringClass(D);
+ Decl *ImmediateChild;
+ if (D->getDeclContext() == DeclaringClass)
+ ImmediateChild = D;
+ else {
+ DeclContext *DC = D->getDeclContext();
+ while (DC->getParent() != DeclaringClass)
+ DC = DC->getParent();
+ ImmediateChild = cast<Decl>(DC);
+ }
- // Easy case: the decl's natural access determined its path access.
- // We have to check against AS_private here in case Access is AS_none,
- // indicating a non-public member of a private base class.
- if (D && (Access == D->getAccess() || D->getAccess() == AS_private)) {
- switch (HasAccess(S, EC, DeclaringClass, D->getAccess(), Entity)) {
- case AR_inaccessible: {
- if (Access == AS_protected &&
- TryDiagnoseProtectedAccess(S, EC, Entity))
- return;
-
- // Find an original declaration.
- while (D->isOutOfLine()) {
- NamedDecl *PrevDecl = 0;
- if (VarDecl *VD = dyn_cast<VarDecl>(D))
- PrevDecl = VD->getPreviousDecl();
- else if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
- PrevDecl = FD->getPreviousDecl();
- else if (TypedefNameDecl *TND = dyn_cast<TypedefNameDecl>(D))
- PrevDecl = TND->getPreviousDecl();
- else if (TagDecl *TD = dyn_cast<TagDecl>(D)) {
- if (isa<RecordDecl>(D) && cast<RecordDecl>(D)->isInjectedClassName())
- break;
- PrevDecl = TD->getPreviousDecl();
- }
- if (!PrevDecl) break;
- D = PrevDecl;
- }
+ // Check whether there's an AccessSpecDecl preceding this in the
+ // chain of the DeclContext.
+ bool isImplicit = true;
+ for (CXXRecordDecl::decl_iterator
+ I = DeclaringClass->decls_begin(), E = DeclaringClass->decls_end();
+ I != E; ++I) {
+ if (*I == ImmediateChild) break;
+ if (isa<AccessSpecDecl>(*I)) {
+ isImplicit = false;
+ break;
+ }
+ }
- CXXRecordDecl *DeclaringClass = FindDeclaringClass(D);
- Decl *ImmediateChild;
- if (D->getDeclContext() == DeclaringClass)
- ImmediateChild = D;
- else {
- DeclContext *DC = D->getDeclContext();
- while (DC->getParent() != DeclaringClass)
- DC = DC->getParent();
- ImmediateChild = cast<Decl>(DC);
- }
-
- // Check whether there's an AccessSpecDecl preceding this in the
- // chain of the DeclContext.
- bool Implicit = true;
- for (CXXRecordDecl::decl_iterator
- I = DeclaringClass->decls_begin(), E = DeclaringClass->decls_end();
- I != E; ++I) {
- if (*I == ImmediateChild) break;
- if (isa<AccessSpecDecl>(*I)) {
- Implicit = false;
- break;
- }
- }
+ S.Diag(D->getLocation(), diag::note_access_natural)
+ << (unsigned) (D->getAccess() == AS_protected)
+ << isImplicit;
+}
- S.Diag(D->getLocation(), diag::note_access_natural)
- << (unsigned) (Access == AS_protected)
- << Implicit;
- return;
- }
+/// Diagnose the path which caused the given declaration or base class
+/// to become inaccessible.
+static void DiagnoseAccessPath(Sema &S,
+ const EffectiveContext &EC,
+ AccessTarget &entity) {
+ // Save the instance context to preserve invariants.
+ AccessTarget::SavedInstanceContext _ = entity.saveInstanceContext();
+
+ // This basically repeats the main algorithm but keeps some more
+ // information.
+
+ // The natural access so far.
+ AccessSpecifier accessSoFar = AS_public;
+
+ // Check whether we have special rights to the declaring class.
+ if (entity.isMemberAccess()) {
+ NamedDecl *D = entity.getTargetDecl();
+ accessSoFar = D->getAccess();
+ const CXXRecordDecl *declaringClass = entity.getDeclaringClass();
+
+ switch (HasAccess(S, EC, declaringClass, accessSoFar, entity)) {
+ // If the declaration is accessible when named in its declaring
+ // class, then we must be constrained by the path.
+ case AR_accessible:
+ accessSoFar = AS_public;
+ entity.suppressInstanceContext();
+ break;
- case AR_accessible: break;
+ case AR_inaccessible:
+ if (accessSoFar == AS_private ||
+ declaringClass == entity.getEffectiveNamingClass())
+ return diagnoseBadDirectAccess(S, EC, entity);
+ break;
case AR_dependent:
- llvm_unreachable("can't diagnose dependent access failures");
+ llvm_unreachable("cannot diagnose dependent access");
}
}
- CXXBasePaths Paths;
- CXXBasePath &Path = *FindBestPath(S, EC, Entity, AS_public, Paths);
-
- CXXBasePath::iterator I = Path.end(), E = Path.begin();
- while (I != E) {
- --I;
-
- const CXXBaseSpecifier *BS = I->Base;
- AccessSpecifier BaseAccess = BS->getAccessSpecifier();
-
- // If this is public inheritance, or the derived class is a friend,
- // skip this step.
- if (BaseAccess == AS_public)
- continue;
+ CXXBasePaths paths;
+ CXXBasePath &path = *FindBestPath(S, EC, entity, accessSoFar, paths);
+ assert(path.Access != AS_public);
+
+ CXXBasePath::iterator i = path.end(), e = path.begin();
+ CXXBasePath::iterator constrainingBase = i;
+ while (i != e) {
+ --i;
+
+ assert(accessSoFar != AS_none && accessSoFar != AS_private);
+
+ // Is the entity accessible when named in the deriving class, as
+ // modified by the base specifier?
+ const CXXRecordDecl *derivingClass = i->Class->getCanonicalDecl();
+ const CXXBaseSpecifier *base = i->Base;
+
+ // If the access to this base is worse than the access we have to
+ // the declaration, remember it.
+ AccessSpecifier baseAccess = base->getAccessSpecifier();
+ if (baseAccess > accessSoFar) {
+ constrainingBase = i;
+ accessSoFar = baseAccess;
+ }
- switch (GetFriendKind(S, EC, I->Class)) {
- case AR_accessible: continue;
+ switch (HasAccess(S, EC, derivingClass, accessSoFar, entity)) {
case AR_inaccessible: break;
+ case AR_accessible:
+ accessSoFar = AS_public;
+ entity.suppressInstanceContext();
+ constrainingBase = 0;
+ break;
case AR_dependent:
- llvm_unreachable("can't diagnose dependent access failures");
+ llvm_unreachable("cannot diagnose dependent access");
}
- // Check whether this base specifier is the tighest point
- // constraining access. We have to check against AS_private for
- // the same reasons as above.
- if (BaseAccess == AS_private || BaseAccess >= Access) {
-
- // We're constrained by inheritance, but we want to say
- // "declared private here" if we're diagnosing a hierarchy
- // conversion and this is the final step.
- unsigned diagnostic;
- if (D) diagnostic = diag::note_access_constrained_by_path;
- else if (I + 1 == Path.end()) diagnostic = diag::note_access_natural;
- else diagnostic = diag::note_access_constrained_by_path;
-
- S.Diag(BS->getSourceRange().getBegin(), diagnostic)
- << BS->getSourceRange()
- << (BaseAccess == AS_protected)
- << (BS->getAccessSpecifierAsWritten() == AS_none);
-
- if (D)
- S.Diag(D->getLocation(), diag::note_field_decl);
-
- return;
+ // If this was private inheritance, but we don't have access to
+ // the deriving class, we're done.
+ if (accessSoFar == AS_private) {
+ assert(baseAccess == AS_private);
+ assert(constrainingBase == i);
+ break;
}
}
- llvm_unreachable("access not apparently constrained by path");
+ // If we don't have a constraining base, the access failure must be
+ // due to the original declaration.
+ if (constrainingBase == path.end())
+ return diagnoseBadDirectAccess(S, EC, entity);
+
+ // We're constrained by inheritance, but we want to say
+ // "declared private here" if we're diagnosing a hierarchy
+ // conversion and this is the final step.
+ unsigned diagnostic;
+ if (entity.isMemberAccess() ||
+ constrainingBase + 1 != path.end()) {
+ diagnostic = diag::note_access_constrained_by_path;
+ } else {
+ diagnostic = diag::note_access_natural;
+ }
+
+ const CXXBaseSpecifier *base = constrainingBase->Base;
+
+ S.Diag(base->getSourceRange().getBegin(), diagnostic)
+ << base->getSourceRange()
+ << (base->getAccessSpecifier() == AS_protected)
+ << (base->getAccessSpecifierAsWritten() == AS_none);
+
+ if (entity.isMemberAccess())
+ S.Diag(entity.getTargetDecl()->getLocation(), diag::note_field_decl);
}
static void DiagnoseBadAccess(Sema &S, SourceLocation Loc,
@@ -1273,10 +1325,7 @@ static AccessResult IsAccessible(Sema &S
const EffectiveContext &EC,
AccessTarget &Entity) {
// Determine the actual naming class.
- CXXRecordDecl *NamingClass = Entity.getNamingClass();
- while (NamingClass->isAnonymousStructOrUnion())
- NamingClass = cast<CXXRecordDecl>(NamingClass->getParent());
- NamingClass = NamingClass->getCanonicalDecl();
+ const CXXRecordDecl *NamingClass = Entity.getEffectiveNamingClass();
AccessSpecifier UnprivilegedAccess = Entity.getAccess();
assert(UnprivilegedAccess != AS_public && "public access not weeded out");
Modified: cfe/trunk/test/CXX/class.access/class.access.base/p5.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.access/class.access.base/p5.cpp?rev=176146&r1=176145&r2=176146&view=diff
==============================================================================
--- cfe/trunk/test/CXX/class.access/class.access.base/p5.cpp (original)
+++ cfe/trunk/test/CXX/class.access/class.access.base/p5.cpp Tue Feb 26 18:08:19 2013
@@ -72,4 +72,27 @@ namespace test3 {
};
}
+// Don't crash. <rdar://12926092>
+// Note that 'field' is indeed a private member of X but that access
+// is indeed ultimately constrained by the protected inheritance from Y.
+// If someone wants to put the effort into improving this diagnostic,
+// they can feel free; even explaining it in person would be a pain.
+namespace test4 {
+ class Z;
+ class X {
+ public:
+ void f(Z *p);
+
+ private:
+ int field; // expected-note {{member is declared here}}
+ };
+
+ class Y : public X { };
+ class Z : protected Y { }; // expected-note 2 {{constrained by protected inheritance here}}
+
+ void X::f(Z *p) {
+ p->field = 0; // expected-error {{cannot cast 'test4::Z' to its protected base class 'test4::X'}} expected-error {{'field' is a private member of 'test4::X'}}
+ }
+}
+
// TODO: flesh out these cases
More information about the cfe-commits
mailing list