[PATCH] Ensure visibility attribute on namespace can't be popped using #pragma GCC visibility pop and vice versa (PR c++/41774)

[PATCH] Ensure visibility attribute on namespace can't be popped using #pragma GCC visibility pop and vice versa (PR c++/41774)

[Jakub Jelinek]

Hi!

As shown by the attached testcase, currently #pragma GCC visibility pop
can pop visibility state pushed by namespace with visibility attribute on it
and as pop_visibility when leaving such namespace doesn't expect the stack
to be empty, we get ICE.
One fix would be just to check for empty visstack in that case, but IMNSHO
we shouldn't allow pushing visibility state using one syntax and popping it
through other syntax.  So, this patch changes the stack to also record a
kind of push, now #pragma GCC visibility push/pop needs to be paired
within namespace with visibility attribute.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2009-10-21  Jakub Jelinek  <jakub@redhat.com>

	PR c++/41774
	* c-pragma.c (visstack): Change into vector of ints rather than
	enum symbol_visibility.
	(push_visibility): Add kind argument, push default_visibility together
	with kind.
	(pop_visibility): Add kind argument, return true if successful, fail
	if visibility stack is empty or if stack top is of different kind.
	(handle_pragma_visibility): Don't check length of visstack, instead
	call pop_visibility and issue diagnostics if it failed.  Pass 0
	as last argument to push_visibility and pop_visibility.
	* c-pragma.h (push_visibility): Add kind argument.
	(pop_visibility): Likewise.  Return bool instead of void.

	* name-lookup.c (handle_namespace_attrs): Pass 1 as last argument to
	push_visibility.
	* parser.c (cp_parser_namespace_definition): Pass 1 as argument to
	pop_visibility.
	* rtti.c (push_abi_namespace): Pass 2 as last argument to
	push_visibility.
	(pop_abi_namespace): Pass 2 as argument to pop_visibility.

	* g++.dg/ext/visibility/namespace3.C: New test.

