public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] PR71482: Add -Wglobal-constructors
@ 2019-05-28 22:06 Sean Gillespie
  2019-05-30 19:44 ` Marek Polacek
  2019-05-30 23:49 ` Martin Sebor
  0 siblings, 2 replies; 12+ messages in thread
From: Sean Gillespie @ 2019-05-28 22:06 UTC (permalink / raw)
  To: gcc-patches; +Cc: Sean Gillespie

This adds a new warning, -Wglobal-constructors, that warns whenever a
decl requires a global constructor or destructor. This new warning fires
whenever a thread_local or global variable is declared whose type has a
non-trivial constructor or destructor. When faced with both a constructor
and a destructor, the error message mentions the destructor and is only
fired once.

This warning mirrors the Clang option -Wglobal-constructors, which warns
on the same thing. -Wglobal-constructors was present in Apple's GCC and
later made its way into Clang.

Bootstrapped and regression-tested on x86-64 linux, new tests passing.

gcc/ChangeLog:

2019-05-28  Sean Gillespie  <sean@swgillespie.me>

	* doc/invoke.texi: Add new flag -Wglobal-constructors.

gcc/c-family/ChangeLog:

2019-05-28  Sean Gillespie  <sean@swgillespie.me>

	* c.opt: Add new flag -Wglobal-constructors.

gcc/cp/ChangeLog:

2019-05-28  Sean Gillespie  <sean@swgillespie.me>

	* decl.c (expand_static_init): Warn if a thread local or static decl
	requires a non-trivial constructor or destructor.

gcc/testsuite/ChangeLog:

2019-05-28  Sean Gillespie  <sean@swgillespie.me>

	* g++.dg/warn/global-constructors-1.C: New test.
	* g++.dg/warn/global-constructors-2.C: New test.
---
 gcc/c-family/c.opt                            |  4 +++
 gcc/cp/decl.c                                 | 17 ++++++++++---
 gcc/doc/invoke.texi                           |  7 ++++++
 .../g++.dg/warn/global-constructors-1.C       | 25 +++++++++++++++++++
 .../g++.dg/warn/global-constructors-2.C       | 23 +++++++++++++++++
 5 files changed, 73 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/global-constructors-1.C
 create mode 100644 gcc/testsuite/g++.dg/warn/global-constructors-2.C

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 046d489f7eb..cf62625ca42 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -613,6 +613,10 @@ Wformat-truncation=
 C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) Warning LangEnabledBy(C ObjC C++ LTO ObjC++,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
 Warn about calls to snprintf and similar functions that truncate output.
 
+Wglobal-constructors
+C++ ObjC++ Var(warn_global_constructors) Warning
+Warn when a global declaration requires a constructor to initialize.
+
 Wif-not-aligned
 C ObjC C++ ObjC++ Var(warn_if_not_aligned) Init(1) Warning
 Warn when the field in a struct is not aligned.
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 19d14a6a5e9..c1d66195bd7 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -8467,10 +8467,21 @@ expand_static_init (tree decl, tree init)
       finish_then_clause (if_stmt);
       finish_if_stmt (if_stmt);
     }
-  else if (CP_DECL_THREAD_LOCAL_P (decl))
-    tls_aggregates = tree_cons (init, decl, tls_aggregates);
   else
-    static_aggregates = tree_cons (init, decl, static_aggregates);
+   {
+    if (CP_DECL_THREAD_LOCAL_P (decl))
+      tls_aggregates = tree_cons (init, decl, tls_aggregates);
+    else
+      static_aggregates = tree_cons (init, decl, static_aggregates);
+
+    if (warn_global_constructors)
+     {
+      if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (decl)))
+        warning(OPT_Wglobal_constructors, "declaration requires a global destructor");
+      else
+        warning(OPT_Wglobal_constructors, "declaration requires a global constructor");
+     }
+   }
 }
 
 \f
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4964cc41ba3..a9b2e47a302 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -312,6 +312,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wformat-security  -Wformat-signedness  -Wformat-truncation=@var{n} @gol
 -Wformat-y2k  -Wframe-address @gol
 -Wframe-larger-than=@var{byte-size}  -Wno-free-nonheap-object @gol
+-Wglobal-constructors @gol
 -Wjump-misses-init @gol
 -Whsa  -Wif-not-aligned @gol
 -Wignored-qualifiers  -Wignored-attributes  -Wincompatible-pointer-types @gol
@@ -6509,6 +6510,12 @@ to @option{-Wframe-larger-than=}@samp{SIZE_MAX} or larger.
 Do not warn when attempting to free an object that was not allocated
 on the heap.
 
+@item -Wglobal-constructors @r{(C++ and Objective-C++ only)}
+@opindex Wglobal-constructors
+@opindex Wno-global-constructors
+Warn if the compiler detects a global declaration that requires
+a constructor to initialize.
+
 @item -Wstack-usage=@var{byte-size}
 @opindex Wstack-usage
 @opindex Wno-stack-usage
diff --git a/gcc/testsuite/g++.dg/warn/global-constructors-1.C b/gcc/testsuite/g++.dg/warn/global-constructors-1.C
new file mode 100644
index 00000000000..5a6869e9dcd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/global-constructors-1.C
@@ -0,0 +1,25 @@
+// { dg-do compile }
+// { dg-options "-Wglobal-constructors" }
+
+struct with_ctor {
+    int x;
+    with_ctor() : x(42) {}
+};
+
+struct with_dtor {
+    ~with_dtor() {}
+};
+
+struct with_both {
+    int x;
+    with_both() : x(42) {}
+    ~with_both() {}
+};
+
+with_ctor global_var; /* { dg-warning "declaration requires a global constructor" } */
+with_dtor global_var2; /* { dg-warning "declaration requires a global destructor" } */
+with_both global_var3; /* { dg-warning "declaration requires a global destructor" } */
+
+int main() {
+    static with_ctor global_var; /* { dg-bogus "declaration requires a global constructor" } */
+}
diff --git a/gcc/testsuite/g++.dg/warn/global-constructors-2.C b/gcc/testsuite/g++.dg/warn/global-constructors-2.C
new file mode 100644
index 00000000000..e753b9cb507
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/global-constructors-2.C
@@ -0,0 +1,23 @@
+// { dg-do compile }
+// { dg-require-effective-target c++11 }
+// { dg-options "-Wglobal-constructors" }
+
+struct with_ctor {
+    int x;
+
+    with_ctor() : x(42) {}
+};
+
+struct with_dtor {
+    ~with_dtor() {}
+};
+
+struct with_both {
+    int x;
+    with_both() : x(42) {}
+    ~with_both() {}
+};
+
+thread_local with_ctor global_var; /* { dg-warning "declaration requires a global constructor" } */
+thread_local with_dtor global_var2; /* { dg-warning "declaration requires a global destructor" } */
+thread_local with_both global_var3; /* { dg-warning "declaration requires a global destructor" } */
-- 
2.20.1


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

* Re: [PATCH] PR71482: Add -Wglobal-constructors
  2019-05-28 22:06 [PATCH] PR71482: Add -Wglobal-constructors Sean Gillespie
