Drop MALLOC attribute for void functions.

Message ID e2627959-892d-9abe-636d-880b4180d47d@suse.cz
State New
Headers show
Series
  • Drop MALLOC attribute for void functions.
Related show

Commit Message

Martin Liška Feb. 17, 2020, 12:16 p.m.
Hello.

As mentioned in the PR, we end up with a void function
call that has set MALLOC attribute. That causes problems in RTL
expansion.

I believe a proper fix is to drop the attribute when a callgraph
clone with void type is created.

I would like to see Martin's comment about the hunk in propagate_malloc
which is maybe suboptimal and one can find a better way?

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
It fixes LTO bootstrap on ppc64le.

Thanks,
Martin

gcc/ChangeLog:

2020-02-17  Martin Liska  <mliska@suse.cz>

	PR ipa/93583
	* cgraph.c (cgraph_node::verify_node): Verify MALLOC attribute
	and return type of functions.
	* ipa-param-manipulation.c (ipa_param_adjustments::adjust_decl):
	Drop MALLOC attribute for void functions.
	* ipa-pure-const.c (propagate_malloc): Do not set malloc flag
	for void functions.

gcc/testsuite/ChangeLog:

2020-02-17  Martin Liska  <mliska@suse.cz>

	PR ipa/93583
	* gcc.dg/ipa/pr93583.c: New test.
---
  gcc/cgraph.c                       |  6 ++++++
  gcc/ipa-param-manipulation.c       |  4 ++++
  gcc/ipa-pure-const.c               |  9 ++++++---
  gcc/testsuite/gcc.dg/ipa/pr93583.c | 15 +++++++++++++++
  4 files changed, 31 insertions(+), 3 deletions(-)
  create mode 100644 gcc/testsuite/gcc.dg/ipa/pr93583.c

Comments

Martin Jambor Feb. 17, 2020, 12:53 p.m. | #1
Hi,

On Mon, Feb 17 2020, Martin Liška wrote:
> Hello.

>

> As mentioned in the PR, we end up with a void function

> call that has set MALLOC attribute. That causes problems in RTL

> expansion.

>

> I believe a proper fix is to drop the attribute when a callgraph

> clone with void type is created.

>

> I would like to see Martin's comment about the hunk in propagate_malloc

> which is maybe suboptimal and one can find a better way?

>

> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

> It fixes LTO bootstrap on ppc64le.

>


It looks sensible to me.  I did not quite took into consideration that
ipa-pure-const can add the attribute and only dropped in tree-inline's
tree_function_versioning.  Just the hunk in
ipa_param_adjustments::adjust_decl could be simplified a tiny bit...

> Thanks,

> Martin

>

> gcc/ChangeLog:

>

> 2020-02-17  Martin Liska  <mliska@suse.cz>

>

> 	PR ipa/93583

> 	* cgraph.c (cgraph_node::verify_node): Verify MALLOC attribute

> 	and return type of functions.

> 	* ipa-param-manipulation.c (ipa_param_adjustments::adjust_decl):

> 	Drop MALLOC attribute for void functions.

> 	* ipa-pure-const.c (propagate_malloc): Do not set malloc flag

> 	for void functions.

>

> gcc/testsuite/ChangeLog:

>

> 2020-02-17  Martin Liska  <mliska@suse.cz>

>

> 	PR ipa/93583

> 	* gcc.dg/ipa/pr93583.c: New test.

> ---

>   gcc/cgraph.c                       |  6 ++++++

>   gcc/ipa-param-manipulation.c       |  4 ++++

>   gcc/ipa-pure-const.c               |  9 ++++++---

>   gcc/testsuite/gcc.dg/ipa/pr93583.c | 15 +++++++++++++++

>   4 files changed, 31 insertions(+), 3 deletions(-)

>   create mode 100644 gcc/testsuite/gcc.dg/ipa/pr93583.c

>


[...]

> diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c

> index 19ec87358fa..5f6f0575b06 100644

> --- a/gcc/ipa-param-manipulation.c

> +++ b/gcc/ipa-param-manipulation.c

> @@ -410,6 +410,10 @@ ipa_param_adjustments::adjust_decl (tree orig_decl)

>    DECL_VIRTUAL_P (new_decl) = 0;

>    DECL_LANG_SPECIFIC (new_decl) = NULL;

>  

