[PATCH] D55260: [CodeComplete] Fix a crash in access checks of inner classes

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: kadircet.

The crash was introduced in r348135.


Repository:
  rC Clang

https://reviews.llvm.org/D55260

Files:
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/accessibility.cpp


Index: test/CodeCompletion/accessibility.cpp
===================================================================
--- test/CodeCompletion/accessibility.cpp
+++ test/CodeCompletion/accessibility.cpp
@@ -71,3 +71,27 @@
     // RUN: | FileCheck -check-prefix=UNRELATED %s
   }
 };
+
+class Outer {
+ public:
+  static int pub;
+ protected:
+  static int prot;
+ private:
+  static int priv;
+
+  class Inner {
+    int test() {
+      Outer::pub = 10;
+    // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:85:14 %s -o - \
+    // RUN: | FileCheck -check-prefix=OUTER %s
+    // OUTER: priv : [#int#]priv
+    // OUTER: prot : [#int#]prot
+    // OUTER: pub : [#int#]pub
+
+    // Also check the unqualified case.
+    // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:85:1 %s -o - \
+    // RUN: | FileCheck -check-prefix=OUTER %s
+    }
+  };
+};
Index: lib/Sema/SemaCodeComplete.cpp
===================================================================
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -11,6 +11,7 @@
 //
 //===----------------------------------------------------------------------===//
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/ExprCXX.h"
@@ -1313,23 +1314,39 @@
 
   void FoundDecl(NamedDecl *ND, NamedDecl *Hiding, DeclContext *Ctx,
                  bool InBaseClass) override {
-    // Naming class to use for access check. In most cases it was provided
-    // explicitly (e.g. member access (lhs.foo) or qualified lookup (X::)), for
-    // unqualified lookup we fallback to the \p Ctx in which we found the
-    // member.
-    auto *NamingClass = this->NamingClass;
-    if (!NamingClass)
-      NamingClass = llvm::dyn_cast_or_null<CXXRecordDecl>(Ctx);
-    bool Accessible =
-        Results.getSema().IsSimplyAccessible(ND, NamingClass, BaseType);
     ResultBuilder::Result Result(ND, Results.getBasePriority(ND), nullptr,
-                                 false, Accessible, FixIts);
+                                 false, IsAccessible(ND, Ctx), FixIts);
     Results.AddResult(Result, InitialLookupCtx, Hiding, InBaseClass);
   }
 
   void EnteredContext(DeclContext *Ctx) override {
     Results.addVisitedContext(Ctx);
   }
+
+  private:
+  bool IsAccessible(NamedDecl *ND, DeclContext *Ctx) {
+    auto *Cls = llvm::dyn_cast_or_null<CXXRecordDecl>(Ctx);
+    if (!Cls)
+      return true; // Only class members require access checking.
+
+    // Naming class to use for access check. In most cases it was provided
+    // explicitly (e.g. member access (lhs.foo) or qualified lookup (X::)),
+    // for unqualified lookup we fallback to the \p Ctx in which we found the
+    // member.
+    auto *NamingClass = this->NamingClass;
+    QualType BaseType = this->BaseType;
+    if (!NamingClass)
+      NamingClass = Cls;
+    // When we emulate implicit 'this->' in an unqualified lookup, we might end
+    // up with an invalid naming class. In that case, we avoid emulating
+    // 'this->' qualifier to satisfy preconditions of the access checking.
+    if (NamingClass->getCanonicalDecl() != Cls->getCanonicalDecl() &&
+        !NamingClass->isDerivedFrom(Cls)) {
+      NamingClass = Cls;
+      BaseType = QualType();
+    }
+    return Results.getSema().IsSimplyAccessible(ND, Cls, BaseType);
+  }
 };
 } // namespace
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D55260.176578.patch
Type: text/x-patch
Size: 3399 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181204/657ab13c/attachment.bin>