Warray-bounds: Warn only for generic address spaces

  The warning is falsely triggered for THREAD_SELF in glibc when
accessing TCB through the segment register.

gcc/ChangeLog:

	* gimple-array-bounds.cc
	(array_bounds_checker::check_mem_ref): Bail out for
	non-generic address spaces.

gcc/testsuite/ChangeLog:

	* gcc.target/i386/addr-space-3.c: New test case.

Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
---
 gcc/gimple-array-bounds.cc                   | 3 +++
 gcc/testsuite/gcc.target/i386/addr-space-3.c | 5 +++++
 2 files changed, 8 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/i386/addr-space-3.c
  

On 10/12/21 12:33 PM, Siddhesh Poyarekar wrote:
> The warning is falsely triggered for THREAD_SELF in glibc when
> accessing TCB through the segment register.

Thanks for looking into it!  The Glibc warning is being tracked
in PR 102630.  The root cause behind it is in compute_objsize_r
in pointer-query.cc (which is used by -Warray-bounds as well as
other warnings).  I just posted a patch for it the other day;
it's waiting for approval (though as Joseph noted, I need to
adjust the test and either make it target-independent or move
it under i386).

Martin

PS Noticing gcc.target/i386/addr-space-2.c makes me wish
-Warray-bounds were enabled by default, like other out-of-bounds
warnings, and reminds me that it should be able to run even at
-O1 (and -O0).

> 
> gcc/ChangeLog:
> 
> 	* gimple-array-bounds.cc
> 	(array_bounds_checker::check_mem_ref): Bail out for
> 	non-generic address spaces.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/i386/addr-space-3.c: New test case.
> 
> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
> ---
>   gcc/gimple-array-bounds.cc                   | 3 +++
>   gcc/testsuite/gcc.target/i386/addr-space-3.c | 5 +++++
>   2 files changed, 8 insertions(+)
>   create mode 100644 gcc/testsuite/gcc.target/i386/addr-space-3.c
> 
> diff --git a/gcc/gimple-array-bounds.cc b/gcc/gimple-array-bounds.cc
> index 0517e5ddd8e..36fc1dbe3f8 100644
> --- a/gcc/gimple-array-bounds.cc
> +++ b/gcc/gimple-array-bounds.cc
> @@ -432,6 +432,9 @@ array_bounds_checker::check_mem_ref (location_t location, tree ref,
>     if (aref.offset_in_range (axssize))
>       return false;
>   
> +  if (!ADDR_SPACE_GENERIC_P (TYPE_ADDR_SPACE (axstype)))
> +    return false;
> +
>     if (TREE_CODE (aref.ref) == SSA_NAME)
>       {
>         gimple *def = SSA_NAME_DEF_STMT (aref.ref);
> diff --git a/gcc/testsuite/gcc.target/i386/addr-space-3.c b/gcc/testsuite/gcc.target/i386/addr-space-3.c
> new file mode 100644
> index 00000000000..4bd940e696a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/addr-space-3.c
> @@ -0,0 +1,5 @@
> +/* Verify that __seg_fs/gs marked variables do not trigger an array bounds
> +   warning.  */
> +/* { dg-do compile */
> +/* { dg-options "-O2 -Warray-bounds" } */
> +#include "addr-space-2.c"
>
  

On 10/13/21 00:36, Martin Sebor wrote:
> On 10/12/21 12:33 PM, Siddhesh Poyarekar wrote:
>> The warning is falsely triggered for THREAD_SELF in glibc when
>> accessing TCB through the segment register.
> 
> Thanks for looking into it!  The Glibc warning is being tracked
> in PR 102630.  The root cause behind it is in compute_objsize_r
> in pointer-query.cc (which is used by -Warray-bounds as well as
> other warnings).  I just posted a patch for it the other day;
> it's waiting for approval (though as Joseph noted, I need to
> adjust the test and either make it target-independent or move
> it under i386).

Ahh, targetm.addr_space.zero_address_valid was what I was looking for 
and didn't find.  Your fix looks good to me module moving the test out 
into gcc.target/i386.

Thanks,
Siddhesh
  

On Tue, Oct 12, 2021 at 8:34 PM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>
> The warning is falsely triggered for THREAD_SELF in glibc when
> accessing TCB through the segment register.

I think this is a more generic bug - the warning is also bogus if the
general address space is involved.

Martin?

> gcc/ChangeLog:
>
>         * gimple-array-bounds.cc
>         (array_bounds_checker::check_mem_ref): Bail out for
>         non-generic address spaces.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/i386/addr-space-3.c: New test case.
>
> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
> ---
>  gcc/gimple-array-bounds.cc                   | 3 +++
>  gcc/testsuite/gcc.target/i386/addr-space-3.c | 5 +++++
>  2 files changed, 8 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/i386/addr-space-3.c
>
> diff --git a/gcc/gimple-array-bounds.cc b/gcc/gimple-array-bounds.cc
> index 0517e5ddd8e..36fc1dbe3f8 100644
> --- a/gcc/gimple-array-bounds.cc
> +++ b/gcc/gimple-array-bounds.cc
> @@ -432,6 +432,9 @@ array_bounds_checker::check_mem_ref (location_t location, tree ref,
>    if (aref.offset_in_range (axssize))
>      return false;
>
> +  if (!ADDR_SPACE_GENERIC_P (TYPE_ADDR_SPACE (axstype)))
> +    return false;
> +
>    if (TREE_CODE (aref.ref) == SSA_NAME)
>      {
>        gimple *def = SSA_NAME_DEF_STMT (aref.ref);
> diff --git a/gcc/testsuite/gcc.target/i386/addr-space-3.c b/gcc/testsuite/gcc.target/i386/addr-space-3.c
> new file mode 100644
> index 00000000000..4bd940e696a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/addr-space-3.c
> @@ -0,0 +1,5 @@
> +/* Verify that __seg_fs/gs marked variables do not trigger an array bounds
> +   warning.  */
> +/* { dg-do compile */
> +/* { dg-options "-O2 -Warray-bounds" } */
> +#include "addr-space-2.c"
> --
> 2.31.1
>
  

On 10/13/21 13:50, Richard Biener wrote:
> On Tue, Oct 12, 2021 at 8:34 PM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>>
>> The warning is falsely triggered for THREAD_SELF in glibc when
>> accessing TCB through the segment register.
> 
> I think this is a more generic bug - the warning is also bogus if the
> general address space is involved.

I agree, Martin pointed me to a different fix that he posted earlier:

https://patchwork.sourceware.org/project/gcc/patch/806cc630-86ef-2e3f-f72b-68bab2cd3e86@gmail.com/

Thanks,
Siddhesh