> +  /* Drop MALLOC attribute for a void function.  */

> +  if (VOID_TYPE_P (TREE_TYPE (TREE_TYPE (new_decl))))

> +    DECL_IS_MALLOC (new_decl) = 0;


by just testing m_skip_return here (but it is no big deal).

Thanks!

Martin
Richard Biener Feb. 17, 2020, 2:25 p.m. | #2
On Mon, Feb 17, 2020 at 1:16 PM Martin Liška <mliska@suse.cz> wrote:
>

> Hello.

>

> As mentioned in the PR, we end up with a void function

> call that has set MALLOC attribute. That causes problems in RTL

> expansion.

>

> I believe a proper fix is to drop the attribute when a callgraph

> clone with void type is created.

>

> I would like to see Martin's comment about the hunk in propagate_malloc

> which is maybe suboptimal and one can find a better way?

>

> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

> It fixes LTO bootstrap on ppc64le.


Is the ipa-pure-const.c hunk really necessary? malloc_candidate_p shouldn't
see any such function as candidate.  If so it should be fixed there?

Thanks,
Richard.

> Thanks,

> Martin

>

> gcc/ChangeLog:

>

> 2020-02-17  Martin Liska  <mliska@suse.cz>

>

>         PR ipa/93583

>         * cgraph.c (cgraph_node::verify_node): Verify MALLOC attribute

>         and return type of functions.

>         * ipa-param-manipulation.c (ipa_param_adjustments::adjust_decl):

>         Drop MALLOC attribute for void functions.

>         * ipa-pure-const.c (propagate_malloc): Do not set malloc flag

>         for void functions.

>

> gcc/testsuite/ChangeLog:

>

> 2020-02-17  Martin Liska  <mliska@suse.cz>

>

>         PR ipa/93583

>         * gcc.dg/ipa/pr93583.c: New test.

> ---

>   gcc/cgraph.c                       |  6 ++++++

>   gcc/ipa-param-manipulation.c       |  4 ++++

>   gcc/ipa-pure-const.c               |  9 ++++++---

>   gcc/testsuite/gcc.dg/ipa/pr93583.c | 15 +++++++++++++++

>   4 files changed, 31 insertions(+), 3 deletions(-)

>   create mode 100644 gcc/testsuite/gcc.dg/ipa/pr93583.c

>

>
Martin Liška Feb. 17, 2020, 2:44 p.m. | #3
On 2/17/20 3:25 PM, Richard Biener wrote:
> On Mon, Feb 17, 2020 at 1:16 PM Martin Liška <mliska@suse.cz> wrote:

>>

>> Hello.

>>

>> As mentioned in the PR, we end up with a void function

>> call that has set MALLOC attribute. That causes problems in RTL

>> expansion.

>>

>> I believe a proper fix is to drop the attribute when a callgraph

>> clone with void type is created.

>>

>> I would like to see Martin's comment about the hunk in propagate_malloc

>> which is maybe suboptimal and one can find a better way?

>>

>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

>> It fixes LTO bootstrap on ppc64le.

> 

> Is the ipa-pure-const.c hunk really necessary?


Yes.

> malloc_candidate_p shouldn't

> see any such function as candidate.  If so it should be fixed there?


I've got hopefully a proper fix. Which is where we clone funct_state_summary_t
summary for the newly cloned IPA SRA function.

I'm going to test the patch.
Martin

> 

> Thanks,

> Richard.

> 

>> Thanks,

>> Martin

>>

>> gcc/ChangeLog:

>>

>> 2020-02-17  Martin Liska  <mliska@suse.cz>

>>

>>          PR ipa/93583

>>          * cgraph.c (cgraph_node::verify_node): Verify MALLOC attribute

>>          and return type of functions.

>>          * ipa-param-manipulation.c (ipa_param_adjustments::adjust_decl):

>>          Drop MALLOC attribute for void functions.

>>          * ipa-pure-const.c (propagate_malloc): Do not set malloc flag

>>          for void functions.

>>

>> gcc/testsuite/ChangeLog:

>>

>> 2020-02-17  Martin Liska  <mliska@suse.cz>

>>

>>          PR ipa/93583

>>          * gcc.dg/ipa/pr93583.c: New test.

>> ---

>>    gcc/cgraph.c                       |  6 ++++++

>>    gcc/ipa-param-manipulation.c       |  4 ++++