@ 2019-05-30 19:44 ` Marek Polacek
  2019-05-30 22:56   ` Sean Gillespie
  2019-05-30 23:49 ` Martin Sebor
  1 sibling, 1 reply; 12+ messages in thread
From: Marek Polacek @ 2019-05-30 19:44 UTC (permalink / raw)
  To: Sean Gillespie; +Cc: gcc-patches

On Tue, May 28, 2019 at 09:30:56PM +0000, Sean Gillespie wrote:
> This adds a new warning, -Wglobal-constructors, that warns whenever a
> decl requires a global constructor or destructor. This new warning fires
> whenever a thread_local or global variable is declared whose type has a
> non-trivial constructor or destructor. When faced with both a constructor
> and a destructor, the error message mentions the destructor and is only
> fired once.
> 
> This warning mirrors the Clang option -Wglobal-constructors, which warns
> on the same thing. -Wglobal-constructors was present in Apple's GCC and
> later made its way into Clang.
> 
> Bootstrapped and regression-tested on x86-64 linux, new tests passing.

Thanks for the patch!  Do you have a copyright assignment on file?  Unsure
if this patch is small enough not to need it.

> gcc/ChangeLog:
> 
> 2019-05-28  Sean Gillespie  <sean@swgillespie.me>
> 
> 	* doc/invoke.texi: Add new flag -Wglobal-constructors.
> 
> gcc/c-family/ChangeLog:
> 
> 2019-05-28  Sean Gillespie  <sean@swgillespie.me>
> 
> 	* c.opt: Add new flag -Wglobal-constructors.
> 
> gcc/cp/ChangeLog:
> 
> 2019-05-28  Sean Gillespie  <sean@swgillespie.me>
> 
> 	* decl.c (expand_static_init): Warn if a thread local or static decl
> 	requires a non-trivial constructor or destructor.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-05-28  Sean Gillespie  <sean@swgillespie.me>
> 
> 	* g++.dg/warn/global-constructors-1.C: New test.
> 	* g++.dg/warn/global-constructors-2.C: New test.

These are fine but please also mention "PR c++/71482" in them.

> ---
>  gcc/c-family/c.opt                            |  4 +++
>  gcc/cp/decl.c                                 | 17 ++++++++++---
>  gcc/doc/invoke.texi                           |  7 ++++++
>  .../g++.dg/warn/global-constructors-1.C       | 25 +++++++++++++++++++
>  .../g++.dg/warn/global-constructors-2.C       | 23 +++++++++++++++++
>  5 files changed, 73 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/warn/global-constructors-1.C
>  create mode 100644 gcc/testsuite/g++.dg/warn/global-constructors-2.C
> 
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index 046d489f7eb..cf62625ca42 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -613,6 +613,10 @@ Wformat-truncation=
>  C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) Warning LangEnabledBy(C ObjC C++ LTO ObjC++,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
>  Warn about calls to snprintf and similar functions that truncate output.
>  
> +Wglobal-constructors
> +C++ ObjC++ Var(warn_global_constructors) Warning
> +Warn when a global declaration requires a constructor to initialize.
> +

Ok, I agree this shouldn't be turned on by -Wall or -Wextra.

> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index 19d14a6a5e9..c1d66195bd7 100644
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -8467,10 +8467,21 @@ expand_static_init (tree decl, tree init)
>        finish_then_clause (if_stmt);
>        finish_if_stmt (if_stmt);
>      }
> -  else if (CP_DECL_THREAD_LOCAL_P (decl))
> -    tls_aggregates = tree_cons (init, decl, tls_aggregates);
>    else
> -    static_aggregates = tree_cons (init, decl, static_aggregates);
> +   {
> +    if (CP_DECL_THREAD_LOCAL_P (decl))
> +      tls_aggregates = tree_cons (init, decl, tls_aggregates);
> +    else
> +      static_aggregates = tree_cons (init, decl, static_aggregates);
> +
> +    if (warn_global_constructors)

This check is not wrong but you can probably drop it.  We use it if the warning
is expensive, but this doesn't seem to be the case.

> +     {
> +      if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (decl)))
> +        warning(OPT_Wglobal_constructors, "declaration requires a global destructor");
> +      else
> +        warning(OPT_Wglobal_constructors, "declaration requires a global constructor");
> +     }
> +   }

The formatting a bit wrong, please use two spaces to indent.  There should be
a space before a (.  And the lines should not exceed 80 characters.

I think it would be better to use warning_at instead of warning.  As the
location, you can use 'dloc' defined earlier in the function if you move
it out of the block.


It seems this patch will also warn for

struct A {
  A() = default;
};

A a;

which I think we don't want?

I also think you want to add a test using a constexpr constructor.

Marek

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

* Re: [PATCH] PR71482: Add -Wglobal-constructors
  2019-05-30 19:44 ` Marek Polacek
