public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: Fix handling of __thread/thread_local extern vars declared at function scope [PR102496]
@ 2021-09-29  8:36 Jakub Jelinek
  2021-09-30 12:06 ` Jason Merrill
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2021-09-29  8:36 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

Hi!

The introduction of push_local_extern_decl_alias in
r11-3699-g4e62aca0e0520e4ed2532f2d8153581190621c1a
broke tls vars, while the decl they are created for has the tls model
set properly, nothing sets it for the alias that is actually used,
so accesses to it are done as if they were normal variables.
This is then diagnosed at link time if the definition of the extern
vars is __thread/thread_local.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
ok for trunk/11.3?

2021-09-28  Jakub Jelinek  <jakub@redhat.com>

	PR c++/102496
	* name-lookup.c: Include varasm.h.
	(push_local_extern_decl_alias): For CP_DECL_THREAD_LOCAL_P vars
	call set_decl_tls_model on alias.

	* g++.dg/tls/pr102496-1.C: New test.
	* g++.dg/tls/pr102496-2.C: New test.

--- gcc/cp/name-lookup.c.jj	2021-09-28 15:51:54.035601004 +0200
+++ gcc/cp/name-lookup.c	2021-09-28 16:22:51.169954638 +0200
@@ -36,6 +36,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "c-family/known-headers.h"
 #include "c-family/c-spellcheck.h"
 #include "bitmap.h"
+#include "varasm.h"
 
 static cxx_binding *cxx_binding_make (tree value, tree type);
 static cp_binding_level *innermost_nonclass_level (void);
@@ -3471,6 +3472,10 @@ push_local_extern_decl_alias (tree decl)
 	  push_nested_namespace (ns);
 	  alias = do_pushdecl (alias, /* hiding= */true);
 	  pop_nested_namespace (ns);
+	  if (VAR_P (decl) && CP_DECL_THREAD_LOCAL_P (decl))
+	    set_decl_tls_model (alias, processing_template_decl
+				       ? decl_default_tls_model (decl)
+				       : DECL_TLS_MODEL (decl));
 	}
     }
 
--- gcc/testsuite/g++.dg/tls/pr102496-1.C.jj	2021-09-28 16:25:47.330522476 +0200
+++ gcc/testsuite/g++.dg/tls/pr102496-1.C	2021-09-28 16:28:44.888071020 +0200
@@ -0,0 +1,20 @@
+// PR c++/102496
+// { dg-do link { target c++11 } }
+// { dg-require-effective-target tls }
+// { dg-add-options tls }
+// { dg-additional-sources pr102496-2.C }
+
+template <int N>
+int
+foo ()
+{
+  extern __thread int t1;
+  return t1;
+}
+
+int
+main ()
+{
+  extern __thread int t2;
+  return foo <0> () + t2;
+}
--- gcc/testsuite/g++.dg/tls/pr102496-2.C.jj	2021-09-28 16:25:43.815571005 +0200
+++ gcc/testsuite/g++.dg/tls/pr102496-2.C	2021-09-28 16:28:54.132943380 +0200
@@ -0,0 +1,6 @@
+// PR c++/102496
+// { dg-do compile { target c++11 } }
+// { dg-require-effective-target tls }
+
+__thread int t1;
+__thread int t2;

	Jakub


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] c++: Fix handling of __thread/thread_local extern vars declared at function scope [PR102496]
  2021-09-29  8:36 [PATCH] c++: Fix handling of __thread/thread_local extern vars declared at function scope [PR102496] Jakub Jelinek
@ 2021-09-30 12:06 ` Jason Merrill
  2021-09-30 15:45   ` Jakub Jelinek
  0 siblings, 1 reply; 4+ messages in thread
From: Jason Merrill @ 2021-09-30 12:06 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 9/29/21 04:36, Jakub Jelinek wrote:
> Hi!
> 
> The introduction of push_local_extern_decl_alias in
> r11-3699-g4e62aca0e0520e4ed2532f2d8153581190621c1a
> broke tls vars, while the decl they are created for has the tls model
> set properly, nothing sets it for the alias that is actually used,
> so accesses to it are done as if they were normal variables.
> This is then diagnosed at link time if the definition of the extern
> vars is __thread/thread_local.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
> ok for trunk/11.3?

> 2021-09-28  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/102496
> 	* name-lookup.c: Include varasm.h.
> 	(push_local_extern_decl_alias): For CP_DECL_THREAD_LOCAL_P vars
> 	call set_decl_tls_model on alias.
> 
> 	* g++.dg/tls/pr102496-1.C: New test.
> 	* g++.dg/tls/pr102496-2.C: New test.
> 
> --- gcc/cp/name-lookup.c.jj	2021-09-28 15:51:54.035601004 +0200
> +++ gcc/cp/name-lookup.c	2021-09-28 16:22:51.169954638 +0200
> @@ -36,6 +36,7 @@ along with GCC; see the file COPYING3.  If not see
>   #include "c-family/known-headers.h"
>   #include "c-family/c-spellcheck.h"
>   #include "bitmap.h"
> +#include "varasm.h"
>   
>   static cxx_binding *cxx_binding_make (tree value, tree type);
>   static cp_binding_level *innermost_nonclass_level (void);
> @@ -3471,6 +3472,10 @@ push_local_extern_decl_alias (tree decl)
>   	  push_nested_namespace (ns);
>   	  alias = do_pushdecl (alias, /* hiding= */true);
>   	  pop_nested_namespace (ns);
> +	  if (VAR_P (decl) && CP_DECL_THREAD_LOCAL_P (decl))
> +	    set_decl_tls_model (alias, processing_template_decl
> +				       ? decl_default_tls_model (decl)
> +				       : DECL_TLS_MODEL (decl));

Hmm, what if decl has the tls_model attribute?

We could decide not to push the alias for a thread-local variable when 
processing_template_decl, like we don't if the type is dependent; in 
either case we'll push it at instantiation time.

>   	}
>       }
>   
> --- gcc/testsuite/g++.dg/tls/pr102496-1.C.jj	2021-09-28 16:25:47.330522476 +0200
> +++ gcc/testsuite/g++.dg/tls/pr102496-1.C	2021-09-28 16:28:44.888071020 +0200
> @@ -0,0 +1,20 @@
> +// PR c++/102496
> +// { dg-do link { target c++11 } }
> +// { dg-require-effective-target tls }
> +// { dg-add-options tls }
> +// { dg-additional-sources pr102496-2.C }
> +
> +template <int N>
> +int
> +foo ()
> +{
> +  extern __thread int t1;
> +  return t1;
> +}
> +
> +int
> +main ()
> +{
> +  extern __thread int t2;
> +  return foo <0> () + t2;
> +}
> --- gcc/testsuite/g++.dg/tls/pr102496-2.C.jj	2021-09-28 16:25:43.815571005 +0200
> +++ gcc/testsuite/g++.dg/tls/pr102496-2.C	2021-09-28 16:28:54.132943380 +0200
> @@ -0,0 +1,6 @@
> +// PR c++/102496
> +// { dg-do compile { target c++11 } }
> +// { dg-require-effective-target tls }
> +
> +__thread int t1;
> +__thread int t2;
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] c++: Fix handling of __thread/thread_local extern vars declared at function scope [PR102496]
  2021-09-30 12:06 ` Jason Merrill
