diff mbox series

[1/2] ipa: Treat static constructors and destructors as non-local (PR 115815)

Message ID ri6a5i4bc6k.fsf@virgil.suse.cz
State New
Headers show
Series [1/2] ipa: Treat static constructors and destructors as non-local (PR 115815) | expand

Commit Message

Martin Jambor July 26, 2024, 11:54 a.m. UTC 
Hi,

in PR 115815, IPA-SRA thought it had control over all invocations of a
(recursive) static destructor but it did not see the implied
invocation which led to the original being left behind and the
clean-up code encountering uses of SSAs that definitely should have
been dead.

Fixed by teaching cgraph_node::can_be_local_p about static
constructors and destructors.  Similar test is missing in
cgraph_node::local_p so I added the check there as well.

Bootstrapped and tested on x86_64-linux.  OK for master and after a
while to gcc14 and gcc13 release branches?

Thanks,

Martin


gcc/ChangeLog:

2024-07-25  Martin Jambor  <mjambor@suse.cz>

	PR ipa/115815
	* cgraph.cc (cgraph_node_cannot_be_local_p_1): Also check
	DECL_STATIC_CONSTRUCTOR and DECL_STATIC_DESTRUCTOR.
	* ipa-visibility.cc (non_local_p): Likewise.
	(cgraph_node::local_p): Delete extraneous line of tabs.

gcc/testsuite/ChangeLog:

2024-07-25  Martin Jambor  <mjambor@suse.cz>

	PR ipa/115815
	* gcc.dg/lto/pr115815_0.c: New test.
---
 gcc/cgraph.cc                         |  4 +++-
 gcc/ipa-visibility.cc                 |  5 +++--
 gcc/testsuite/gcc.dg/lto/pr115815_0.c | 18 ++++++++++++++++++
 3 files changed, 24 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/lto/pr115815_0.c

Comments

Martin Jambor Aug. 9, 2024, 12:03 p.m. UTC | #1 
Hello,

and ping please.

Martin

On Fri, Jul 26 2024, Martin Jambor wrote:
> Hi,
>
> in PR 115815, IPA-SRA thought it had control over all invocations of a
> (recursive) static destructor but it did not see the implied
> invocation which led to the original being left behind and the
> clean-up code encountering uses of SSAs that definitely should have
> been dead.
>
> Fixed by teaching cgraph_node::can_be_local_p about static
> constructors and destructors.  Similar test is missing in
> cgraph_node::local_p so I added the check there as well.
>
> Bootstrapped and tested on x86_64-linux.  OK for master and after a
> while to gcc14 and gcc13 release branches?
>
> Thanks,
>
> Martin
>
>
> gcc/ChangeLog:
>
> 2024-07-25  Martin Jambor  <mjambor@suse.cz>
>
> 	PR ipa/115815
> 	* cgraph.cc (cgraph_node_cannot_be_local_p_1): Also check
> 	DECL_STATIC_CONSTRUCTOR and DECL_STATIC_DESTRUCTOR.
> 	* ipa-visibility.cc (non_local_p): Likewise.
> 	(cgraph_node::local_p): Delete extraneous line of tabs.
>
> gcc/testsuite/ChangeLog:
>
> 2024-07-25  Martin Jambor  <mjambor@suse.cz>
>
> 	PR ipa/115815
> 	* gcc.dg/lto/pr115815_0.c: New test.
> ---
>  gcc/cgraph.cc                         |  4 +++-
>  gcc/ipa-visibility.cc                 |  5 +++--
>  gcc/testsuite/gcc.dg/lto/pr115815_0.c | 18 ++++++++++++++++++
>  3 files changed, 24 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/lto/pr115815_0.c
>
> diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
> index 473d8410bc9..39a3adbc7c3 100644
> --- a/gcc/cgraph.cc
> +++ b/gcc/cgraph.cc
> @@ -2434,7 +2434,9 @@ cgraph_node_cannot_be_local_p_1 (cgraph_node *node, void *)
>  		&& !node->forced_by_abi
>  		&& !node->used_from_object_file_p ()
>  		&& !node->same_comdat_group)
> -	       || !node->externally_visible));
> +	       || !node->externally_visible)
> +	   && !DECL_STATIC_CONSTRUCTOR (node->decl)
> +	   && !DECL_STATIC_DESTRUCTOR (node->decl));
>  }
>  
>  /* Return true if cgraph_node can be made local for API change.
> diff --git a/gcc/ipa-visibility.cc b/gcc/ipa-visibility.cc
> index 501d3c304aa..21f0c47f388 100644
> --- a/gcc/ipa-visibility.cc
> +++ b/gcc/ipa-visibility.cc
> @@ -102,7 +102,9 @@ non_local_p (struct cgraph_node *node, void *data ATTRIBUTE_UNUSED)
>  	   && !node->externally_visible
>  	   && !node->used_from_other_partition
>  	   && !node->in_other_partition
> -	   && node->get_availability () >= AVAIL_AVAILABLE);
> +	   && node->get_availability () >= AVAIL_AVAILABLE
> +	   && !DECL_STATIC_CONSTRUCTOR (node->decl)
> +	   && !DECL_STATIC_DESTRUCTOR (node->decl));
>  }
>  
>  /* Return true when function can be marked local.  */
> @@ -116,7 +118,6 @@ cgraph_node::local_p (void)
>       return n->callees->callee->local_p ();
>     return !n->call_for_symbol_thunks_and_aliases (non_local_p,
>  						  NULL, true);
> -					
>  }
>  
>  /* A helper for comdat_can_be_unshared_p.  */
> diff --git a/gcc/testsuite/gcc.dg/lto/pr115815_0.c b/gcc/testsuite/gcc.dg/lto/pr115815_0.c
> new file mode 100644
> index 00000000000..d938ae4c802
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/lto/pr115815_0.c
> @@ -0,0 +1,18 @@
> +int a;
> +volatile int v;
> +volatile int w;
> +
> +int __attribute__((destructor))
> +b() {
> +  if (v)
> +    return a + b();
> +  v = 5;
> +  return 0;
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  w = 1;
> +  return 0;
> +}
> -- 
> 2.45.2
Martin Jambor Aug. 27, 2024, 7 a.m. UTC | #2 
Hello,