@ 2019-05-30 22:56   ` Sean Gillespie
  0 siblings, 0 replies; 12+ messages in thread
From: Sean Gillespie @ 2019-05-30 22:56 UTC (permalink / raw)
  To: Marek Polacek; +Cc: gcc-patches

Thank you for the review!

On Thu, May 30, 2019 at 12:38 PM Marek Polacek <polacek@redhat.com> wrote:
>
> Thanks for the patch!  Do you have a copyright assignment on file?  Unsure
> if this patch is small enough not to need it.
>

Also unsure, though I assumed that this patch was small enough to not need one.
I'll go ahead and do it regardless.

> > gcc/ChangeLog:
> >
> > 2019-05-28  Sean Gillespie  <sean@swgillespie.me>
> >
> >       * doc/invoke.texi: Add new flag -Wglobal-constructors.
> >
> > gcc/c-family/ChangeLog:
> >
> > 2019-05-28  Sean Gillespie  <sean@swgillespie.me>
> >
> >       * c.opt: Add new flag -Wglobal-constructors.
> >
> > gcc/cp/ChangeLog:
> >
> > 2019-05-28  Sean Gillespie  <sean@swgillespie.me>
> >
> >       * decl.c (expand_static_init): Warn if a thread local or static decl
> >       requires a non-trivial constructor or destructor.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2019-05-28  Sean Gillespie  <sean@swgillespie.me>
> >
> >       * g++.dg/warn/global-constructors-1.C: New test.
> >       * g++.dg/warn/global-constructors-2.C: New test.
>
> These are fine but please also mention "PR c++/71482" in them.
>

Will fix.

> > ---
> >  gcc/c-family/c.opt                            |  4 +++
> >  gcc/cp/decl.c                                 | 17 ++++++++++---
> >  gcc/doc/invoke.texi                           |  7 ++++++
> >  .../g++.dg/warn/global-constructors-1.C       | 25 +++++++++++++++++++
> >  .../g++.dg/warn/global-constructors-2.C       | 23 +++++++++++++++++
> >  5 files changed, 73 insertions(+), 3 deletions(-)
> >  create mode 100644 gcc/testsuite/g++.dg/warn/global-constructors-1.C
> >  create mode 100644 gcc/testsuite/g++.dg/warn/global-constructors-2.C
> >
> > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> > index 046d489f7eb..cf62625ca42 100644
> > --- a/gcc/c-family/c.opt
> > +++ b/gcc/c-family/c.opt
> > @@ -613,6 +613,10 @@ Wformat-truncation=
> >  C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) Warning LangEnabledBy(C ObjC C++ LTO ObjC++,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
> >  Warn about calls to snprintf and similar functions that truncate output.
> >
> > +Wglobal-constructors
> > +C++ ObjC++ Var(warn_global_constructors) Warning
> > +Warn when a global declaration requires a constructor to initialize.
> > +
>
> Ok, I agree this shouldn't be turned on by -Wall or -Wextra.

Yeah - the way I use this with clang is to forbid global constructors
with -Werror=global-constructors and -Wglobal-constructors. Since
this is a legitimate language feature I don't think it should be
included in either of those catch-all warning sets.

>
> > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> > index 19d14a6a5e9..c1d66195bd7 100644
> > --- a/gcc/cp/decl.c
> > +++ b/gcc/cp/decl.c
> > @@ -8467,10 +8467,21 @@ expand_static_init (tree decl, tree init)
> >        finish_then_clause (if_stmt);
> >        finish_if_stmt (if_stmt);
> >      }
> > -  else if (CP_DECL_THREAD_LOCAL_P (decl))
> > -    tls_aggregates = tree_cons (init, decl, tls_aggregates);
> >    else
> > -    static_aggregates = tree_cons (init, decl, static_aggregates);
> > +   {
> > +    if (CP_DECL_THREAD_LOCAL_P (decl))
> > +      tls_aggregates = tree_cons (init, decl, tls_aggregates);
> > +    else
> > +      static_aggregates = tree_cons (init, decl, static_aggregates);
> > +
> > +    if (warn_global_constructors)
>
> This check is not wrong but you can probably drop it.  We use it if the warning
> is expensive, but this doesn't seem to be the case.
>

Will fix.

> > +     {
> > +      if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (decl)))
> > +        warning(OPT_Wglobal_constructors, "declaration requires a global destructor");
> > +      else
> > +        warning(OPT_Wglobal_constructors, "declaration requires a global constructor");
> > +     }
> > +   }
>
> The formatting a bit wrong, please use two spaces to indent.  There should be
> a space before a (.  And the lines should not exceed 80 characters.
>
> I think it would be better to use warning_at instead of warning.  As the
> location, you can use 'dloc' defined earlier in the function if you move
> it out of the block.
>

Will fix.

>
> It seems this patch will also warn for
>
> struct A {
>   A() = default;
> };
>
> A a;
>
> which I think we don't want?
>
> I also think you want to add a test using a constexpr constructor.
>
> Marek

You're completely right. I have a modified version of this patch that
expands the test coverage to include these cases while also
handling the general problem of constant and non-constant
initializers. I'll submit it shortly.


Sean Gillespie

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

* Re: [PATCH] PR71482: Add -Wglobal-constructors
  2019-05-28 22:06 [PATCH] PR71482: Add -Wglobal-constructors Sean Gillespie
  2019-05-30 19:44 ` Marek Polacek