>>    gcc/ipa-pure-const.c               |  9 ++++++---

>>    gcc/testsuite/gcc.dg/ipa/pr93583.c | 15 +++++++++++++++

>>    4 files changed, 31 insertions(+), 3 deletions(-)

>>    create mode 100644 gcc/testsuite/gcc.dg/ipa/pr93583.c

>>

>>
From c49949575f661d67afc7cc77d8e6f5f8bea922a2 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>

Date: Mon, 17 Feb 2020 12:33:28 +0100
Subject: [PATCH] Drop MALLOC attribute for void functions.

gcc/ChangeLog:

2020-02-17  Martin Liska  <mliska@suse.cz>

	PR ipa/93583
	* cgraph.c (cgraph_node::verify_node): Verify MALLOC attribute
	and return type of functions.
	* ipa-param-manipulation.c (ipa_param_adjustments::adjust_decl):
	Drop MALLOC attribute for void functions.
	* ipa-pure-const.c (funct_state_summary_t::duplicate): Drop
	malloc_state for a new VOID clone.

gcc/testsuite/ChangeLog:

2020-02-17  Martin Liska  <mliska@suse.cz>

	PR ipa/93583
	* gcc.dg/ipa/pr93583.c: New test.
---
 gcc/cgraph.c                       |  6 ++++++
 gcc/ipa-param-manipulation.c       |  4 ++++
 gcc/ipa-pure-const.c               | 14 ++++++++++----
 gcc/testsuite/gcc.dg/ipa/pr93583.c | 15 +++++++++++++++
 4 files changed, 35 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/pr93583.c

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 294b2d392fd..6e99fbb45f4 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -3374,6 +3374,12 @@ cgraph_node::verify_node (void)
       error ("calls_comdat_local is set outside of a comdat group");
       error_found = true;
     }
+  if (DECL_IS_MALLOC (decl)
+      && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (decl))))
+    {
+      error ("MALLOC attribute set on a void function");
+      error_found = true;
+    }
   for (e = indirect_calls; e; e = e->next_callee)
     {
       if (e->aux)
diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
index 19ec87358fa..839bd2ef870 100644
--- a/gcc/ipa-param-manipulation.c
+++ b/gcc/ipa-param-manipulation.c
@@ -410,6 +410,10 @@ ipa_param_adjustments::adjust_decl (tree orig_decl)
   DECL_VIRTUAL_P (new_decl) = 0;
   DECL_LANG_SPECIFIC (new_decl) = NULL;
 
+  /* Drop MALLOC attribute for a void function.  */
+  if (m_skip_return)
+    DECL_IS_MALLOC (new_decl) = 0;
+
   return new_decl;
 }
 
diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c
index ccd0918c120..a2d0792b0ce 100644
--- a/gcc/ipa-pure-const.c
+++ b/gcc/ipa-pure-const.c
@@ -1157,11 +1157,14 @@ funct_state_summary_t::insert (cgraph_node *node, funct_state_d *state)
 /* Called when new clone is inserted to callgraph late.  */
 
 void
-funct_state_summary_t::duplicate (cgraph_node *, cgraph_node *,
+funct_state_summary_t::duplicate (cgraph_node *src, cgraph_node *dst,
 				  funct_state_d *src_data,
 				  funct_state_d *dst_data)
 {
   new (dst_data) funct_state_d (*src_data);
+  if (dst_data->malloc_state == STATE_MALLOC
+      && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (dst->decl))))
+    dst_data->malloc_state = STATE_MALLOC_BOTTOM;
 }
 
 
@@ -1973,9 +1976,12 @@ propagate_malloc (void)
 		       node->dump_name ());
 
 	    bool malloc_decl_p = DECL_IS_MALLOC (node->decl);
-	    node->set_malloc_flag (true);
-	    if (!malloc_decl_p && warn_suggest_attribute_malloc)
-		warn_function_malloc (node->decl);
+	    if (!VOID_TYPE_P (TREE_TYPE (TREE_TYPE (node->decl))))
+	      {
+		node->set_malloc_flag (true);
+		if (!malloc_decl_p && warn_suggest_attribute_malloc)
+		  warn_function_malloc (node->decl);
+	      }
 	  }
       }
 
