[RFC] tree-optimization: fix optimize-out variables passed into func to alloc

[RFC] tree-optimization: fix optimize-out variables passed into func to alloc

[Alexey Lapshin]

After updating to GCC newer than 11.4.0 we found that some code started
to fail if it was built with size optimization (-Os).
You can find testsuite for reproduction in the attached patch.

The simplified version affected code looks like this:

void alloc_function (unsigned char **data_p) {
  *data_p = malloc (8);
  assert(*data_p != NULL);
}
int main () {
  int *data;
  alloc_function (&data);
  printf ("data pointer is %p", data); // prints NULL(compile with -Os)
}

If the type of passed argument is equal to the type in alloc_function
declaration it works perfectly. Also helps change one or both types to
void.

I found that issue started to appear from commit
d119f34c952f8718fdbabc63e2f369a16e92fa07
if-statement which leads to this issue was found and after being
removed seems it works well.

Could you please elaborate on what cases exactly this checking should
optimize?
I think it should also contain at least one more check for accessing
variable's memory to write..


---
 gcc/testsuite/gcc.dg/tree-ssa/alloc-in-func.c | 17 +++++++++++++++++
 gcc/tree-ssa-alias.cc                         |  2 --
 2 files changed, 17 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/alloc-in-func.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/alloc-in-func.c
b/gcc/testsuite/gcc.dg/tree-ssa/alloc-in-func.c
new file mode 100644
index 00000000000..b30c1cedcb9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/alloc-in-func.c
@@ -0,0 +1,17 @@
+/* { dg-do run } */
+/* { dg-options "-Os" } */
+
+#define assert(x) if (!(x)) __builtin_abort ()
+
+static inline void alloc_function (unsigned char **data_p)
+{
+    *data_p = (unsigned char *) __builtin_malloc (10);
+    assert (*data_p != (void *)0);
+}
+
+int main ()
+{
+    int *data = (void *)0;
+    alloc_function ((unsigned char **) &data);
+    assert (data != (void *)0);
+}
diff --git a/gcc/tree-ssa-alias.cc b/gcc/tree-ssa-alias.cc
index b8f107dfa52..9068db300e5 100644
--- a/gcc/tree-ssa-alias.cc
+++ b/gcc/tree-ssa-alias.cc
@@ -2608,8 +2608,6 @@ modref_may_conflict (const gcall *stmt,
 	  if (num_tests >= max_tests)
 	    return true;
 	  alias_stats.modref_tests++;
-	  if (!alias_sets_conflict_p (base_set, base_node->base))
-	    continue;
 	  num_tests++;
 	}
 
-- 
2.34.1

Re: [RFC] tree-optimization: fix optimize-out variables passed into func to alloc

[Andrew Pinski]

On Tue, Jan 17, 2023 at 7:36 AM Alexey Lapshin via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> After updating to GCC newer than 11.4.0 we found that some code started
> to fail if it was built with size optimization (-Os).
> You can find testsuite for reproduction in the attached patch.
>
> The simplified version affected code looks like this:
>
> void alloc_function (unsigned char **data_p) {
>   *data_p = malloc (8);
>   assert(*data_p != NULL);
> }
> int main () {
>   int *data;
>   alloc_function (&data);
>   printf ("data pointer is %p", data); // prints NULL(compile with -Os)
> }

This code is violating C/C++ aliasing rules.
You store to data via a "unsigned char*" and then do a load via "int*".
You can just use -fno-strict-aliasing if you want your code to just work.

Thanks,
Andrew Pinski