@ 2021-09-30 15:45   ` Jakub Jelinek
  2021-09-30 15:49     ` Jason Merrill
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2021-09-30 15:45 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On Thu, Sep 30, 2021 at 08:06:52AM -0400, Jason Merrill wrote:
> Hmm, what if decl has the tls_model attribute?
> 
> We could decide not to push the alias for a thread-local variable when
> processing_template_decl, like we don't if the type is dependent; in either
> case we'll push it at instantiation time.

So like this (assuming it passes full bootstrap/regtest, so far it passed
tls.exp)?

2021-09-28  Jakub Jelinek  <jakub@redhat.com>

	PR c++/102496
	* name-lookup.c (push_local_extern_decl_alias): Return early even for
	tls vars with non-dependent type when processing_template_decl.  For
	CP_DECL_THREAD_LOCAL_P vars call set_decl_tls_model on alias.

	* g++.dg/tls/pr102496-1.C: New test.
	* g++.dg/tls/pr102496-2.C: New test.

--- gcc/cp/name-lookup.c.jj	2021-09-29 10:07:28.838061585 +0200
+++ gcc/cp/name-lookup.c	2021-09-30 17:30:46.010100552 +0200
@@ -3375,7 +3375,10 @@ set_decl_context_in_fn (tree ctx, tree d
 void
 push_local_extern_decl_alias (tree decl)
 {
-  if (dependent_type_p (TREE_TYPE (decl)))
+  if (dependent_type_p (TREE_TYPE (decl))
+      || (processing_template_decl
+	  && VAR_P (decl)
+	  && CP_DECL_THREAD_LOCAL_P (decl)))
     return;
   /* EH specs were not part of the function type prior to c++17, but
      we still can't go pushing dependent eh specs into the namespace.  */
@@ -3471,6 +3474,8 @@ push_local_extern_decl_alias (tree decl)
 	  push_nested_namespace (ns);
 	  alias = do_pushdecl (alias, /* hiding= */true);
 	  pop_nested_namespace (ns);
+	  if (VAR_P (decl) && CP_DECL_THREAD_LOCAL_P (decl))
+	    set_decl_tls_model (alias, DECL_TLS_MODEL (decl));
 	}
     }
 