@ 2019-05-30 23:49 ` Martin Sebor
  2019-05-31 15:56   ` Marek Polacek
  2019-05-31 16:25   ` Sean Gillespie
  1 sibling, 2 replies; 12+ messages in thread
From: Martin Sebor @ 2019-05-30 23:49 UTC (permalink / raw)
  To: Sean Gillespie, gcc-patches

On 5/28/19 3:30 PM, Sean Gillespie wrote:
> This adds a new warning, -Wglobal-constructors, that warns whenever a
> decl requires a global constructor or destructor. This new warning fires
> whenever a thread_local or global variable is declared whose type has a
> non-trivial constructor or destructor. When faced with both a constructor
> and a destructor, the error message mentions the destructor and is only
> fired once.
> 
> This warning mirrors the Clang option -Wglobal-constructors, which warns
> on the same thing. -Wglobal-constructors was present in Apple's GCC and
> later made its way into Clang.
> 
> Bootstrapped and regression-tested on x86-64 linux, new tests passing.

I can't tell from the Clang online manual:

Is the warning meant to trigger just for globals, or for all
objects with static storage duration regardless of scope (i.e.,
including namespace-scope objects and static locals and static
class members)?

"Requires a constructor to initialize" doesn't seem very clear
to me.  Is the warning intended to trigger for objects that
require dynamic initialization?  If so, then what about dynamic
intialization of objects of trivial types, such as this:

   static int i = std::string ("foo").length ();

or even

   static int j = strlen (getenv ("TMP"));

If these aren't meant to be diagnosed then the description should
make it clear (the first one involves a ctor, the second one does
not).  But I would think that if the goal is to find sources of
dynamic initialization then diagnosing the above would be useful.
If so, the description should make it clear and tests verifying
that it works should be added.

Martin

PS Dynamic initialization can be optimized into static
initialization, even when it involves a user-defined constructor.
If the purpose of the warning is to find objects that are
dynamically initialized in the sense of the C++ language then
implementing it in the front-end is sufficient.  But if the goal
is to detect constructors that will actually run at runtime (i.e.,
have a startup cost and may have dependencies on one another) then
it needs to be implemented much later.

> 
> gcc/ChangeLog:
> 
> 2019-05-28  Sean Gillespie  <sean@swgillespie.me>
> 
> 	* doc/invoke.texi: Add new flag -Wglobal-constructors.
> 
> gcc/c-family/ChangeLog:
> 
> 2019-05-28  Sean Gillespie  <sean@swgillespie.me>
> 
> 	* c.opt: Add new flag -Wglobal-constructors.
> 
> gcc/cp/ChangeLog:
> 
> 2019-05-28  Sean Gillespie  <sean@swgillespie.me>
> 
> 	* decl.c (expand_static_init): Warn if a thread local or static decl
> 	requires a non-trivial constructor or destructor.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-05-28  Sean Gillespie  <sean@swgillespie.me>
> 
> 	* g++.dg/warn/global-constructors-1.C: New test.
> 	* g++.dg/warn/global-constructors-2.C: New test.
> ---
>   gcc/c-family/c.opt                            |  4 +++
>   gcc/cp/decl.c                                 | 17 ++++++++++---
>   gcc/doc/invoke.texi                           |  7 ++++++
>   .../g++.dg/warn/global-constructors-1.C       | 25 +++++++++++++++++++
>   .../g++.dg/warn/global-constructors-2.C       | 23 +++++++++++++++++
>   5 files changed, 73 insertions(+), 3 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/warn/global-constructors-1.C
>   create mode 100644 gcc/testsuite/g++.dg/warn/global-constructors-2.C
> 
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index 046d489f7eb..cf62625ca42 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -613,6 +613,10 @@ Wformat-truncation=
>   C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) Warning LangEnabledBy(C ObjC C++ LTO ObjC++,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
>   Warn about calls to snprintf and similar functions that truncate output.
>   
> +Wglobal-constructors
> +C++ ObjC++ Var(warn_global_constructors) Warning
> +Warn when a global declaration requires a constructor to initialize.
> +
>   Wif-not-aligned
>   C ObjC C++ ObjC++ Var(warn_if_not_aligned) Init(1) Warning
>   Warn when the field in a struct is not aligned.
> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index 19d14a6a5e9..c1d66195bd7 100644
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -8467,10 +8467,21 @@ expand_static_init (tree decl, tree init)
>         finish_then_clause (if_stmt);
>         finish_if_stmt (if_stmt);
>       }
> -  else if (CP_DECL_THREAD_LOCAL_P (decl))
> -    tls_aggregates = tree_cons (init, decl, tls_aggregates);
>     else
> -    static_aggregates = tree_cons (init, decl, static_aggregates);
> +   {
> +    if (CP_DECL_THREAD_LOCAL_P (decl))
> +      tls_aggregates = tree_cons (init, decl, tls_aggregates);
> +    else
> +      static_aggregates = tree_cons (init, decl, static_aggregates);
> +
> +    if (warn_global_constructors)
> +     {
> +      if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (decl)))
> +        warning(OPT_Wglobal_constructors, "declaration requires a global destructor");
> +      else
> +        warning(OPT_Wglobal_constructors, "declaration requires a global constructor");
> +     }
> +   }
>   }
>   
>   \f
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 4964cc41ba3..a9b2e47a302 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -312,6 +312,7 @@ Objective-C and Objective-C++ Dialects}.
>   -Wformat-security  -Wformat-signedness  -Wformat-truncation=@var{n} @gol
>   -Wformat-y2k  -Wframe-address @gol
>   -Wframe-larger-than=@var{byte-size}  -Wno-free-nonheap-object @gol
> +-Wglobal-constructors @gol
>   -Wjump-misses-init @gol
>   -Whsa  -Wif-not-aligned @gol
>   -Wignored-qualifiers  -Wignored-attributes  -Wincompatible-pointer-types @gol
> @@ -6509,6 +6510,12 @@ to @option{-Wframe-larger-than=}@samp{SIZE_MAX} or larger.
>   Do not warn when attempting to free an object that was not allocated
>   on the heap.
>   
> +@item -Wglobal-constructors @r{(C++ and Objective-C++ only)}
> +@opindex Wglobal-constructors
> +@opindex Wno-global-constructors
> +Warn if the compiler detects a global declaration that requires
> +a constructor to initialize.
> +
>   @item -Wstack-usage=@var{byte-size}
>   @opindex Wstack-usage
>   @opindex Wno-stack-usage
> diff --git a/gcc/testsuite/g++.dg/warn/global-constructors-1.C b/gcc/testsuite/g++.dg/warn/global-constructors-1.C
> new file mode 100644
> index 00000000000..5a6869e9dcd
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/global-constructors-1.C
> @@ -0,0 +1,25 @@
> +// { dg-do compile }
> +// { dg-options "-Wglobal-constructors" }
> +
> +struct with_ctor {
> +    int x;
> +    with_ctor() : x(42) {}
> +};
> +
> +struct with_dtor {
> +    ~with_dtor() {}
> +};
> +
> +struct with_both {
> +    int x;
> +    with_both() : x(42) {}
> +    ~with_both() {}
> +};
> +
> +with_ctor global_var; /* { dg-warning "declaration requires a global constructor" } */
> +with_dtor global_var2; /* { dg-warning "declaration requires a global destructor" } */
> +with_both global_var3; /* { dg-warning "declaration requires a global destructor" } */
> +
> +int main() {
> +    static with_ctor global_var; /* { dg-bogus "declaration requires a global constructor" } */
> +}
> diff --git a/gcc/testsuite/g++.dg/warn/global-constructors-2.C b/gcc/testsuite/g++.dg/warn/global-constructors-2.C
> new file mode 100644
> index 00000000000..e753b9cb507
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/global-constructors-2.C
> @@ -0,0 +1,23 @@
> +// { dg-do compile }
> +// { dg-require-effective-target c++11 }
> +// { dg-options "-Wglobal-constructors" }
> +
> +struct with_ctor {
> +    int x;
> +
> +    with_ctor() : x(42) {}
> +};
> +
> +struct with_dtor {
> +    ~with_dtor() {}
> +};
> +
> +struct with_both {
> +    int x;
> +    with_both() : x(42) {}
> +    ~with_both() {}
> +};
> +
> +thread_local with_ctor global_var; /* { dg-warning "declaration requires a global constructor" } */
> +thread_local with_dtor global_var2; /* { dg-warning "declaration requires a global destructor" } */
> +thread_local with_both global_var3; /* { dg-warning "declaration requires a global destructor" } */
> 

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

* Re: [PATCH] PR71482: Add -Wglobal-constructors
  2019-05-30 23:49 ` Martin Sebor
