Re: [PR c++/84497] ref to undefined tls init

2018-03-05 Thread Nathan Sidwell 

On 03/02/2018 01:43 PM, Jason Merrill wrote:

On Fri, Mar 2, 2018 at 11:07 AM, Nathan Sidwell  wrote:



NEEDS_CONSTRUCTING && !HAS_CONSTEXPR_CTOR && !HAS_DEFAULT_CONSTRUCTOR seems
like it would be sufficient. and indeed that works in this case.


Do you mean !HAS_TRIVIAL_DFLT rather than !HAS_DEFAULT_CONSTRUCTOR?
That makes sense to me.


Even better!

nathan

--
Nathan Sidwell
2018-03-05  Pádraig Brady  
	Jason  Merrill  
	Nathan Sidwell  

	PR c++/84497
	* decl2.c (get_tls_init_fn): Check TYPE_HAS_TRIVIAL_DFLT too.

	PR c++/84497
	* g++.dg/cpp0x/pr84497.C: New.

Index: cp/decl2.c
===
--- cp/decl2.c	(revision 258114)
+++ cp/decl2.c	(working copy)
@@ -3337,7 +3337,8 @@ get_tls_init_fn (tree var)
 	  /* If the variable is defined somewhere else and might have static
 	 initialization, make the init function a weak reference.  */
 	  if ((!TYPE_NEEDS_CONSTRUCTING (obtype)
-	   || TYPE_HAS_CONSTEXPR_CTOR (obtype))
+	   || TYPE_HAS_CONSTEXPR_CTOR (obtype)
+	   || TYPE_HAS_TRIVIAL_DFLT (obtype))
 	  && TYPE_HAS_TRIVIAL_DESTRUCTOR (obtype)
 	  && DECL_EXTERNAL (var))
 	declare_weak (fn);
Index: testsuite/g++.dg/cpp0x/pr84497.C
===
--- testsuite/g++.dg/cpp0x/pr84497.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/pr84497.C	(working copy)
@@ -0,0 +1,37 @@
+// PR 84497 mismatch with thread constructor fn weakness
+// { dg-do compile { target c++11 } }
+// { dg-require-weak "" }
+
+struct Base
+{
+  int m;
+
+  Base() noexcept = default;  // trivial but not constexpr
+  ~Base() noexcept = default;
+};
+
+struct Derived : Base {};
+struct Container {
+  Base m;
+};
+
+#ifdef DEF
+// This bit for exposition only.
+// All items placed in .tbss
+// __tls_init simply sets __tls_guard
+// no aliases to __tls_init generated
+thread_local Base base_obj;
+thread_local Derived derived_obj;
+thread_local Container container_obj;
+#else
+// Erroneously created strong undef refs to
+// _ZTH11derived_obj, _ZTH13container_obj, _ZTH8base_obj
+extern thread_local Base base_obj;
+extern thread_local Derived derived_obj;
+extern thread_local Container container_obj;
+int main() { return !(&base_obj && &derived_obj && &container_obj);}
+#endif
+
+// { dg-final { scan-assembler ".weak\[ \t\]*_ZTH8base_obj" } }
+// { dg-final { scan-assembler ".weak\[ \t\]*_ZTH11derived_obj" } }
+// { dg-final { scan-assembler ".weak\[ \t\]*_ZTH13container_obj" } }

Re: [PR c++/84497] ref to undefined tls init

2018-03-02 Thread Jason Merrill 
On Fri, Mar 2, 2018 at 11:07 AM, Nathan Sidwell  wrote:
> Jason, Jakub,
> I've simplified the testcase Padraig provided and attached it to
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84497
>
> Notice that 'Base::Base ()' is not a constexpr ctor, because the integer
> member is not initialized.  That means 10.1.4/4.5 is unsatisfied.  So struct
> Base gets its 'has_constexpr' flag set via the Base::Base (int) ctor.
> struct Derived is not so lucky, and 'has_constexpr' is unset.
>
> Thus we end up with an unsatisfied strong reference to _ZTH11derived_obj
>
> NEEDS_CONSTRUCTING && !HAS_CONSTEXPR_CTOR is insufficient to determine
> whether there must be an init fn.
>
> While the patch does indeed work, it may be too pessimal, making the
> reference weak in more cases than necessary.
>
> NEEDS_CONSTRUCTING && !HAS_CONSTEXPR_CTOR && !HAS_DEFAULT_CONSTRUCTOR seems
> like it would be sufficient. and indeed that works in this case.

Do you mean !HAS_TRIVIAL_DFLT rather than !HAS_DEFAULT_CONSTRUCTOR?
That makes sense to me.

Jason

[PR c++/84497] ref to undefined tls init

2018-03-02 Thread Nathan Sidwell 

Jason, Jakub,
I've simplified the testcase Padraig provided and attached it to 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84497


Notice that 'Base::Base ()' is not a constexpr ctor, because the integer 
member is not initialized.  That means 10.1.4/4.5 is unsatisfied.  So 
struct Base gets its 'has_constexpr' flag set via the Base::Base (int) 
ctor.  struct Derived is not so lucky, and 'has_constexpr' is unset.


Thus we end up with an unsatisfied strong reference to _ZTH11derived_obj

NEEDS_CONSTRUCTING && !HAS_CONSTEXPR_CTOR is insufficient to determine 
whether there must be an init fn.


While the patch does indeed work, it may be too pessimal, making the 
reference weak in more cases than necessary.


NEEDS_CONSTRUCTING && !HAS_CONSTEXPR_CTOR && !HAS_DEFAULT_CONSTRUCTOR 
seems like it would be sufficient. and indeed that works in this case.


WDYT?

nathan

--
Nathan Sidwell