Reject mismatched "#pragma GCC visibility push" and "#pragma GCC visibility pop".

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@149559 91177308-0d34-0410-b5e6-96231b3b80d8

6 files changed, 58 insertions, 24 deletions

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index ff3b1d6e31..1c48a83ba6 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -372,6 +372,14 @@ def warn_pragma_unused_expected_var_arg : Warning<
   "only variables can be arguments to '#pragma unused'">;
 def err_unsupported_pragma_weak : Error<
   "using '#pragma weak' to refer to an undeclared identifier is not yet supported">;
+def err_pragma_push_visibility_mismatch : Error<
+  "#pragma visibility push with no matching #pragma visibility pop">;
+def note_surrounding_namespace_ends_here : Note<
+  "surrounding namespace with visibility attribute ends here">;
+def err_pragma_pop_visibility_mismatch : Error<
+  "#pragma visibility pop with no matching #pragma visibility push">;
+def note_surrounding_namespace_starts_here : Note<
+  "surrounding namespace with visibility attribute starts here">;
 
 /// Objective-C parser diagnostics
 def err_duplicate_class_def : Error<
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 609c64e0c7..ff6b1ae8af 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -5625,7 +5625,8 @@ public:
 
   /// PushNamespaceVisibilityAttr - Note that we've entered a
   /// namespace with a visibility attribute.
-  void PushNamespaceVisibilityAttr(const VisibilityAttr *Attr);
+  void PushNamespaceVisibilityAttr(const VisibilityAttr *Attr,
+                                   SourceLocation Loc);
 
   /// AddPushedVisibilityAttribute - If '#pragma GCC visibility' was used,
   /// add an appropriate visibility attribute.
@@ -5633,7 +5634,7 @@ public:
 
   /// PopPragmaVisibility - Pop the top element of the visibility stack; used
   /// for '#pragma GCC visibility' and visibility attributes on namespaces.
-  void PopPragmaVisibility();
+  void PopPragmaVisibility(bool IsNamespaceEnd, SourceLocation EndLoc);
 
   /// FreeVisContext - Deallocate and null out VisContext.
   void FreeVisContext();
diff --git a/lib/Sema/SemaAttr.cpp b/lib/Sema/SemaAttr.cpp
index 0859cb727b..8b438a4801 100644
--- a/lib/Sema/SemaAttr.cpp
+++ b/lib/Sema/SemaAttr.cpp
@@ -365,7 +365,7 @@ void Sema::ActOnPragmaVisibility(const IdentifierInfo* VisType,
     }
     PushPragmaVisibility(*this, type, PragmaLoc);
   } else {
-    PopPragmaVisibility();
+    PopPragmaVisibility(false, PragmaLoc);
   }
 }
 
@@ -383,23 +383,44 @@ void Sema::ActOnPragmaFPContract(tok::OnOffSwitch OOS) {
   }
 }
 