@ 2019-05-31 15:56   ` Marek Polacek
  2019-05-31 16:27     ` Marek Polacek
  2019-05-31 16:25   ` Sean Gillespie
  1 sibling, 1 reply; 12+ messages in thread
From: Marek Polacek @ 2019-05-31 15:56 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Sean Gillespie, gcc-patches

On Thu, May 30, 2019 at 05:28:47PM -0600, Martin Sebor wrote:
> On 5/28/19 3:30 PM, Sean Gillespie wrote:
> > This adds a new warning, -Wglobal-constructors, that warns whenever a
> > decl requires a global constructor or destructor. This new warning fires
> > whenever a thread_local or global variable is declared whose type has a
> > non-trivial constructor or destructor. When faced with both a constructor
> > and a destructor, the error message mentions the destructor and is only
> > fired once.
> > 
> > This warning mirrors the Clang option -Wglobal-constructors, which warns
> > on the same thing. -Wglobal-constructors was present in Apple's GCC and
> > later made its way into Clang.
> > 
> > Bootstrapped and regression-tested on x86-64 linux, new tests passing.
> 
> I can't tell from the Clang online manual:
> 
> Is the warning meant to trigger just for globals, or for all
> objects with static storage duration regardless of scope (i.e.,
> including namespace-scope objects and static locals and static
> class members)?
> 
> "Requires a constructor to initialize" doesn't seem very clear
> to me.  Is the warning intended to trigger for objects that
> require dynamic initialization?  If so, then what about dynamic
> intialization of objects of trivial types, such as this:
> 
>   static int i = std::string ("foo").length ();
> 
> or even
> 
>   static int j = strlen (getenv ("TMP"));

The warning is not meant to diagnose these.  But I do agree that the
documentation for the new warning should be improved.

> If these aren't meant to be diagnosed then the description should
> make it clear (the first one involves a ctor, the second one does
> not).  But I would think that if the goal is to find sources of
> dynamic initialization then diagnosing the above would be useful.
> If so, the description should make it clear and tests verifying
> that it works should be added.
> 
> Martin
> 
> PS Dynamic initialization can be optimized into static
> initialization, even when it involves a user-defined constructor.
> If the purpose of the warning is to find objects that are
> dynamically initialized in the sense of the C++ language then
> implementing it in the front-end is sufficient.  But if the goal

My sense is that this is what we're doing here.  So the patch seems reasonable
to me.  I suspect we'll have to tweak the warning a little bit, but overall it
looks fine to me.  As always, there's room to expand and improve, but the
warning looks useful to me as-is.

Marek

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

* Re: [PATCH] PR71482: Add -Wglobal-constructors
  2019-05-30 23:49 ` Martin Sebor
  2019-05-31 15:56   ` Marek Polacek
@ 2019-05-31 16:25   ` Sean Gillespie
  2019-05-31 16:54     ` Martin Sebor
  1 sibling, 1 reply; 12+ messages in thread
From: Sean Gillespie @ 2019-05-31 16:25 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches

On Thu, May 30, 2019 at 4:28 PM Martin Sebor <msebor@gmail.com> wrote:
>
> On 5/28/19 3:30 PM, Sean Gillespie wrote:
> > This adds a new warning, -Wglobal-constructors, that warns whenever a
> > decl requires a global constructor or destructor. This new warning fires
> > whenever a thread_local or global variable is declared whose type has a
> > non-trivial constructor or destructor. When faced with both a constructor
> > and a destructor, the error message mentions the destructor and is only
> > fired once.
> >
> > This warning mirrors the Clang option -Wglobal-constructors, which warns
> > on the same thing. -Wglobal-constructors was present in Apple's GCC and
> > later made its way into Clang.
> >
> > Bootstrapped and regression-tested on x86-64 linux, new tests passing.
>
> I can't tell from the Clang online manual:
>
> Is the warning meant to trigger just for globals, or for all
> objects with static storage duration regardless of scope (i.e.,
> including namespace-scope objects and static locals and static
> class members)?
>
> "Requires a constructor to initialize" doesn't seem very clear
> to me.  Is the warning intended to trigger for objects that
> require dynamic initialization?  If so, then what about dynamic
> intialization of objects of trivial types, such as this:
>
>    static int i = std::string ("foo").length ();
>
> or even
>
>    static int j = strlen (getenv ("TMP"));
>
> If these aren't meant to be diagnosed then the description should
> make it clear (the first one involves a ctor, the second one does
> not).  But I would think that if the goal is to find sources of
> dynamic initialization then diagnosing the above would be useful.
> If so, the description should make it clear and tests verifying
> that it works should be added.
>
> Martin

The answer to both of those is yes. Clang warns on both of these with
-Wglobal-constructors because the goal of -Wglobal-constructors is to
catch and warn against potential life-before-main behavior.

Based on Marek's feedback I'm about to push another patch that also
warns on your above two examples and adds tests for them. The idea
is that any C++-level dynamically initialized globals get warned. Constexpr
constructors or things that otherwise can be statically initialized
are not warned.

Regarding the doc description, how about something like this:

"Warns whenever an object with static storage duration requires
dynamic initialiation,
through global constructors or otherwise."

>
> PS Dynamic initialization can be optimized into static
> initialization, even when it involves a user-defined constructor.
> If the purpose of the warning is to find objects that are
> dynamically initialized in the sense of the C++ language then
> implementing it in the front-end is sufficient.  But if the goal
> is to detect constructors that will actually run at runtime (i.e.,
> have a startup cost and may have dependencies on one another) then
> it needs to be implemented much later.
>

Clang sticks to the C++ language definition of dynamic initialization for the
purposes of this warning and I think we should as well. Without runtime checks
this is a best-effort warning, but it is capable of catching a bunch
of the common
ways that you can get constructors running on startup, which is the goal.

Sean Gillespie

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

* Re: [PATCH] PR71482: Add -Wglobal-constructors
  2019-05-31 15:56   ` Marek Polacek
@ 2019-05-31 16:27     ` Marek Polacek
  2019-05-31 16:29       ` Jakub Jelinek
  0 siblings, 1 reply; 12+ messages in thread
From: Marek Polacek @ 2019-05-31 16:27 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Sean Gillespie, gcc-patches

On Fri, May 31, 2019 at 11:50:47AM -0400, Marek Polacek wrote:
> On Thu, May 30, 2019 at 05:28:47PM -0600, Martin Sebor wrote:
> > On 5/28/19 3:30 PM, Sean Gillespie wrote:
> > > This adds a new warning, -Wglobal-constructors, that warns whenever a
> > > decl requires a global constructor or destructor. This new warning fires
> > > whenever a thread_local or global variable is declared whose type has a
> > > non-trivial constructor or destructor. When faced with both a constructor
> > > and a destructor, the error message mentions the destructor and is only
> > > fired once.
> > > 
> > > This warning mirrors the Clang option -Wglobal-constructors, which warns
> > > on the same thing. -Wglobal-constructors was present in Apple's GCC and
> > > later made its way into Clang.
> > > 
> > > Bootstrapped and regression-tested on x86-64 linux, new tests passing.
> > 
> > I can't tell from the Clang online manual:
> > 
> > Is the warning meant to trigger just for globals, or for all
> > objects with static storage duration regardless of scope (i.e.,
> > including namespace-scope objects and static locals and static
> > class members)?
> > 
> > "Requires a constructor to initialize" doesn't seem very clear
> > to me.  Is the warning intended to trigger for objects that
> > require dynamic initialization?  If so, then what about dynamic
> > intialization of objects of trivial types, such as this:
> > 
> >   static int i = std::string ("foo").length ();
> > 
> > or even
> > 
> >   static int j = strlen (getenv ("TMP"));
> 
> The warning is not meant to diagnose these.  But I do agree that the
> documentation for the new warning should be improved.

I was partially wrong: if i/j are at file scope, then the warning triggers.
But if i/j are in a function, then it doesn't warn.

My apologies.

Marek

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

* Re: [PATCH] PR71482: Add -Wglobal-constructors
  2019-05-31 16:27     ` Marek Polacek
@ 2019-05-31 16:29       ` Jakub Jelinek
  2019-05-31 16:30         ` Marek Polacek
  0 siblings, 1 reply; 12+ messages in thread
From: Jakub Jelinek @ 2019-05-31 16:29 UTC (permalink / raw)
  To: Marek Polacek; +Cc: Martin Sebor, Sean Gillespie, gcc-patches

On Fri, May 31, 2019 at 12:16:58PM -0400, Marek Polacek wrote:
> > The warning is not meant to diagnose these.  But I do agree that the
> > documentation for the new warning should be improved.
> 
> I was partially wrong: if i/j are at file scope, then the warning triggers.
> But if i/j are in a function, then it doesn't warn.

That is correct, because the block scope statics aren't actually initialized
from a global constructor, but with a guard variable whenever a function is
called the first time.  So on
int bar ();
struct S { S (); ~S (); };

void
foo ()
{
  static int a = bar ();
  static S s;
}
this warning shouldn't warn at all.  It might be useful to have another
warning for these, but it should be a different warning.

	Jakub

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

* Re: [PATCH] PR71482: Add -Wglobal-constructors
  2019-05-31 16:29       ` Jakub Jelinek
