From: Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>
To: gcc Patches <gcc-patches@gcc.gnu.org>,
Richard Biener <rguenther@suse.de>
Subject: PR80613
Date: Thu, 04 May 2017 16:03:00 -0000 [thread overview]
Message-ID: <CAAgBjM=DUQ=mxxXscCzNXY1mJ=OsxP3yUAJBF6AZxrL6Zk6QwA@mail.gmail.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 953 bytes --]
Hi,
As mentioned in PR, the issue is that cddce1 marks the call to
__builtin_strdup as necessary:
marking necessary through .MEM_6 stmt p_7 = __builtin_strdup (&d);
and since p_7 doesn't get added to worklist in propagate_necessity()
because it's used only within free(), it's treated as "dead"
and wrongly gets released.
The patch fixes that by adding strdup/strndup in corresponding condition
in eliminate_unnecessary_stmts().
Another issue, was that my previous patch failed to remove multiple
calls to strdup:
char *f(char **tt)
{
char *t = *tt;
char *p;
p = __builtin_strdup (t);
p = __builtin_strdup (t);
return p;
}
That's fixed in patch by adding strdup/strndup to another
corresponding condition in propagate_necessity() so that only one
instance of strdup would be kept.
Bootstrapped+tested on x86_64-unknown-linux-gnu.
Cross-testing on arm*-*-* and aarch64*-*-* in progress.
OK to commit if testing passes ?
Thanks
Prathamesh
[-- Attachment #2: pr80613-1.txt --]
[-- Type: text/plain, Size: 2526 bytes --]
2017-05-04 Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>
PR tree-optimization/80613
* tree-ssa-dce.c (propagate_necessity): Add cases for BUILT_IN_STRDUP
and BUILT_IN_STRNDUP.
* (eliminate_unnecessary_stmts): Likewise.
testsuite/
* gcc.dg/tree-ssa/pr80613-1.c: New test-case.
* gcc.dg/tree-ssa/pr80613-2.c: New test-case.
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr80613-1.c b/gcc/testsuite/gcc.dg/tree-ssa/pr80613-1.c
new file mode 100644
index 00000000000..56176427922
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr80613-1.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+char *a(int);
+int b;
+
+void c() {
+ for (;;) {
+ char d = *a(b);
+ char *e = __builtin_strdup (&d);
+ __builtin_free(e);
+ }
+}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr80613-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr80613-2.c
new file mode 100644
index 00000000000..c58cc08d6c5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr80613-2.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-cddce1" } */
+
+/* There should only be one instance of __builtin_strdup after cddce1. */
+
+char *f(char **tt)
+{
+ char *t = *tt;
+ char *p;
+
+ p = __builtin_strdup (t);
+ p = __builtin_strdup (t);
+ return p;
+}
+
+/* { dg-final { scan-tree-dump-times "__builtin_strdup" 1 "cddce1" } } */
diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
index e17659df91f..7c05f981307 100644
--- a/gcc/tree-ssa-dce.c
+++ b/gcc/tree-ssa-dce.c
@@ -852,7 +852,9 @@ propagate_necessity (bool aggressive)
== BUILT_IN_ALLOCA_WITH_ALIGN)
|| DECL_FUNCTION_CODE (callee) == BUILT_IN_STACK_SAVE
|| DECL_FUNCTION_CODE (callee) == BUILT_IN_STACK_RESTORE
- || DECL_FUNCTION_CODE (callee) == BUILT_IN_ASSUME_ALIGNED))
+ || DECL_FUNCTION_CODE (callee) == BUILT_IN_ASSUME_ALIGNED
+ || DECL_FUNCTION_CODE (callee) == BUILT_IN_STRDUP
+ || DECL_FUNCTION_CODE (callee) == BUILT_IN_STRNDUP))
continue;
/* Calls implicitly load from memory, their arguments
@@ -1353,6 +1355,8 @@ eliminate_unnecessary_stmts (void)
&& DECL_FUNCTION_CODE (call) != BUILT_IN_MALLOC
&& DECL_FUNCTION_CODE (call) != BUILT_IN_CALLOC
&& DECL_FUNCTION_CODE (call) != BUILT_IN_ALLOCA
+ && DECL_FUNCTION_CODE (call) != BUILT_IN_STRDUP
+ && DECL_FUNCTION_CODE (call) != BUILT_IN_STRNDUP
&& (DECL_FUNCTION_CODE (call)
!= BUILT_IN_ALLOCA_WITH_ALIGN)))
/* Avoid doing so for bndret calls for the same reason. */
next reply other threads:[~2017-05-04 16:01 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-04 16:03 Prathamesh Kulkarni [this message]
2017-05-04 17:55 ` PR80613 Martin Sebor
2017-05-04 18:19 ` PR80613 Trevor Saunders
2017-05-04 19:03 ` PR80613 Jeff Law
2017-05-04 19:06 ` PR80613 Jeff Law
2017-05-05 7:20 ` PR80613 Richard Biener
2017-05-05 9:16 ` PR80613 Prathamesh Kulkarni
2017-05-05 10:27 ` PR80613 Richard Biener
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAAgBjM=DUQ=mxxXscCzNXY1mJ=OsxP3yUAJBF6AZxrL6Zk6QwA@mail.gmail.com' \
--to=prathamesh.kulkarni@linaro.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=rguenther@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).