--- gcc/testsuite/g++.dg/tls/pr102496-1.C.jj	2021-09-30 17:22:47.867769063 +0200
+++ gcc/testsuite/g++.dg/tls/pr102496-1.C	2021-09-30 17:22:47.867769063 +0200
@@ -0,0 +1,20 @@
+// PR c++/102496
+// { dg-do link { target c++11 } }
+// { dg-require-effective-target tls }
+// { dg-add-options tls }
+// { dg-additional-sources pr102496-2.C }
+
+template <int N>
+int
+foo ()
+{
+  extern __thread int t1;
+  return t1;
+}
+
+int
+main ()
+{
+  extern __thread int t2;
+  return foo <0> () + t2;
+}
--- gcc/testsuite/g++.dg/tls/pr102496-2.C.jj	2021-09-30 17:22:47.867769063 +0200
+++ gcc/testsuite/g++.dg/tls/pr102496-2.C	2021-09-30 17:22:47.867769063 +0200
@@ -0,0 +1,6 @@
+// PR c++/102496
+// { dg-do compile { target c++11 } }
+// { dg-require-effective-target tls }
+
+__thread int t1;
+__thread int t2;


	Jakub


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] c++: Fix handling of __thread/thread_local extern vars declared at function scope [PR102496]
  2021-09-30 15:45   ` Jakub Jelinek
@ 2021-09-30 15:49     ` Jason Merrill
  0 siblings, 0 replies; 4+ messages in thread
From: Jason Merrill @ 2021-09-30 15:49 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 9/30/21 11:45, Jakub Jelinek wrote:
> On Thu, Sep 30, 2021 at 08:06:52AM -0400, Jason Merrill wrote:
>> Hmm, what if decl has the tls_model attribute?
>>
>> We could decide not to push the alias for a thread-local variable when
>> processing_template_decl, like we don't if the type is dependent; in either
>> case we'll push it at instantiation time.
> 
> So like this (assuming it passes full bootstrap/regtest, so far it passed
> tls.exp)?

OK.

> 2021-09-28  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/102496
> 	* name-lookup.c (push_local_extern_decl_alias): Return early even for
> 	tls vars with non-dependent type when processing_template_decl.  For
> 	CP_DECL_THREAD_LOCAL_P vars call set_decl_tls_model on alias.
> 
> 	* g++.dg/tls/pr102496-1.C: New test.
> 	* g++.dg/tls/pr102496-2.C: New test.
> 
> --- gcc/cp/name-lookup.c.jj	2021-09-29 10:07:28.838061585 +0200
> +++ gcc/cp/name-lookup.c	2021-09-30 17:30:46.010100552 +0200
> @@ -3375,7 +3375,10 @@ set_decl_context_in_fn (tree ctx, tree d
>   void
>   push_local_extern_decl_alias (tree decl)
>   {
> -  if (dependent_type_p (TREE_TYPE (decl)))
> +  if (dependent_type_p (TREE_TYPE (decl))
> +      || (processing_template_decl
> +	  && VAR_P (decl)
> +	  && CP_DECL_THREAD_LOCAL_P (decl)))
>       return;
>     /* EH specs were not part of the function type prior to c++17, but
>        we still can't go pushing dependent eh specs into the namespace.  */
> @@ -3471,6 +3474,8 @@ push_local_extern_decl_alias (tree decl)
>   	  push_nested_namespace (ns);
>   	  alias = do_pushdecl (alias, /* hiding= */true);
>   	  pop_nested_namespace (ns);
> +	  if (VAR_P (decl) && CP_DECL_THREAD_LOCAL_P (decl))
> +	    set_decl_tls_model (alias, DECL_TLS_MODEL (decl));
>   	}
>       }
>   
> --- gcc/testsuite/g++.dg/tls/pr102496-1.C.jj	2021-09-30 17:22:47.867769063 +0200
> +++ gcc/testsuite/g++.dg/tls/pr102496-1.C	2021-09-30 17:22:47.867769063 +0200
> @@ -0,0 +1,20 @@
> +// PR c++/102496
> +// { dg-do link { target c++11 } }
> +// { dg-require-effective-target tls }
> +// { dg-add-options tls }
> +// { dg-additional-sources pr102496-2.C }
> +
> +template <int N>
> +int
> +foo ()
> +{
> +  extern __thread int t1;
> +  return t1;
> +}
> +
> +int
> +main ()
> +{
> +  extern __thread int t2;
> +  return foo <0> () + t2;
> +}
> --- gcc/testsuite/g++.dg/tls/pr102496-2.C.jj	2021-09-30 17:22:47.867769063 +0200
> +++ gcc/testsuite/g++.dg/tls/pr102496-2.C	2021-09-30 17:22:47.867769063 +0200
> @@ -0,0 +1,6 @@
> +// PR c++/102496
> +// { dg-do compile { target c++11 } }
> +// { dg-require-effective-target tls }
> +
> +__thread int t1;
> +__thread int t2;
> 
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-09-30 15:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-29  8:36 [PATCH] c++: Fix handling of __thread/thread_local extern vars declared at function scope [PR102496] Jakub Jelinek
2021-09-30 12:06 ` Jason Merrill
2021-09-30 15:45   ` Jakub Jelinek
2021-09-30 15:49     ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).