@ 2019-05-31 16:30         ` Marek Polacek
  0 siblings, 0 replies; 12+ messages in thread
From: Marek Polacek @ 2019-05-31 16:30 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Martin Sebor, Sean Gillespie, gcc-patches

On Fri, May 31, 2019 at 06:25:03PM +0200, Jakub Jelinek wrote:
> On Fri, May 31, 2019 at 12:16:58PM -0400, Marek Polacek wrote:
> > > The warning is not meant to diagnose these.  But I do agree that the
> > > documentation for the new warning should be improved.
> > 
> > I was partially wrong: if i/j are at file scope, then the warning triggers.
> > But if i/j are in a function, then it doesn't warn.
> 
> That is correct, because the block scope statics aren't actually initialized
> from a global constructor, but with a guard variable whenever a function is
> called the first time.  So on
> int bar ();
> struct S { S (); ~S (); };
> 
> void
> foo ()
> {
>   static int a = bar ();
>   static S s;
> }
> this warning shouldn't warn at all.  It might be useful to have another
> warning for these, but it should be a different warning.

Right, agreed -- and the patch already does the right thing.

Marek

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

* Re: [PATCH] PR71482: Add -Wglobal-constructors
  2019-05-31 16:25   ` Sean Gillespie
@ 2019-05-31 16:54     ` Martin Sebor
  2019-06-01  2:01       ` Sean Gillespie
  2019-06-01 18:07       ` [PATCH v2] " Sean Gillespie
  0 siblings, 2 replies; 12+ messages in thread
From: Martin Sebor @ 2019-05-31 16:54 UTC (permalink / raw)
  To: Sean Gillespie; +Cc: gcc-patches

On 5/31/19 10:12 AM, Sean Gillespie wrote:
> On Thu, May 30, 2019 at 4:28 PM Martin Sebor <msebor@gmail.com> wrote:
>>
>> On 5/28/19 3:30 PM, Sean Gillespie wrote:
>>> This adds a new warning, -Wglobal-constructors, that warns whenever a
>>> decl requires a global constructor or destructor. This new warning fires
>>> whenever a thread_local or global variable is declared whose type has a
>>> non-trivial constructor or destructor. When faced with both a constructor
>>> and a destructor, the error message mentions the destructor and is only
>>> fired once.
>>>
>>> This warning mirrors the Clang option -Wglobal-constructors, which warns
>>> on the same thing. -Wglobal-constructors was present in Apple's GCC and
>>> later made its way into Clang.
>>>
>>> Bootstrapped and regression-tested on x86-64 linux, new tests passing.
>>
>> I can't tell from the Clang online manual:
>>
>> Is the warning meant to trigger just for globals, or for all
>> objects with static storage duration regardless of scope (i.e.,
>> including namespace-scope objects and static locals and static
>> class members)?
>>
>> "Requires a constructor to initialize" doesn't seem very clear
>> to me.  Is the warning intended to trigger for objects that
>> require dynamic initialization?  If so, then what about dynamic
>> intialization of objects of trivial types, such as this:
>>
>>     static int i = std::string ("foo").length ();
>>
>> or even
>>
>>     static int j = strlen (getenv ("TMP"));
>>
>> If these aren't meant to be diagnosed then the description should
>> make it clear (the first one involves a ctor, the second one does
>> not).  But I would think that if the goal is to find sources of
>> dynamic initialization then diagnosing the above would be useful.
>> If so, the description should make it clear and tests verifying
>> that it works should be added.
>>
>> Martin
> 
> The answer to both of those is yes. Clang warns on both of these with
> -Wglobal-constructors because the goal of -Wglobal-constructors is to
> catch and warn against potential life-before-main behavior.
> 
> Based on Marek's feedback I'm about to push another patch that also
> warns on your above two examples and adds tests for them. The idea
> is that any C++-level dynamically initialized globals get warned. Constexpr
> constructors or things that otherwise can be statically initialized
> are not warned.

That makes sense.

> 
> Regarding the doc description, how about something like this:
> 
> "Warns whenever an object with static storage duration requires
> dynamic initialiation,
> through global constructors or otherwise."

This makes it clearer -- i.e, global constructors refers to the ELF
concept rather than to the C++ one.  What does the otherwise part
refer to?  Some non-ELF mechanism?

>> PS Dynamic initialization can be optimized into static
>> initialization, even when it involves a user-defined constructor.
>> If the purpose of the warning is to find objects that are
>> dynamically initialized in the sense of the C++ language then
>> implementing it in the front-end is sufficient.  But if the goal
>> is to detect constructors that will actually run at runtime (i.e.,
>> have a startup cost and may have dependencies on one another) then
>> it needs to be implemented much later.
>>
> 
> Clang sticks to the C++ language definition of dynamic initialization for the
> purposes of this warning and I think we should as well. Without runtime checks
> this is a best-effort warning, but it is capable of catching a bunch
> of the common
> ways that you can get constructors running on startup, which is the goal.

It might be worth mentioning that in the manual, perhaps along with
an example showing an instance of the warning that will most likely
be a true positive vs a false positive.  E.g., something like this:

   const char* const tmp = getenv ("TMP");

vs

   const int n = strlen ("123");   // initialized statically

Martin

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

* Re: [PATCH] PR71482: Add -Wglobal-constructors
  2019-05-31 16:54     ` Martin Sebor
@ 2019-06-01  2:01       ` Sean Gillespie
  2019-06-01 18:07       ` [PATCH v2] " Sean Gillespie
  1 sibling, 0 replies; 12+ messages in thread
From: Sean Gillespie @ 2019-06-01  2:01 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches

On Fri, May 31, 2019 at 9:30 AM Martin Sebor <msebor@gmail.com> wrote:
>
> On 5/31/19 10:12 AM, Sean Gillespie wrote:
> > On Thu, May 30, 2019 at 4:28 PM Martin Sebor <msebor@gmail.com> wrote:
> >>
> >> On 5/28/19 3:30 PM, Sean Gillespie wrote:
> >>> This adds a new warning, -Wglobal-constructors, that warns whenever a
> >>> decl requires a global constructor or destructor. This new warning fires
> >>> whenever a thread_local or global variable is declared whose type has a
> >>> non-trivial constructor or destructor. When faced with both a constructor
> >>> and a destructor, the error message mentions the destructor and is only
> >>> fired once.
> >>>
> >>> This warning mirrors the Clang option -Wglobal-constructors, which warns
> >>> on the same thing. -Wglobal-constructors was present in Apple's GCC and
> >>> later made its way into Clang.
> >>>
> >>> Bootstrapped and regression-tested on x86-64 linux, new tests passing.
> >>
> >> I can't tell from the Clang online manual:
> >>
> >> Is the warning meant to trigger just for globals, or for all
> >> objects with static storage duration regardless of scope (i.e.,
> >> including namespace-scope objects and static locals and static
> >> class members)?
> >>
> >> "Requires a constructor to initialize" doesn't seem very clear
> >> to me.  Is the warning intended to trigger for objects that
> >> require dynamic initialization?  If so, then what about dynamic
> >> intialization of objects of trivial types, such as this:
> >>
> >>     static int i = std::string ("foo").length ();
> >>
> >> or even
> >>
> >>     static int j = strlen (getenv ("TMP"));
> >>
> >> If these aren't meant to be diagnosed then the description should
> >> make it clear (the first one involves a ctor, the second one does
> >> not).  But I would think that if the goal is to find sources of
> >> dynamic initialization then diagnosing the above would be useful.
> >> If so, the description should make it clear and tests verifying
> >> that it works should be added.
> >>
> >> Martin
> >
> > The answer to both of those is yes. Clang warns on both of these with
> > -Wglobal-constructors because the goal of -Wglobal-constructors is to
> > catch and warn against potential life-before-main behavior.
> >
> > Based on Marek's feedback I'm about to push another patch that also
> > warns on your above two examples and adds tests for them. The idea
> > is that any C++-level dynamically initialized globals get warned. Constexpr
> > constructors or things that otherwise can be statically initialized
> > are not warned.
>
> That makes sense.
>
> >
> > Regarding the doc description, how about something like this:
> >
> > "Warns whenever an object with static storage duration requires
> > dynamic initialiation,
> > through global constructors or otherwise."
>
> This makes it clearer -- i.e, global constructors refers to the ELF
> concept rather than to the C++ one.  What does the otherwise part
> refer to?  Some non-ELF mechanism?

Ah, I was thinking "otherwise" to mean the C++ sense of constructor,
and not the ELF one. I realize now that it's clearer without "for
otherwise", since then "global constructors" would refer to just the
ELF concept.

>
> >> PS Dynamic initialization can be optimized into static
> >> initialization, even when it involves a user-defined constructor.
> >> If the purpose of the warning is to find objects that are
> >> dynamically initialized in the sense of the C++ language then
> >> implementing it in the front-end is sufficient.  But if the goal
> >> is to detect constructors that will actually run at runtime (i.e.,
> >> have a startup cost and may have dependencies on one another) then
> >> it needs to be implemented much later.
> >>
> >
> > Clang sticks to the C++ language definition of dynamic initialization for the
> > purposes of this warning and I think we should as well. Without runtime checks
> > this is a best-effort warning, but it is capable of catching a bunch
> > of the common
> > ways that you can get constructors running on startup, which is the goal.
>
> It might be worth mentioning that in the manual, perhaps along with
> an example showing an instance of the warning that will most likely
> be a true positive vs a false positive.  E.g., something like this:
>
>    const char* const tmp = getenv ("TMP");
>
> vs
>
>    const int n = strlen ("123");   // initialized statically
>

Will do.

Sean

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

* [PATCH v2] PR71482: Add -Wglobal-constructors
  2019-05-31 16:54     ` Martin Sebor
  2019-06-01  2:01       ` Sean Gillespie