-void Sema::PushNamespaceVisibilityAttr(const VisibilityAttr *Attr) {
+void Sema::PushNamespaceVisibilityAttr(const VisibilityAttr *Attr,
+                                       SourceLocation Loc) {
   // Visibility calculations will consider the namespace's visibility.
   // Here we just want to note that we're in a visibility context
   // which overrides any enclosing #pragma context, but doesn't itself
   // contribute visibility.
-  PushPragmaVisibility(*this, NoVisibility, SourceLocation());
+  PushPragmaVisibility(*this, NoVisibility, Loc);
 }
 
-void Sema::PopPragmaVisibility() {
-  // Pop visibility from stack, if there is one on the stack.
-  if (VisContext) {
-    VisStack *Stack = static_cast<VisStack*>(VisContext);
+void Sema::PopPragmaVisibility(bool IsNamespaceEnd, SourceLocation EndLoc) {
+  if (!VisContext) {
+    Diag(EndLoc, diag::err_pragma_pop_visibility_mismatch);
+    return;
+  }
+
+  // Pop visibility from stack
+  VisStack *Stack = static_cast<VisStack*>(VisContext);
 
-    Stack->pop_back();
-    // To simplify the implementation, never keep around an empty stack.
-    if (Stack->empty())
-      FreeVisContext();
+  const std::pair<unsigned, SourceLocation> *Back = &Stack->back();
+  bool StartsWithPragma = Back->first != NoVisibility;
+  if (StartsWithPragma && IsNamespaceEnd) {
+    Diag(Back->second, diag::err_pragma_push_visibility_mismatch);
+    Diag(EndLoc, diag::note_surrounding_namespace_ends_here);
+
+    // For better error recovery, eat all pushes inside the namespace.
+    do {
+      Stack->pop_back();
+      Back = &Stack->back();
+      StartsWithPragma = Back->first != NoVisibility;
+    } while (StartsWithPragma);
+  } else if (!StartsWithPragma && !IsNamespaceEnd) {
+    Diag(EndLoc, diag::err_pragma_pop_visibility_mismatch);
+    Diag(Back->second, diag::note_surrounding_namespace_starts_here);
+    return;
   }
-  // FIXME: Add diag for pop without push.
+
+  Stack->pop_back();
+  // To simplify the implementation, never keep around an empty stack.
+  if (Stack->empty())
+    FreeVisContext();
 }
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index fd35733cf3..d83bf08891 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -5683,7 +5683,7 @@ Decl *Sema::ActOnStartNamespaceDef(Scope *NamespcScope,
 
   // FIXME: Should we be merging attributes?
   if (const VisibilityAttr *Attr = Namespc->getAttr<VisibilityAttr>())
-    PushNamespaceVisibilityAttr(Attr);
+    PushNamespaceVisibilityAttr(Attr, Loc);
 
   if (IsStd)
     StdNamespace = Namespc;
@@ -5758,7 +5758,7 @@ void Sema::ActOnFinishNamespaceDef(Decl *Dcl, SourceLocation RBrace) {
   Namespc->setRBraceLoc(RBrace);
   PopDeclContext();
   if (Namespc->hasAttr<VisibilityAttr>())
-    PopPragmaVisibility();
+    PopPragmaVisibility(true, RBrace);
 }
 
 CXXRecordDecl *Sema::getStdBadAlloc() const {
diff --git a/test/CodeGenCXX/pragma-visibility.cpp b/test/CodeGenCXX/pragma-visibility.cpp
index 97f8cc8bb8..e54626ee2e 100644
--- a/test/CodeGenCXX/pragma-visibility.cpp
+++ b/test/CodeGenCXX/pragma-visibility.cpp
@@ -60,11 +60,3 @@ namespace n __attribute((visibility("default")))  {
   // CHECK: define hidden void @_ZN1n1gEv
 #pragma GCC visibility pop
 }
-
-// We used to test this, but it's insane, so unless it happens in
-// headers, we should not support it.
-namespace n __attribute((visibility("hidden"))) {
-  #pragma GCC visibility pop
-  void h() {}
-  // CHECK disabled: define void @_ZN1n1hEv
-}
diff --git a/test/SemaCXX/pragma-visibility.cpp b/test/SemaCXX/pragma-visibility.cpp
new file mode 100644
index 0000000000..d63f19c693
--- /dev/null
+++ b/test/SemaCXX/pragma-visibility.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+namespace test1 __attribute__((visibility("hidden"))) { // expected-note{{surrounding namespace with visibility attribute starts here}}
+#pragma GCC visibility pop // expected-error{{#pragma visibility pop with no matching #pragma visibility push}}
+}
+
+// GCC 4.6 accepts this, but the "hidden" leaks past the namespace end.
+namespace test2 __attribute__((visibility("hidden"))) {
+#pragma GCC visibility push(protected) // expected-error{{#pragma visibility push with no matching #pragma visibility pop}}
+} // expected-note{{surrounding namespace with visibility attribute ends here}}
+
+#pragma GCC visibility pop // expected-error{{#pragma visibility pop with no matching #pragma visibility push}}