[PATCH] tree-optimization/109609 - correctly interpret arg size in fnspec

[PATCH] tree-optimization/109609 - correctly interpret arg size in fnspec

[Richard Biener]

By majority vote and a hint from the API name which is
arg_max_access_size_given_by_arg_p this interprets a memory access
size specified as given as other argument such as for strncpy
in the testcase which has "1cO313" as specifying the _maximum_
size read/written rather than the exact size.  There are two
uses interpreting it that way already and one differing.  The
following adjusts the differing and clarifies the documentation.

Bootstrap & regtest running on x86_64-unknown-linux-gnu.

It looks like arg_access_size_given_by_type_p is interpreted in
a similar way but the API doesn't reflect this (no _max).  On IRC
we discussed it would be nice to have both, exact and non-exact
size (for example use 'a'..'j' for the alternate variant).

I still wonder if my interpretation is correct in that '1'..'9'
specify a _bound_ for the size.  Honza, do you remember why
you wrote the IPA modref users this way?  Was that intentional?
Thus is my reverse engineering of the desired semantics correct?

Thanks,
Richard.

	PR tree-optimization/109609
	* attr-fnspec.h (arg_max_access_size_given_by_arg_p):
	Clarify semantics.
	* tree-ssa-alias.cc (check_fnspec): Correctly interpret
	the size given by arg_max_access_size_given_by_arg_p as
	maximum, not exact, size.

	* gcc.dg/torture/pr109609.c: New testcase.
---
 gcc/attr-fnspec.h                       |  4 ++--
 gcc/testsuite/gcc.dg/torture/pr109609.c | 26 +++++++++++++++++++++++++
 gcc/tree-ssa-alias.cc                   | 18 ++++++++++++++---
 3 files changed, 43 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr109609.c

diff --git a/gcc/attr-fnspec.h b/gcc/attr-fnspec.h
index acf1c5f6be7..99d5f896e8b 100644
--- a/gcc/attr-fnspec.h
+++ b/gcc/attr-fnspec.h
@@ -54,7 +54,7 @@
      ' '        nothing is known
      't'	the size of value written/read corresponds to the size of
 		of the pointed-to type of the argument type
-     '1'...'9'  specifies the size of value written/read is given by the
+     '1'...'9'  specifies the size of value written/read is bound by the
 		specified argument
  */
 
@@ -169,7 +169,7 @@ public:
 	   && str[idx] != 'x' && str[idx] != 'X';
   }
 
-  /* Return true if load of memory pointed to by argument I is specified
+  /* Return true if load of memory pointed to by argument I is bound
      by another argument.  In this case set ARG.  */
   bool
   arg_max_access_size_given_by_arg_p (unsigned int i, unsigned int *arg)
diff --git a/gcc/testsuite/gcc.dg/torture/pr109609.c b/gcc/testsuite/gcc.dg/torture/pr109609.c
new file mode 100644
index 00000000000..0e191cd1ee8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr109609.c
@@ -0,0 +1,26 @@
+/* { dg-do run } */
+
+#define N 23
+#define MAX_LEN 13
+char dst[N + 1];
+
+void __attribute__((noipa))
+invert(const char *id)
+{
+  char buf[MAX_LEN];
+  char *ptr = buf + sizeof(buf);  // start from the end of buf
+  *(--ptr) = '\0';                // terminate string
+  while (*id && ptr > buf) {
+    *(--ptr) = *(id++);           // copy id backwards
+  }
+  __builtin_strncpy(dst, ptr, N);           // copy ptr/buf to dst
+}
+
+
+int main()
+{
+  invert("abcde");
+  if (__builtin_strcmp(dst, "edcba"))
+    __builtin_abort();
+  return 0;
+}
diff --git a/gcc/tree-ssa-alias.cc b/gcc/tree-ssa-alias.cc
index 8a1ec9091fa..e0693e146bf 100644
--- a/gcc/tree-ssa-alias.cc
+++ b/gcc/tree-ssa-alias.cc
@@ -2726,9 +2726,21 @@ check_fnspec (gcall *call, ao_ref *ref, bool clobber)
 		      t = TREE_CHAIN (t);
 		    size = TYPE_SIZE_UNIT (TREE_TYPE (TREE_VALUE (t)));
 		  }
