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
2017-05-04  Prathamesh Kulkarni  <prathamesh.kulka...@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.  */

Reply via email to