diff mbox series

PR c++/91476 - anon-namespace reference temp clash between TUs.

Message ID 20200121151731.24798-1-jason@redhat.com
State New
Headers show
Series PR c++/91476 - anon-namespace reference temp clash between TUs. | expand

Commit Message

Jason Merrill Jan. 21, 2020, 3:17 p.m. UTC 
The problem in the PR was that make_temporary_var_for_ref_to_temp ran before
determine_visibility, so when we copied the linkage of the reference
variable it had not yet been restricted by its anonymous namespace context,
so the temporary wrongly ended up with TREE_PUBLIC set.  The natural
solution is to run determine_visibility earlier.  But that needs to happen
after maybe_commonize_var increases the linkage of some local variables, and
on targets without weak symbol support, that function does different things
based on the results of check_initializer, which is what calls
make_temporary_var_for_ref_to_temp.  To break this circular dependency I'm
calling maybe_commonize_var early, and then again later if the target
doesn't support weak symbols.

It also occurred to me that make_temporary_var_for_ref_to_temp wasn't
handling DECL_VISIBILITY at all, and verified that we were doing the wrong
thing.  So I've combined the linkage-copying code from there and two other
places.

Tested x86_64-pc-linux-gnu, applying to trunk.

	* decl2.c (copy_linkage): Factor out of get_guard.
	* call.c (make_temporary_var_for_ref_to_temp): Use it.
	* decl.c (cp_finish_decomp): Use it.
	(cp_finish_decl): determine_visibility sooner.
---
 gcc/cp/cp-tree.h                              |  1 +
 gcc/cp/call.c                                 |  9 +----
 gcc/cp/decl.c                                 | 33 +++++++----------
 gcc/cp/decl2.c                                | 36 ++++++++++++-------
 .../g++.dg/ext/visibility/ref-temp1.C         | 17 +++++++++
 5 files changed, 55 insertions(+), 41 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/ext/visibility/ref-temp1.C


base-commit: f0aec8643830a50812aeec0296086ed338aac678

Comments

Jason Merrill Jan. 21, 2020, 3:20 p.m. UTC | #1 
On 1/21/20 10:17 AM, Jason Merrill wrote:
> The problem in the PR was that make_temporary_var_for_ref_to_temp ran before
> determine_visibility, so when we copied the linkage of the reference
> variable it had not yet been restricted by its anonymous namespace context,
> so the temporary wrongly ended up with TREE_PUBLIC set.  The natural
> solution is to run determine_visibility earlier.  But that needs to happen
> after maybe_commonize_var increases the linkage of some local variables, and
> on targets without weak symbol support, that function does different things
> based on the results of check_initializer, which is what calls
> make_temporary_var_for_ref_to_temp.  To break this circular dependency I'm
> calling maybe_commonize_var early, and then again later if the target
> doesn't support weak symbols.
> 
> It also occurred to me that make_temporary_var_for_ref_to_temp wasn't
> handling DECL_VISIBILITY at all, and verified that we were doing the wrong
> thing.  So I've combined the linkage-copying code from there and two other
> places.
> 
> Tested x86_64-pc-linux-gnu, applying to trunk.
> 
> 	* decl2.c (copy_linkage): Factor out of get_guard.
> 	* call.c (make_temporary_var_for_ref_to_temp): Use it.
> 	* decl.c (cp_finish_decomp): Use it.
> 	(cp_finish_decl): determine_visibility sooner.

And a much simpler patch for GCC 9, that doesn't address visibility:
diff mbox series

Patch

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 890d5a27350..77bcf046608 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6636,6 +6636,7 @@  extern bool mark_used				(tree);
 extern bool mark_used			        (tree, tsubst_flags_t);
 extern void finish_static_data_member_decl	(tree, tree, bool, tree, int);
 extern tree cp_build_parm_decl			(tree, tree, tree);
+extern void copy_linkage			(tree, tree);
 extern tree get_guard				(tree);
 extern tree get_guard_cond			(tree, bool);
 extern tree set_guard				(tree);
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index d47747117b9..aacd961faa1 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -11973,14 +11973,7 @@  make_temporary_var_for_ref_to_temp (tree decl, tree type)
 	 GR and suffixed with a sequence number mangled using the usual rules
 	 for a seq-id. Temporaries are numbered with a pre-order, depth-first,
 	 left-to-right walk of the complete initializer.  */
-
-      TREE_STATIC (var) = TREE_STATIC (decl);
-      TREE_PUBLIC (var) = TREE_PUBLIC (decl);
-      if (vague_linkage_p (decl))
-	comdat_linkage (var);
-
-      CP_DECL_THREAD_LOCAL_P (var) = CP_DECL_THREAD_LOCAL_P (decl);
-      set_decl_tls_model (var, DECL_TLS_MODEL (decl));
+      copy_linkage (var, decl);
 
       tree name = mangle_ref_init_variable (decl);
       DECL_NAME (var) = name;
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 28a79029d92..47ff7eea88f 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -7657,6 +7657,14 @@  cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
       TREE_READONLY (decl) = 0;
     }
 