@ 2019-06-01 18:07       ` Sean Gillespie
  1 sibling, 0 replies; 12+ messages in thread
From: Sean Gillespie @ 2019-06-01 18:07 UTC (permalink / raw)
  To: gcc-patches; +Cc: Sean Gillespie

This adds a new warning, -Wglobal-constructors, that warns whenever a
decl requires a global constructor or destructor. Global destructors are
required whenever a decl with thread_local or global storage is declared
with a type with a nontrivial destructor. Global constructors are required
whenever a declaration's initializer is a non-trivial, non-constant
initializtion.

This warning mirrors the Clang option -Wglobal-constructors, which warns
on the same thing. -Wglobal-constructors was present in Apple's GCC and
later made its way into Clang.

Bootstrapped and regression-tested on x86-64 linux, new tests passing.

gcc/ChangeLog:

2019-05-28  Sean Gillespie  <sean@swgillespie.me>

	PR c++/71482
	* doc/invoke.texi: Add new flag -Wglobal-constructors.

gcc/c-family/ChangeLog:

2019-05-28  Sean Gillespie  <sean@swgillespie.me>

	PR c++/71482
	* c.opt: Add new flag -Wglobal-constructors.

gcc/cp/ChangeLog:

2019-05-28  Sean Gillespie  <sean@swgillespie.me>

	PR c++/71482
	* decl.c (expand_static_init): Warn if a thread local or static decl
	requires a non-trivial constructor or destructor.

gcc/testsuite/ChangeLog:

2019-05-28  Sean Gillespie  <sean@swgillespie.me>

	PR c++/71482
	* g++.dg/warn/global-constructors-1.C: New test.
	* g++.dg/warn/global-constructors-2.C: New test.
---
 gcc/c-family/c.opt                            |  4 ++
 gcc/cp/decl.c                                 | 22 ++++++--
 gcc/doc/invoke.texi                           | 35 ++++++++++++
 .../g++.dg/warn/global-constructors-1.C       | 53 +++++++++++++++++++
 .../g++.dg/warn/global-constructors-2.C       | 49 +++++++++++++++++
 5 files changed, 159 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/global-constructors-1.C
 create mode 100644 gcc/testsuite/g++.dg/warn/global-constructors-2.C

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 046d489f7eb..21f12d4f7b2 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -613,6 +613,10 @@ Wformat-truncation=
 C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) Warning LangEnabledBy(C ObjC C++ LTO ObjC++,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
 Warn about calls to snprintf and similar functions that truncate output.
 
+Wglobal-constructors
+C++ ObjC++ Var(warn_global_constructors) Warning
+Warn about objects with static storage duration that require dynamic initialization or have nontrivial destructors.
+
 Wif-not-aligned
 C ObjC C++ ObjC++ Var(warn_if_not_aligned) Init(1) Warning
 Warn when the field in a struct is not aligned.
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 19d14a6a5e9..8dc366aa0c6 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -8324,10 +8324,10 @@ expand_static_init (tree decl, tree init)
 	return;
     }
 
+  location_t dloc = DECL_SOURCE_LOCATION (decl);
   if (CP_DECL_THREAD_LOCAL_P (decl) && DECL_GNU_TLS_P (decl)
       && !DECL_FUNCTION_SCOPE_P (decl))
     {
-      location_t dloc = DECL_SOURCE_LOCATION (decl);
       if (init)
 	error_at (dloc, "non-local variable %qD declared %<__thread%> "
 		  "needs dynamic initialization", decl);
@@ -8467,10 +8467,24 @@ expand_static_init (tree decl, tree init)
       finish_then_clause (if_stmt);
       finish_if_stmt (if_stmt);
     }
-  else if (CP_DECL_THREAD_LOCAL_P (decl))
-    tls_aggregates = tree_cons (init, decl, tls_aggregates);
   else
-    static_aggregates = tree_cons (init, decl, static_aggregates);
+    {
+      if (CP_DECL_THREAD_LOCAL_P (decl))
+        tls_aggregates = tree_cons (init, decl, tls_aggregates);
+      else
+        static_aggregates = tree_cons (init, decl, static_aggregates);
+
+      if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (decl)))
+        {
+          warning_at (dloc, OPT_Wglobal_constructors,
+            "declaration requires a global destructor");
+          return;
+        }
+
+      if (DECL_NONTRIVIALLY_INITIALIZED_P (decl) && !decl_constant_var_p (decl))
+        warning_at (dloc, OPT_Wglobal_constructors,
+          "declaration requires a global constructor");
+    }
 }
 
 \f
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4964cc41ba3..77d324584ec 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -312,6 +312,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wformat-security  -Wformat-signedness  -Wformat-truncation=@var{n} @gol
 -Wformat-y2k  -Wframe-address @gol
 -Wframe-larger-than=@var{byte-size}  -Wno-free-nonheap-object @gol
+-Wglobal-constructors @gol
 -Wjump-misses-init @gol
 -Whsa  -Wif-not-aligned @gol
 -Wignored-qualifiers  -Wignored-attributes  -Wincompatible-pointer-types @gol
@@ -6509,6 +6510,40 @@ to @option{-Wframe-larger-than=}@samp{SIZE_MAX} or larger.
 Do not warn when attempting to free an object that was not allocated
 on the heap.
 
+@item -Wglobal-constructors @r{(C++ and Objective-C++ only)}
+@opindex Wglobal-constructors
+@opindex Wno-global-constructors
+Warn whenever an object with static storage duration either requires dynamic
+initialization or has a nontrivial destructor.  The compiler will issue a
+warning if a declaration's initializer is not a constant expression, as shown
+in the following examples:
+
+@smallexample
+@group
+const char* const tmp = getenv ("TMP");
+// warning: declaration requires a global constructor
+
+static int initialize() { return 42; }
+static int global = initialize ();
+// warning: declaration requires a global constructor
+
+@end group
+@end smallexample
+
+In cases where the compiler can treat the initializer as a constant expression,
+no warning is raised, as in the following example:
+
+@smallexample
+@group
+
+constexpr int initialize() { return 42; }
+static int i = initialize ();
+// no warning: initialize in this case is a constant expression
+
+@end group
+@end smallexample
+
+
 @item -Wstack-usage=@var{byte-size}
 @opindex Wstack-usage
 @opindex Wno-stack-usage
diff --git a/gcc/testsuite/g++.dg/warn/global-constructors-1.C b/gcc/testsuite/g++.dg/warn/global-constructors-1.C
new file mode 100644
index 00000000000..eb08379b680
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/global-constructors-1.C
@@ -0,0 +1,53 @@
+// { dg-do compile }
+// { dg-options "-Wglobal-constructors" }
+
+struct with_ctor {
+    int x;
+    with_ctor() : x(42) {}
+};
+
+struct with_dtor {
+    ~with_dtor() {}
+};
+
+struct with_both {
+    int x;
+    with_both() : x(42) {}
+    ~with_both() {}
+};
+
+struct with_default_ctor {
+    int x;
+    with_default_ctor() = default;
+};
+
+struct with_constexpr_ctor {
+    int x;
+    constexpr with_constexpr_ctor() : x(42) {}
+    constexpr with_constexpr_ctor(int x) : x(x) {}
+};
+
+with_ctor global_var; /* { dg-warning "declaration requires a global constructor" } */
+with_dtor global_var2; /* { dg-warning "declaration requires a global destructor" } */
+with_both global_var3; /* { dg-warning "declaration requires a global destructor" } */
+with_default_ctor global_var4; /* { dg-bogus "declaration requires a global constructor" } */
+with_constexpr_ctor global_var5; /* { dg-bogus "declaration requires a global constructor" } */
+
+int initialize() {
+    return 42;
+}
+
+int global_var6 = initialize(); /* { dg-warning "declaration requires a global constructor" } */
+with_constexpr_ctor global_var7(initialize()); /* { dg-warning "declaration requires a global constructor" } */
+
+constexpr int initialize_const() {
+    return 42;
+}
+
+int global_var8 = initialize_const(); /* { dg-bogus "declaration requires a global constructor" } */
+constexpr int global_var9 = initialize_const(); /* { dg-bogus "declaration requires a global constructor" } */
+
+
+int main() {
+    static with_ctor global_var; /* { dg-bogus "declaration requires a global constructor" } */
+}
diff --git a/gcc/testsuite/g++.dg/warn/global-constructors-2.C b/gcc/testsuite/g++.dg/warn/global-constructors-2.C
new file mode 100644
index 00000000000..e96da348458
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/global-constructors-2.C
@@ -0,0 +1,49 @@
+// { dg-do compile }
+// { dg-require-effective-target c++11 }
+// { dg-options "-Wglobal-constructors" }
+
+struct with_ctor {
+    int x;
+    with_ctor() : x(42) {}
+};
+
+struct with_dtor {
+    ~with_dtor() {}
+};
+
+struct with_both {
+    int x;
+    with_both() : x(42) {}
+    ~with_both() {}
+};
+
+struct with_default_ctor {
+    int x;
+    with_default_ctor() = default;
+};
+
+struct with_constexpr_ctor {
+    int x;
+    constexpr with_constexpr_ctor() : x(42) {}
+    constexpr with_constexpr_ctor(int x) : x(x) {}
+};
+
+thread_local with_ctor global_var; /* { dg-warning "declaration requires a global constructor" } */
+thread_local with_dtor global_var2; /* { dg-warning "declaration requires a global destructor" } */
+thread_local with_both global_var3; /* { dg-warning "declaration requires a global destructor" } */
+thread_local with_default_ctor global_var4; /* { dg-bogus "declaration requires a global constructor" } */
+thread_local with_constexpr_ctor global_var5; /* { dg-bogus "declaration requires a global constructor" } */
+
+int initialize() {
+    return 42;
+}
+
+thread_local int global_var6 = initialize(); /* { dg-warning "declaration requires a global constructor" } */
+thread_local with_constexpr_ctor global_var7(initialize()); /* { dg-warning "declaration requires a global constructor" } */
+
+constexpr int initialize_const() {
+    return 42;
+}
+
+thread_local int global_var8 = initialize_const(); /* { dg-bogus "declaration requires a global constructor" } */
+thread_local constexpr int global_var9 = initialize_const(); /* { dg-bogus "declaration requires a global constructor" } */
-- 
2.20.1


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

end of thread, other threads:[~2019-06-01 18:07 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-28 22:06 [PATCH] PR71482: Add -Wglobal-constructors Sean Gillespie
2019-05-30 19:44 ` Marek Polacek
2019-05-30 22:56   ` Sean Gillespie
2019-05-30 23:49 ` Martin Sebor
2019-05-31 15:56   ` Marek Polacek
2019-05-31 16:27     ` Marek Polacek
2019-05-31 16:29       ` Jakub Jelinek
2019-05-31 16:30         ` Marek Polacek
2019-05-31 16:25   ` Sean Gillespie
2019-05-31 16:54     ` Martin Sebor
2019-06-01  2:01       ` Sean Gillespie
2019-06-01 18:07       ` [PATCH v2] " Sean Gillespie

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).