and ping please.

Martin


On Fri, Aug 09 2024, Martin Jambor wrote:
> Hello,
>
> and ping please.
>
> Martin
>
> On Fri, Jul 26 2024, Martin Jambor wrote:
>> Hi,
>>
>> in PR 115815, IPA-SRA thought it had control over all invocations of a
>> (recursive) static destructor but it did not see the implied
>> invocation which led to the original being left behind and the
>> clean-up code encountering uses of SSAs that definitely should have
>> been dead.
>>
>> Fixed by teaching cgraph_node::can_be_local_p about static
>> constructors and destructors.  Similar test is missing in
>> cgraph_node::local_p so I added the check there as well.
>>
>> Bootstrapped and tested on x86_64-linux.  OK for master and after a
>> while to gcc14 and gcc13 release branches?
>>
>> Thanks,
>>
>> Martin
>>
>>
>> gcc/ChangeLog:
>>
>> 2024-07-25  Martin Jambor  <mjambor@suse.cz>
>>
>> 	PR ipa/115815
>> 	* cgraph.cc (cgraph_node_cannot_be_local_p_1): Also check
>> 	DECL_STATIC_CONSTRUCTOR and DECL_STATIC_DESTRUCTOR.
>> 	* ipa-visibility.cc (non_local_p): Likewise.
>> 	(cgraph_node::local_p): Delete extraneous line of tabs.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2024-07-25  Martin Jambor  <mjambor@suse.cz>
>>
>> 	PR ipa/115815
>> 	* gcc.dg/lto/pr115815_0.c: New test.
>> ---
>>  gcc/cgraph.cc                         |  4 +++-
>>  gcc/ipa-visibility.cc                 |  5 +++--
>>  gcc/testsuite/gcc.dg/lto/pr115815_0.c | 18 ++++++++++++++++++
>>  3 files changed, 24 insertions(+), 3 deletions(-)
>>  create mode 100644 gcc/testsuite/gcc.dg/lto/pr115815_0.c
>>
>> diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
>> index 473d8410bc9..39a3adbc7c3 100644
>> --- a/gcc/cgraph.cc
>> +++ b/gcc/cgraph.cc
>> @@ -2434,7 +2434,9 @@ cgraph_node_cannot_be_local_p_1 (cgraph_node *node, void *)
>>  		&& !node->forced_by_abi
>>  		&& !node->used_from_object_file_p ()
>>  		&& !node->same_comdat_group)
>> -	       || !node->externally_visible));
>> +	       || !node->externally_visible)
>> +	   && !DECL_STATIC_CONSTRUCTOR (node->decl)
>> +	   && !DECL_STATIC_DESTRUCTOR (node->decl));
>>  }
>>  
>>  /* Return true if cgraph_node can be made local for API change.
>> diff --git a/gcc/ipa-visibility.cc b/gcc/ipa-visibility.cc
>> index 501d3c304aa..21f0c47f388 100644
>> --- a/gcc/ipa-visibility.cc
>> +++ b/gcc/ipa-visibility.cc
>> @@ -102,7 +102,9 @@ non_local_p (struct cgraph_node *node, void *data ATTRIBUTE_UNUSED)
>>  	   && !node->externally_visible
>>  	   && !node->used_from_other_partition
>>  	   && !node->in_other_partition
>> -	   && node->get_availability () >= AVAIL_AVAILABLE);
>> +	   && node->get_availability () >= AVAIL_AVAILABLE
>> +	   && !DECL_STATIC_CONSTRUCTOR (node->decl)
>> +	   && !DECL_STATIC_DESTRUCTOR (node->decl));
>>  }
>>  
>>  /* Return true when function can be marked local.  */
>> @@ -116,7 +118,6 @@ cgraph_node::local_p (void)
>>       return n->callees->callee->local_p ();
>>     return !n->call_for_symbol_thunks_and_aliases (non_local_p,
>>  						  NULL, true);
>> -					
>>  }
>>  
>>  /* A helper for comdat_can_be_unshared_p.  */
>> diff --git a/gcc/testsuite/gcc.dg/lto/pr115815_0.c b/gcc/testsuite/gcc.dg/lto/pr115815_0.c
>> new file mode 100644
>> index 00000000000..d938ae4c802
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/lto/pr115815_0.c
>> @@ -0,0 +1,18 @@
>> +int a;
>> +volatile int v;
>> +volatile int w;
>> +
>> +int __attribute__((destructor))
>> +b() {
>> +  if (v)
>> +    return a + b();
>> +  v = 5;
>> +  return 0;
>> +}
>> +
>> +int
>> +main (int argc, char **argv)
>> +{
>> +  w = 1;
>> +  return 0;
>> +}
>> -- 
>> 2.45.2
diff mbox series