--- gcc/c-pragma.c.jj	2009-08-19 17:46:12.000000000 +0200
+++ gcc/c-pragma.c	2009-10-21 09:40:31.000000000 +0200
@@ -723,19 +723,16 @@ maybe_apply_renaming_pragma (tree decl, 
 #ifdef HANDLE_PRAGMA_VISIBILITY
 static void handle_pragma_visibility (cpp_reader *);
 
-typedef enum symbol_visibility visibility;
-DEF_VEC_I (visibility);
-DEF_VEC_ALLOC_I (visibility, heap);
-static VEC (visibility, heap) *visstack;
+static VEC (int, heap) *visstack;
 
 /* Push the visibility indicated by STR onto the top of the #pragma
    visibility stack.  */
 
 void
-push_visibility (const char *str)
+push_visibility (const char *str, int kind)
 {
-  VEC_safe_push (visibility, heap, visstack,
-		 default_visibility);
+  VEC_safe_push (int, heap, visstack,
+		 ((int) default_visibility) | (kind << 8));
   if (!strcmp (str, "default"))
     default_visibility = VISIBILITY_DEFAULT;
   else if (!strcmp (str, "internal"))
@@ -751,12 +748,18 @@ push_visibility (const char *str)
 
 /* Pop a level of the #pragma visibility stack.  */
 
-void
-pop_visibility (void)
+bool
+pop_visibility (int kind)
 {
-  default_visibility = VEC_pop (visibility, visstack);
+  if (!VEC_length (int, visstack))
+    return false;
+  if ((VEC_last (int, visstack) >> 8) != kind)
+    return false;
+  default_visibility
+    = (enum symbol_visibility) (VEC_pop (int, visstack) & 0xff);
   visibility_options.inpragma
-    = VEC_length (visibility, visstack) != 0;
+    = VEC_length (int, visstack) != 0;
+  return true;
 }
 
 /* Sets the default visibility for symbols to something other than that
@@ -785,10 +788,8 @@ handle_pragma_visibility (cpp_reader *du
     {
       if (pop == action)
 	{
-	  if (!VEC_length (visibility, visstack))
+	  if (! pop_visibility (0))
 	    GCC_BAD ("no matching push for %<#pragma GCC visibility pop%>");
-	  else
-	    pop_visibility ();
 	}
       else
 	{
@@ -798,7 +799,7 @@ handle_pragma_visibility (cpp_reader *du
 	  if (token != CPP_NAME)
 	    GCC_BAD ("malformed #pragma GCC visibility push");
 	  else
-	    push_visibility (IDENTIFIER_POINTER (x));
+	    push_visibility (IDENTIFIER_POINTER (x), 0);
 	  if (pragma_lex (&x) != CPP_CLOSE_PAREN)
 	    GCC_BAD ("missing %<(%> after %<#pragma GCC visibility push%> - ignored");
 	}
--- gcc/c-pragma.h.jj	2009-10-13 16:19:52.000000000 +0200
+++ gcc/c-pragma.h	2009-10-21 09:40:57.000000000 +0200
@@ -95,8 +95,8 @@ extern struct cpp_reader* parse_in;
    visibility is not supported on the host OS platform the
    statements are ignored.  */
 #define HANDLE_PRAGMA_VISIBILITY 1
-extern void push_visibility (const char *);
-extern void pop_visibility (void);
+extern void push_visibility (const char *, int);
+extern bool pop_visibility (int);
 
 extern void init_pragma (void);
 
--- gcc/cp/name-lookup.c.jj	2009-10-07 09:24:42.000000000 +0200
+++ gcc/cp/name-lookup.c	2009-10-21 09:41:20.000000000 +0200
@@ -3175,7 +3175,7 @@ handle_namespace_attrs (tree ns, tree at
 		     "%qD attribute is meaningless since members of the "
 		     "anonymous namespace get local symbols", name);
 
-	  push_visibility (TREE_STRING_POINTER (x));
+	  push_visibility (TREE_STRING_POINTER (x), 1);
 	  saw_vis = true;
 	}
       else
--- gcc/cp/parser.c.jj	2009-10-19 16:34:19.000000000 +0200
+++ gcc/cp/parser.c	2009-10-21 09:41:40.000000000 +0200
@@ -12830,7 +12830,7 @@ cp_parser_namespace_definition (cp_parse
 
 #ifdef HANDLE_PRAGMA_VISIBILITY
   if (has_visibility)
-    pop_visibility ();
+    pop_visibility (1);
 #endif
 
   /* Finish the namespace.  */
--- gcc/cp/rtti.c.jj	2009-07-24 11:05:07.000000000 +0200
+++ gcc/cp/rtti.c	2009-10-21 09:49:58.000000000 +0200
@@ -128,13 +128,13 @@ static void
 push_abi_namespace (void)
 {
   push_nested_namespace (abi_node);
-  push_visibility ("default");
+  push_visibility ("default", 2);
 }
 
 static void
 pop_abi_namespace (void)
 {
-  pop_visibility ();
+  pop_visibility (2);
   pop_nested_namespace (abi_node);
 }
 
--- gcc/testsuite/g++.dg/ext/visibility/namespace3.C.jj	2009-10-21 09:51:29.000000000 +0200
+++ gcc/testsuite/g++.dg/ext/visibility/namespace3.C	2009-10-21 09:53:15.000000000 +0200
@@ -0,0 +1,6 @@
+// PR c++/41774
+// { dg-do compile }
+
+namespace std __attribute__ ((__visibility__ ("default"))) {
+#pragma GCC visibility pop	// { dg-warning "no matching push for" }
+}

	Jakub

Re: [PATCH] Ensure visibility attribute on namespace can't be popped using #pragma GCC visibility pop and vice versa (PR c++/41774)

[Jason Merrill]

On 10/21/2009 08:00 AM, Jakub Jelinek wrote:
>   /* Push the visibility indicated by STR onto the top of the #pragma
>      visibility stack.  */
>
>   void
> -push_visibility (const char *str)
> +push_visibility (const char *str, int kind)

This needs documentation about the new parameter.  OK with that change.

Jason