+  /* This needs to happen before extend_ref_init_temps.  */
+  if (VAR_OR_FUNCTION_DECL_P (decl))
+    {
+      if (VAR_P (decl))
+	maybe_commonize_var (decl);
+      determine_visibility (decl);
+    }
+
   if (VAR_P (decl))
     {
       duration_kind dk = decl_storage_duration (decl);
@@ -7786,12 +7794,11 @@  cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
       if (VAR_P (decl))
 	{
 	  layout_var_decl (decl);
-	  maybe_commonize_var (decl);
+	  if (!flag_weak)
+	    /* Check again now that we have an initializer.  */
+	    maybe_commonize_var (decl);
 	}
 
-      /* This needs to happen after the linkage is set. */
-      determine_visibility (decl);
-
       if (var_definition_p && TREE_STATIC (decl))
 	{
 	  /* If a TREE_READONLY variable needs initialization
@@ -8328,23 +8335,7 @@  cp_finish_decomp (tree decl, tree first, unsigned int count)
 	    }
 	  if (!processing_template_decl)
 	    {
-	      TREE_PUBLIC (v[i]) = TREE_PUBLIC (decl);
-	      TREE_STATIC (v[i]) = TREE_STATIC (decl);
-	      DECL_COMMON (v[i]) = DECL_COMMON (decl);
-	      DECL_COMDAT (v[i]) = DECL_COMDAT (decl);
-	      if (TREE_STATIC (v[i]))
-		{
-		  CP_DECL_THREAD_LOCAL_P (v[i])
-		    = CP_DECL_THREAD_LOCAL_P (decl);
-		  set_decl_tls_model (v[i], DECL_TLS_MODEL (decl));
-		  if (DECL_ONE_ONLY (decl))
-		    make_decl_one_only (v[i], cxx_comdat_group (v[i]));
-		  if (TREE_PUBLIC (decl))
-		    DECL_WEAK (v[i]) = DECL_WEAK (decl);
-		  DECL_VISIBILITY (v[i]) = DECL_VISIBILITY (decl);
-		  DECL_VISIBILITY_SPECIFIED (v[i])
-		    = DECL_VISIBILITY_SPECIFIED (decl);
-		}
+	      copy_linkage (v[i], decl);
 	      cp_finish_decl (v[i], init, /*constexpr*/false,
 			      /*asm*/NULL_TREE, LOOKUP_NORMAL);
 	    }
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 042d6fa12df..1ecf0b937d5 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -3214,6 +3214,29 @@  build_cleanup (tree decl)
   return clean;
 }
 
+/* GUARD is a helper variable for DECL; make them have the same linkage and
+   visibility.  */
+
+void
+copy_linkage (tree guard, tree decl)
+{
+  TREE_PUBLIC (guard) = TREE_PUBLIC (decl);
+  TREE_STATIC (guard) = TREE_STATIC (decl);
+  DECL_COMMON (guard) = DECL_COMMON (decl);
+  DECL_COMDAT (guard) = DECL_COMDAT (decl);
+  if (TREE_STATIC (guard))
+    {
+      CP_DECL_THREAD_LOCAL_P (guard) = CP_DECL_THREAD_LOCAL_P (decl);
+      set_decl_tls_model (guard, DECL_TLS_MODEL (decl));
+      /* We can't rely on DECL_WEAK (decl) or DECL_ONE_ONLY (decl) here, as
+	 they may not be set until import_export_decl at EOF.  */
+      if (vague_linkage_p (decl))
+	comdat_linkage (guard);
+      DECL_VISIBILITY (guard) = DECL_VISIBILITY (decl);
+      DECL_VISIBILITY_SPECIFIED (guard) = DECL_VISIBILITY_SPECIFIED (decl);
+    }
+}
+
 /* Returns the initialization guard variable for the variable DECL,
    which has static storage duration.  */
 
@@ -3236,18 +3259,7 @@  get_guard (tree decl)
 			  VAR_DECL, sname, guard_type);
 
       /* The guard should have the same linkage as what it guards.  */
-      TREE_PUBLIC (guard) = TREE_PUBLIC (decl);
-      TREE_STATIC (guard) = TREE_STATIC (decl);
-      DECL_COMMON (guard) = DECL_COMMON (decl);
-      DECL_COMDAT (guard) = DECL_COMDAT (decl);
-      CP_DECL_THREAD_LOCAL_P (guard) = CP_DECL_THREAD_LOCAL_P (decl);
-      set_decl_tls_model (guard, DECL_TLS_MODEL (decl));
-      if (DECL_ONE_ONLY (decl))
-	make_decl_one_only (guard, cxx_comdat_group (guard));
-      if (TREE_PUBLIC (decl))
-	DECL_WEAK (guard) = DECL_WEAK (decl);
-      DECL_VISIBILITY (guard) = DECL_VISIBILITY (decl);
-      DECL_VISIBILITY_SPECIFIED (guard) = DECL_VISIBILITY_SPECIFIED (decl);
+      copy_linkage (guard, decl);
 
       DECL_ARTIFICIAL (guard) = 1;
       DECL_IGNORED_P (guard) = 1;
diff --git a/gcc/testsuite/g++.dg/ext/visibility/ref-temp1.C b/gcc/testsuite/g++.dg/ext/visibility/ref-temp1.C
new file mode 100644
index 00000000000..ecb62326e1b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/visibility/ref-temp1.C
@@ -0,0 +1,17 @@ 
+// PR c++/91476
+// Test that hidden and internal visibility propagates to reference temps.
+
+#define HIDDEN __attribute((visibility("hidden")))
+
+// { dg-final { scan-hidden "_ZGRZ1fvE3foo_" } }
+HIDDEN inline const int* f() { static const int &foo = 1; return &foo; }
+
+// { dg-final { scan-assembler-not "(weak|globl)\[^\n\]*_ZGRN12_GLOBAL__N_13fooE_" } }
+namespace { const int &foo = 1; }
+
+const void *volatile p;
+int main()
+{
+  p = f();
+  p = &foo;
+}