diff --git a/gcc/testsuite/gcc.dg/ipa/pr93583.c b/gcc/testsuite/gcc.dg/ipa/pr93583.c
new file mode 100644
index 00000000000..30ef506553d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr93583.c
@@ -0,0 +1,15 @@
+/* PR ipa/93583 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+void *bar(const char*);
+static void *__attribute__((malloc,noinline)) foo(const char *p)
+{
+  return bar (p);
+}
+
+int main(int argc, char **argv)
+{
+  foo (argv[0]);
+  return 0;
+}
-- 
2.25.0
Martin Liška Feb. 17, 2020, 2:47 p.m. | #4
Sorry, I attached wrong patch.

Martin
From c215a4d5d5d5aa72729c16408805eb9ddf5f0d38 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>

Date: Mon, 17 Feb 2020 12:33:28 +0100
Subject: [PATCH] Drop MALLOC attribute for void functions.

gcc/ChangeLog:

2020-02-17  Martin Liska  <mliska@suse.cz>

	PR ipa/93583
	* cgraph.c (cgraph_node::verify_node): Verify MALLOC attribute
	and return type of functions.
	* ipa-param-manipulation.c (ipa_param_adjustments::adjust_decl):
	Drop MALLOC attribute for void functions.
	* ipa-pure-const.c (funct_state_summary_t::duplicate): Drop
	malloc_state for a new VOID clone.

gcc/testsuite/ChangeLog:

2020-02-17  Martin Liska  <mliska@suse.cz>

	PR ipa/93583
	* gcc.dg/ipa/pr93583.c: New test.
---
 gcc/cgraph.c                       |  6 ++++++
 gcc/ipa-param-manipulation.c       |  4 ++++
 gcc/ipa-pure-const.c               |  5 ++++-
 gcc/testsuite/gcc.dg/ipa/pr93583.c | 15 +++++++++++++++
 4 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/pr93583.c

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 294b2d392fd..6e99fbb45f4 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -3374,6 +3374,12 @@ cgraph_node::verify_node (void)
       error ("calls_comdat_local is set outside of a comdat group");
       error_found = true;
     }
+  if (DECL_IS_MALLOC (decl)
+      && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (decl))))
+    {
+      error ("MALLOC attribute set on a void function");
+      error_found = true;
+    }
   for (e = indirect_calls; e; e = e->next_callee)
     {
       if (e->aux)
diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
index 19ec87358fa..839bd2ef870 100644
--- a/gcc/ipa-param-manipulation.c
+++ b/gcc/ipa-param-manipulation.c
@@ -410,6 +410,10 @@ ipa_param_adjustments::adjust_decl (tree orig_decl)
   DECL_VIRTUAL_P (new_decl) = 0;
   DECL_LANG_SPECIFIC (new_decl) = NULL;
 
+  /* Drop MALLOC attribute for a void function.  */
+  if (m_skip_return)
+    DECL_IS_MALLOC (new_decl) = 0;
+
   return new_decl;
 }
 
diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c
index ccd0918c120..564c6629c92 100644
--- a/gcc/ipa-pure-const.c
+++ b/gcc/ipa-pure-const.c
@@ -1157,11 +1157,14 @@ funct_state_summary_t::insert (cgraph_node *node, funct_state_d *state)
 /* Called when new clone is inserted to callgraph late.  */
 
 void
-funct_state_summary_t::duplicate (cgraph_node *, cgraph_node *,
+funct_state_summary_t::duplicate (cgraph_node *, cgraph_node *dst,
 				  funct_state_d *src_data,
 				  funct_state_d *dst_data)
 {
   new (dst_data) funct_state_d (*src_data);
+  if (dst_data->malloc_state == STATE_MALLOC
+      && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (dst->decl))))
+    dst_data->malloc_state = STATE_MALLOC_BOTTOM;
 }
 
 