-		ao_ref_init_from_ptr_and_size (&dref,
-					       gimple_call_arg (call, i),
-					       size);
+		poly_int64 size_hwi;
+		if (size
+		    && poly_int_tree_p (size, &size_hwi)
+		    && coeffs_in_range_p (size_hwi, 0,
+					  HOST_WIDE_INT_MAX / BITS_PER_UNIT))
+		  {
+		    size_hwi = size_hwi * BITS_PER_UNIT;
+		    ao_ref_init_from_ptr_and_range (&dref,
+						    gimple_call_arg (call, i),
+						    true, 0, -1, size_hwi);
+		  }
+		else
+		  ao_ref_init_from_ptr_and_range (&dref,
+						  gimple_call_arg (call, i),
+						  false, 0, -1, -1);
 		if (refs_may_alias_p_1 (&dref, ref, false))
 		  return 1;
 	      }
-- 
2.35.3

Re: [PATCH] tree-optimization/109609 - correctly interpret arg size in fnspec

[Jan Hubicka]

> By majority vote and a hint from the API name which is
> arg_max_access_size_given_by_arg_p this interprets a memory access
> size specified as given as other argument such as for strncpy
> in the testcase which has "1cO313" as specifying the _maximum_
> size read/written rather than the exact size.  There are two
> uses interpreting it that way already and one differing.  The
> following adjusts the differing and clarifies the documentation.

This makes sense to me. Bit sad is memcpy/memset may be common enough so
the extra disambiguation based on maximal object size may be useful.
> 
> Bootstrap & regtest running on x86_64-unknown-linux-gnu.
> 
> It looks like arg_access_size_given_by_type_p is interpreted in
> a similar way but the API doesn't reflect this (no _max).  On IRC
> we discussed it would be nice to have both, exact and non-exact
> size (for example use 'a'..'j' for the alternate variant).

Hmm 'a'...'j' may work to stick this information in....
> 
> I still wonder if my interpretation is correct in that '1'..'9'
> specify a _bound_ for the size.  Honza, do you remember why
> you wrote the IPA modref users this way?  Was that intentional?
> Thus is my reverse engineering of the desired semantics correct?

My fnspec implementation was based on what the code was doing
beforehand. In GCC 10 we have:

        case BUILT_IN_STRCPY:
        case BUILT_IN_STRNCPY:
        case BUILT_IN_MEMCPY:
        case BUILT_IN_MEMMOVE:
        case BUILT_IN_MEMPCPY:
        case BUILT_IN_STPCPY:
        case BUILT_IN_STPNCPY:
        case BUILT_IN_TM_MEMCPY:
        case BUILT_IN_TM_MEMMOVE:
          {
            ao_ref dref;
            tree size = NULL_TREE;
            if (gimple_call_num_args (call) == 3)
              size = gimple_call_arg (call, 2);
            ao_ref_init_from_ptr_and_size (&dref,
                                           gimple_call_arg (call, 1),
                                           size);
            return refs_may_alias_p_1 (&dref, ref, false);
          }
So it seem that we handled strncpy the way claiming full size access
there too and I simply copied it into the new implementation.

While adding fnspec I just used adapted this code, so the difference
between modref and tree-ssa-alias implementation was not quite
intentional.
> 
> Thanks,
> Richard.
> 
> 	PR tree-optimization/109609
> 	* attr-fnspec.h (arg_max_access_size_given_by_arg_p):
> 	Clarify semantics.
> 	* tree-ssa-alias.cc (check_fnspec): Correctly interpret
> 	the size given by arg_max_access_size_given_by_arg_p as
> 	maximum, not exact, size.
> 
> 	* gcc.dg/torture/pr109609.c: New testcase.
Patch looks good to me (modulo the fact that it would be nice to be also
able to handle memcpy/memset precisely).

Honza