Patch

diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
index 473d8410bc9..39a3adbc7c3 100644
--- a/gcc/cgraph.cc
+++ b/gcc/cgraph.cc
@@ -2434,7 +2434,9 @@  cgraph_node_cannot_be_local_p_1 (cgraph_node *node, void *)
 		&& !node->forced_by_abi
 		&& !node->used_from_object_file_p ()
 		&& !node->same_comdat_group)
-	       || !node->externally_visible));
+	       || !node->externally_visible)
+	   && !DECL_STATIC_CONSTRUCTOR (node->decl)
+	   && !DECL_STATIC_DESTRUCTOR (node->decl));
 }
 
 /* Return true if cgraph_node can be made local for API change.
diff --git a/gcc/ipa-visibility.cc b/gcc/ipa-visibility.cc
index 501d3c304aa..21f0c47f388 100644
--- a/gcc/ipa-visibility.cc
+++ b/gcc/ipa-visibility.cc
@@ -102,7 +102,9 @@  non_local_p (struct cgraph_node *node, void *data ATTRIBUTE_UNUSED)
 	   && !node->externally_visible
 	   && !node->used_from_other_partition
 	   && !node->in_other_partition
-	   && node->get_availability () >= AVAIL_AVAILABLE);
+	   && node->get_availability () >= AVAIL_AVAILABLE
+	   && !DECL_STATIC_CONSTRUCTOR (node->decl)
+	   && !DECL_STATIC_DESTRUCTOR (node->decl));
 }
 
 /* Return true when function can be marked local.  */
@@ -116,7 +118,6 @@  cgraph_node::local_p (void)
      return n->callees->callee->local_p ();
    return !n->call_for_symbol_thunks_and_aliases (non_local_p,
 						  NULL, true);
-					
 }
 
 /* A helper for comdat_can_be_unshared_p.  */
diff --git a/gcc/testsuite/gcc.dg/lto/pr115815_0.c b/gcc/testsuite/gcc.dg/lto/pr115815_0.c
new file mode 100644
index 00000000000..d938ae4c802
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr115815_0.c
@@ -0,0 +1,18 @@ 
+int a;
+volatile int v;
+volatile int w;
+
+int __attribute__((destructor))
+b() {
+  if (v)
+    return a + b();
+  v = 5;
+  return 0;
+}
+
+int
+main (int argc, char **argv)
+{
+  w = 1;
+  return 0;
+}