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

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

[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

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

[Jason Merrill]

On Fri, Mar 2, 2018 at 11:07 AM, Nathan Sidwell <nathan@acm.org> 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

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

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 408 bytes --]

On 03/02/2018 01:43 PM, Jason Merrill wrote:
> On Fri, Mar 2, 2018 at 11:07 AM, Nathan Sidwell <nathan@acm.org> 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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 84497.diff --]
[-- Type: text/x-patch; name="84497.diff", Size: 2191 bytes --]

2018-03-05  Pádraig Brady  <P@draigBrady.com>
	    Jason  Merrill  <jason@redhat.com>
	    Nathan Sidwell  <nathan@acm.org>

	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" } }