>
> If the type of passed argument is equal to the type in alloc_function
> declaration it works perfectly. Also helps change one or both types to
> void.
>
> I found that issue started to appear from commit
> d119f34c952f8718fdbabc63e2f369a16e92fa07
> if-statement which leads to this issue was found and after being
> removed seems it works well.
>
> Could you please elaborate on what cases exactly this checking should
> optimize?
> I think it should also contain at least one more check for accessing
> variable's memory to write..
>
>
> ---
>  gcc/testsuite/gcc.dg/tree-ssa/alloc-in-func.c | 17 +++++++++++++++++
>  gcc/tree-ssa-alias.cc                         |  2 --
>  2 files changed, 17 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/alloc-in-func.c
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/alloc-in-func.c
> b/gcc/testsuite/gcc.dg/tree-ssa/alloc-in-func.c
> new file mode 100644
> index 00000000000..b30c1cedcb9
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/alloc-in-func.c
> @@ -0,0 +1,17 @@
> +/* { dg-do run } */
> +/* { dg-options "-Os" } */
> +
> +#define assert(x) if (!(x)) __builtin_abort ()
> +
> +static inline void alloc_function (unsigned char **data_p)
> +{
> +    *data_p = (unsigned char *) __builtin_malloc (10);
> +    assert (*data_p != (void *)0);
> +}
> +
> +int main ()
> +{
> +    int *data = (void *)0;
> +    alloc_function ((unsigned char **) &data);
> +    assert (data != (void *)0);
> +}
> diff --git a/gcc/tree-ssa-alias.cc b/gcc/tree-ssa-alias.cc
> index b8f107dfa52..9068db300e5 100644
> --- a/gcc/tree-ssa-alias.cc
> +++ b/gcc/tree-ssa-alias.cc
> @@ -2608,8 +2608,6 @@ modref_may_conflict (const gcall *stmt,
>           if (num_tests >= max_tests)
>             return true;
>           alias_stats.modref_tests++;
> -         if (!alias_sets_conflict_p (base_set, base_node->base))
> -           continue;
>           num_tests++;
>         }
>
> --
> 2.34.1
>

Re: [RFC] tree-optimization: fix optimize-out variables passed into func to alloc

[Richard Biener]

On Tue, Jan 17, 2023 at 10:41 PM Andrew Pinski via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Tue, Jan 17, 2023 at 7:36 AM Alexey Lapshin via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > After updating to GCC newer than 11.4.0 we found that some code started
> > to fail if it was built with size optimization (-Os).
> > You can find testsuite for reproduction in the attached patch.
> >
> > The simplified version affected code looks like this:
> >
> > void alloc_function (unsigned char **data_p) {
> >   *data_p = malloc (8);
> >   assert(*data_p != NULL);
> > }
> > int main () {
> >   int *data;
> >   alloc_function (&data);
> >   printf ("data pointer is %p", data); // prints NULL(compile with -Os)
> > }
>
> This code is violating C/C++ aliasing rules.
> You store to data via a "unsigned char*" and then do a load via "int*".
> You can just use -fno-strict-aliasing if you want your code to just work.

You can also use void **data_p as a workaround, GCC treats void *
similar to a character type for aliasing rules (note that this is a GCC
extension and not guaranteed to work by the C/C++ standards).

Richard.

> Thanks,
> Andrew Pinski
>
> >
> > If the type of passed argument is equal to the type in alloc_function
> > declaration it works perfectly. Also helps change one or both types to
> > void.
> >
> > I found that issue started to appear from commit
> > d119f34c952f8718fdbabc63e2f369a16e92fa07
> > if-statement which leads to this issue was found and after being
> > removed seems it works well.
> >
> > Could you please elaborate on what cases exactly this checking should
> > optimize?
> > I think it should also contain at least one more check for accessing
> > variable's memory to write..
> >
> >
> > ---
> >  gcc/testsuite/gcc.dg/tree-ssa/alloc-in-func.c | 17 +++++++++++++++++
> >  gcc/tree-ssa-alias.cc                         |  2 --
> >  2 files changed, 17 insertions(+), 2 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/alloc-in-func.c
> >
> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/alloc-in-func.c
> > b/gcc/testsuite/gcc.dg/tree-ssa/alloc-in-func.c
> > new file mode 100644
> > index 00000000000..b30c1cedcb9
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/alloc-in-func.c
> > @@ -0,0 +1,17 @@
> > +/* { dg-do run } */
> > +/* { dg-options "-Os" } */
> > +
> > +#define assert(x) if (!(x)) __builtin_abort ()
> > +
> > +static inline void alloc_function (unsigned char **data_p)
> > +{
> > +    *data_p = (unsigned char *) __builtin_malloc (10);
> > +    assert (*data_p != (void *)0);
> > +}
> > +
> > +int main ()
> > +{
> > +    int *data = (void *)0;
> > +    alloc_function ((unsigned char **) &data);
> > +    assert (data != (void *)0);
> > +}
> > diff --git a/gcc/tree-ssa-alias.cc b/gcc/tree-ssa-alias.cc
> > index b8f107dfa52..9068db300e5 100644
> > --- a/gcc/tree-ssa-alias.cc
> > +++ b/gcc/tree-ssa-alias.cc
> > @@ -2608,8 +2608,6 @@ modref_may_conflict (const gcall *stmt,
> >           if (num_tests >= max_tests)
> >             return true;
> >           alias_stats.modref_tests++;
> > -         if (!alias_sets_conflict_p (base_set, base_node->base))
> > -           continue;
> >           num_tests++;
> >         }
> >
> > --
> > 2.34.1
> >