diff --git a/gcc/testsuite/gcc.dg/ipa/pr93583.c b/gcc/testsuite/gcc.dg/ipa/pr93583.c
new file mode 100644
index 00000000000..30ef506553d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr93583.c
@@ -0,0 +1,15 @@
+/* PR ipa/93583 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+void *bar(const char*);
+static void *__attribute__((malloc,noinline)) foo(const char *p)
+{
+  return bar (p);
+}
+
+int main(int argc, char **argv)
+{
+  foo (argv[0]);
+  return 0;
+}
-- 
2.25.0
Martin Jambor Feb. 17, 2020, 2:48 p.m. | #5
Hi,

On Mon, Feb 17 2020, Richard Biener wrote:
> On Mon, Feb 17, 2020 at 1:16 PM Martin Liška <mliska@suse.cz> wrote:

>>

>> Hello.

>>

>> As mentioned in the PR, we end up with a void function

>> call that has set MALLOC attribute. That causes problems in RTL

>> expansion.

>>

>> I believe a proper fix is to drop the attribute when a callgraph

>> clone with void type is created.

>>

>> I would like to see Martin's comment about the hunk in propagate_malloc

>> which is maybe suboptimal and one can find a better way?

>>

>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

>> It fixes LTO bootstrap on ppc64le.

>

> Is the ipa-pure-const.c hunk really necessary? malloc_candidate_p shouldn't

> see any such function as candidate.  If so it should be fixed there?


malloc_candidate_p runs at summary building phase (LTO compile time) and
IPA-SRA WPA phase, which actually removes the return value, runs between
that and ipa-pure-const WPA phase. So without the hunk, the pass could
now happily re-introduce the flag.

I have not analyzed the original bug report but I actually believe that
the ipa-pure-const hunk is actually the one fixing the core issue there.
Otherwise, removal of the attribute in tree-inline.c would have been
enough.

If we don't like adding another look at the trees in a WPA phase of an
IPA pass, ipa-pure const could alternatively test:

  if (node->param_adjustments && node->param_adjustments->m_skip_return)
    continue;

Martin


>

> Thanks,

> Richard.

>

>> Thanks,

>> Martin

>>

>> gcc/ChangeLog:

>>

>> 2020-02-17  Martin Liska  <mliska@suse.cz>

>>

>>         PR ipa/93583

>>         * cgraph.c (cgraph_node::verify_node): Verify MALLOC attribute

>>         and return type of functions.

>>         * ipa-param-manipulation.c (ipa_param_adjustments::adjust_decl):

>>         Drop MALLOC attribute for void functions.

>>         * ipa-pure-const.c (propagate_malloc): Do not set malloc flag

>>         for void functions.

>>

>> gcc/testsuite/ChangeLog:

>>

>> 2020-02-17  Martin Liska  <mliska@suse.cz>

>>

>>         PR ipa/93583

>>         * gcc.dg/ipa/pr93583.c: New test.

>> ---

>>   gcc/cgraph.c                       |  6 ++++++

>>   gcc/ipa-param-manipulation.c       |  4 ++++

>>   gcc/ipa-pure-const.c               |  9 ++++++---

>>   gcc/testsuite/gcc.dg/ipa/pr93583.c | 15 +++++++++++++++

>>   4 files changed, 31 insertions(+), 3 deletions(-)

>>   create mode 100644 gcc/testsuite/gcc.dg/ipa/pr93583.c

>>

>>
Jakub Jelinek Feb. 17, 2020, 2:48 p.m. | #6
On Mon, Feb 17, 2020 at 03:44:53PM +0100, Martin Liška wrote:
> +      error ("MALLOC attribute set on a void function");


Why the capitals?  Either malloc or %<malloc%> IMSHO.
What is special about void functions, missing lhs?  That can be missing
for other functions (where you just don't use the return value, or e.g.
noreturn even if you do)?  And otherwise, shouldn't the test be rather
whether the return type is a pointer type?  E.g. float or int return
type for malloc attribute isn't very meaningful.

	Jakub
Martin Liška Feb. 17, 2020, 3:18 p.m. | #7
On 2/17/20 3:48 PM, Jakub Jelinek wrote:
> On Mon, Feb 17, 2020 at 03:44:53PM +0100, Martin Liška wrote:

>> +      error ("MALLOC attribute set on a void function");

> 

> Why the capitals?  Either malloc or %<malloc%> IMSHO.


Sure, I'll fix it.

> What is special about void functions, missing lhs?  That can be missing

> for other functions (where you just don't use the return value, or e.g.

> noreturn even if you do)?  And otherwise, shouldn't the test be rather

> whether the return type is a pointer type?  E.g. float or int return

> type for malloc attribute isn't very meaningful.


Hopefully one can't set malloc attribute to a symbol that does not
return a pointer type:

head malloc.c
float bar(const char*);
static float __attribute__((malloc,noinline)) foo(const char *p)
{
   return bar (p);
}

$ gcc malloc.c -c -O2
malloc.c:3:1: warning: ‘malloc’ attribute ignored [-Wattributes]
     3 | {

Martin

> 

> 	Jakub

>
Richard Biener Feb. 18, 2020, 10:09 a.m. | #8
On Mon, Feb 17, 2020 at 3:47 PM Martin Liška <mliska@suse.cz> wrote:
>

> Sorry, I attached wrong patch.


Ah yeah, that looks like a better place (the IPA pure-const one).  OK
with Jakubs
suggested changes.

Richard.

> Martin
Richard Biener Feb. 18, 2020, 10:10 a.m. | #9
On Mon, Feb 17, 2020 at 3:48 PM Jakub Jelinek <jakub@redhat.com> wrote:
>

> On Mon, Feb 17, 2020 at 03:44:53PM +0100, Martin Liška wrote:

> > +      error ("MALLOC attribute set on a void function");

>

> Why the capitals?  Either malloc or %<malloc%> IMSHO.

> What is special about void functions, missing lhs?  That can be missing

> for other functions (where you just don't use the return value, or e.g.

> noreturn even if you do)?  And otherwise, shouldn't the test be rather

> whether the return type is a pointer type?  E.g. float or int return

> type for malloc attribute isn't very meaningful.


It surely would be better if the expansion code would deal with a
"bogus" ECF_MALLOC (ERF_RETURNS_ARG is probably another candidate
that can get "wrong" ...).  But it doesn't seem as easy as fixing the segfault
(see the PR, somehow we fail to emit some insns).

Richard.

>         Jakub

>
Martin Liška Feb. 18, 2020, 1:09 p.m. | #10
On 2/18/20 11:09 AM, Richard Biener wrote:
> On Mon, Feb 17, 2020 at 3:47 PM Martin Liška <mliska@suse.cz> wrote:

>>

>> Sorry, I attached wrong patch.

> 

> Ah yeah, that looks like a better place (the IPA pure-const one).  OK

> with Jakubs

> suggested changes.

> 

> Richard.

> 

>> Martin


All right. There's hopefully a final version of the patch that includes
Jakub's notes.

I've just tested that ppc64le-linux-gnu LTO bootstrap. I'll install it
if there are no objections.

Martin
From 6c29b60fa67b5cbafec4a1089a4edee03ac97ea1 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>

Date: Mon, 17 Feb 2020 12:33:28 +0100
Subject: [PATCH] Drop MALLOC attribute for void functions.

gcc/ChangeLog:

2020-02-17  Martin Liska  <mliska@suse.cz>

	PR ipa/93583
	* cgraph.c (cgraph_node::verify_node): Verify MALLOC attribute
	and return type of functions.
	* ipa-param-manipulation.c (ipa_param_adjustments::adjust_decl):
	Drop MALLOC attribute for void functions.
	* ipa-pure-const.c (funct_state_summary_t::duplicate): Drop
	malloc_state for a new VOID clone.

gcc/testsuite/ChangeLog:

2020-02-17  Martin Liska  <mliska@suse.cz>

	PR ipa/93583
	* gcc.dg/ipa/pr93583.c: New test.
---
 gcc/cgraph.c                       |  7 +++++++
 gcc/ipa-param-manipulation.c       |  4 ++++
 gcc/ipa-pure-const.c               |  5 ++++-
 gcc/testsuite/gcc.dg/ipa/pr93583.c | 15 +++++++++++++++
 4 files changed, 30 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/pr93583.c

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 294b2d392fd..c3423791c12 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -3374,6 +3374,13 @@ cgraph_node::verify_node (void)
       error ("calls_comdat_local is set outside of a comdat group");
       error_found = true;
     }
+  if (DECL_IS_MALLOC (decl)
+      && !POINTER_TYPE_P (TREE_TYPE (TREE_TYPE (decl))))
+    {
+      error ("malloc attribute should be used for a function that "
+	     "returns a pointer");
+      error_found = true;
+    }
   for (e = indirect_calls; e; e = e->next_callee)
     {
       if (e->aux)
diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
index 19ec87358fa..839bd2ef870 100644
--- a/gcc/ipa-param-manipulation.c
+++ b/gcc/ipa-param-manipulation.c
@@ -410,6 +410,10 @@ ipa_param_adjustments::adjust_decl (tree orig_decl)
   DECL_VIRTUAL_P (new_decl) = 0;
   DECL_LANG_SPECIFIC (new_decl) = NULL;
 
+  /* Drop MALLOC attribute for a void function.  */
+  if (m_skip_return)
+    DECL_IS_MALLOC (new_decl) = 0;
+
   return new_decl;
 }
 
diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c
index ccd0918c120..564c6629c92 100644
--- a/gcc/ipa-pure-const.c
+++ b/gcc/ipa-pure-const.c
@@ -1157,11 +1157,14 @@ funct_state_summary_t::insert (cgraph_node *node, funct_state_d *state)
 /* Called when new clone is inserted to callgraph late.  */
 
 void
-funct_state_summary_t::duplicate (cgraph_node *, cgraph_node *,
+funct_state_summary_t::duplicate (cgraph_node *, cgraph_node *dst,
 				  funct_state_d *src_data,
 				  funct_state_d *dst_data)
 {
   new (dst_data) funct_state_d (*src_data);
+  if (dst_data->malloc_state == STATE_MALLOC
+      && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (dst->decl))))
+    dst_data->malloc_state = STATE_MALLOC_BOTTOM;
 }
 
 
diff --git a/gcc/testsuite/gcc.dg/ipa/pr93583.c b/gcc/testsuite/gcc.dg/ipa/pr93583.c
new file mode 100644
index 00000000000..30ef506553d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr93583.c
@@ -0,0 +1,15 @@
+/* PR ipa/93583 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+void *bar(const char*);
+static void *__attribute__((malloc,noinline)) foo(const char *p)
+{
+  return bar (p);
+}
+
+int main(int argc, char **argv)
+{
+  foo (argv[0]);
+  return 0;
+}
-- 
2.25.0

Patch

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 294b2d392fd..6e99fbb45f4 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -3374,6 +3374,12 @@  cgraph_node::verify_node (void)
       error ("calls_comdat_local is set outside of a comdat group");
       error_found = true;
     }
+  if (DECL_IS_MALLOC (decl)
+      && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (decl))))
+    {
+      error ("MALLOC attribute set on a void function");
+      error_found = true;
+    }
   for (e = indirect_calls; e; e = e->next_callee)
     {
       if (e->aux)
diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
index 19ec87358fa..5f6f0575b06 100644
--- a/gcc/ipa-param-manipulation.c
+++ b/gcc/ipa-param-manipulation.c
@@ -410,6 +410,10 @@  ipa_param_adjustments::adjust_decl (tree orig_decl)
   DECL_VIRTUAL_P (new_decl) = 0;
   DECL_LANG_SPECIFIC (new_decl) = NULL;
 
+  /* Drop MALLOC attribute for a void function.  */
+  if (VOID_TYPE_P (TREE_TYPE (TREE_TYPE (new_decl))))
+    DECL_IS_MALLOC (new_decl) = 0;
+
   return new_decl;
 }
 
diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c
index ccd0918c120..315134d2a94 100644
--- a/gcc/ipa-pure-const.c
+++ b/gcc/ipa-pure-const.c
@@ -1973,9 +1973,12 @@  propagate_malloc (void)
 		       node->dump_name ());
 
 	    bool malloc_decl_p = DECL_IS_MALLOC (node->decl);
-	    node->set_malloc_flag (true);
-	    if (!malloc_decl_p && warn_suggest_attribute_malloc)
-		warn_function_malloc (node->decl);
+	    if (!VOID_TYPE_P (TREE_TYPE (TREE_TYPE (node->decl))))
+	      {
+		node->set_malloc_flag (true);
+		if (!malloc_decl_p && warn_suggest_attribute_malloc)
+		  warn_function_malloc (node->decl);
+	      }
 	  }
       }
 
diff --git a/gcc/testsuite/gcc.dg/ipa/pr93583.c b/gcc/testsuite/gcc.dg/ipa/pr93583.c
new file mode 100644
index 00000000000..30ef506553d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr93583.c
@@ -0,0 +1,15 @@ 
+/* PR ipa/93583 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+void *bar(const char*);
+static void *__attribute__((malloc,noinline)) foo(const char *p)
+{
+  return bar (p);
+}
+
+int main(int argc, char **argv)
+{
+  foo (argv[0]);
+  return 0;
+}