Re: [PATCH 8/9] Add TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID

[Richard Biener <richard dot guenther at gmail dot com>]

On Thu, Oct 8, 2015 at 6:59 AM, Richard Henderson <rth@redhat.com> wrote:
>         * target.def (TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID): New.
>         * targhooks.h (default_addr_space_zero_address_valid): Declare.
>         * targhooks.c (default_addr_space_zero_address_valid): New.
>         * doc/tm.texi, doc/tm.texi.in: Update.
>         * config/i386/i386.c (ix86_addr_space_zero_address_valid): New.
>         (TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID): New.
>         * fold-const.c (const_unop) [ADDR_SPACE_CONVERT_EXPR]: Disable
>         folding of 0 when it is a valid address for the source space.
>         * gimple.c (check_loadstore): Disable noticing dereference when
>         0 is a valid address for the space.

I think this is incomplete and you need to look at all places that check for
flag_delete_null_pointer_checks (ick, the ubsan abuse looks interesting...).
We'd best abstract that flag check somewhere, only doing the address-space
check for ! ADDR_SPACE_GENERIC.

I also wonder about the const_unop change - if the target address-space
has a valid 0 but the source has not then you create a valid object address
from an invalid one?

The check_loadstore change should instead have adjusted the
flag_delete_null_pointer_checks guard in infer_nonnull_range_by_dereference.

Yes, that flag is used to say whether 0 is a valid address ...

Richard.

> ---
>  gcc/config/i386/i386.c | 10 ++++++++++
>  gcc/doc/tm.texi        |  5 +++++
>  gcc/doc/tm.texi.in     |  2 ++
>  gcc/fold-const.c       |  6 +++++-
>  gcc/gimple.c           | 12 +++++++++---
>  gcc/target.def         |  9 +++++++++
>  gcc/targhooks.c        |  9 +++++++++
>  gcc/targhooks.h        |  1 +
>  8 files changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index f7abb00..e2dec2a 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -53669,6 +53669,16 @@ ix86_addr_space_convert (rtx op, tree from_type, tree to_type)
>  #undef TARGET_ADDR_SPACE_CONVERT
>  #define TARGET_ADDR_SPACE_CONVERT ix86_addr_space_convert
>
> +/* All use of segmentation is assumed to make address 0 valid.  */
> +
> +static bool
> +ix86_addr_space_zero_address_valid (addr_space_t as)
> +{
> +  return as != ADDR_SPACE_GENERIC;
> +}
> +#undef TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID
> +#define TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID ix86_addr_space_zero_address_valid
> +
>  /* Initialize the GCC target structure.  */
>  #undef TARGET_RETURN_IN_MEMORY
>  #define TARGET_RETURN_IN_MEMORY ix86_return_in_memory
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index 72366b9..238f5f5 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -10312,6 +10312,11 @@ arithmetic operations.  Pointers to a superset address space can be
>  converted to pointers to a subset address space via explicit casts.
>  @end deftypefn
>
> +@deftypefn {Target Hook} bool TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID (addr_space_t @var{as})
> +Define this to modify the default handling of address 0 for the
> +address space.  Return true if 0 should be considered a valid address.
> +@end deftypefn
> +
>  @deftypefn {Target Hook} rtx TARGET_ADDR_SPACE_CONVERT (rtx @var{op}, tree @var{from_type}, tree @var{to_type})
>  Define this to convert the pointer expression represented by the RTL
>  @var{op} with type @var{from_type} that points to a named address
> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> index d8d0087..9230fb2 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -7432,6 +7432,8 @@ c_register_addr_space ("__ea", ADDR_SPACE_EA);
>
>  @hook TARGET_ADDR_SPACE_SUBSET_P
>
> +@hook TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID
> +
>  @hook TARGET_ADDR_SPACE_CONVERT
>
>  @node Misc
> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
> index 2851a29..2e06217 100644
> --- a/gcc/fold-const.c
> +++ b/gcc/fold-const.c
> @@ -1564,7 +1564,11 @@ const_unop (enum tree_code code, tree type, tree arg0)
>        return fold_convert_const (code, type, arg0);
>
>      case ADDR_SPACE_CONVERT_EXPR:
> -      if (integer_zerop (arg0))
> +      /* If the source address is 0, and the source address space
> +        cannot have a valid object at 0, fold to dest type null.  */
> +      if (integer_zerop (arg0)
> +         && !(targetm.addr_space.zero_address_valid
> +              (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0))))))
>         return fold_convert_const (code, type, arg0);
>        break;
>
> diff --git a/gcc/gimple.c b/gcc/gimple.c
> index c3762e1..e551dac 100644
> --- a/gcc/gimple.c
> +++ b/gcc/gimple.c
> @@ -2618,9 +2618,15 @@ nonfreeing_call_p (gimple *call)
>  static bool
>  check_loadstore (gimple *, tree op, tree, void *data)
>  {
> -  if ((TREE_CODE (op) == MEM_REF || TREE_CODE (op) == TARGET_MEM_REF)
> -      && operand_equal_p (TREE_OPERAND (op, 0), (tree)data, 0))
> -    return true;
> +  if (TREE_CODE (op) == MEM_REF || TREE_CODE (op) == TARGET_MEM_REF)
> +    {
> +      /* Some address spaces may legitimately dereference zero.  */
> +      addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (op));
> +      if (targetm.addr_space.zero_address_valid (as))
> +       return false;
> +
> +      return operand_equal_p (TREE_OPERAND (op, 0), (tree)data, 0);
> +    }
>    return false;
>  }
>
> diff --git a/gcc/target.def b/gcc/target.def
> index d29aad5..f75ce97 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -3185,6 +3185,15 @@ converted to pointers to a subset address space via explicit casts.",
>   bool, (addr_space_t subset, addr_space_t superset),
>   default_addr_space_subset_p)
>
> +/* True if 0 is a valid address in the address space, or false if
> +   0 is a NULL in the address space.  */
> +DEFHOOK
> +(zero_address_valid,
> + "Define this to modify the default handling of address 0 for the\n\
> +address space.  Return true if 0 should be considered a valid address.",
> + bool, (addr_space_t as),
> + default_addr_space_zero_address_valid)
> +
>  /* Function to convert an rtl expression from one address space to another.  */
>  DEFHOOK
>  (convert,
> diff --git a/gcc/targhooks.c b/gcc/targhooks.c
> index a8a243c..c78a540 100644
> --- a/gcc/targhooks.c
> +++ b/gcc/targhooks.c
> @@ -1256,6 +1256,15 @@ default_addr_space_subset_p (addr_space_t subset, addr_space_t superset)
>    return (subset == superset);
>  }
>
> +/* The default hook for determining if 0 within a named address
> +   space is a valid address.  */
> +
> +bool
> +default_addr_space_zero_address_valid (addr_space_t as ATTRIBUTE_UNUSED)
> +{
> +  return false;
> +}
> +
>  /* The default hook for TARGET_ADDR_SPACE_CONVERT. This hook should never be
>     called for targets with only a generic address space.  */
>
> diff --git a/gcc/targhooks.h b/gcc/targhooks.h
> index 77c284a..ade3327 100644
> --- a/gcc/targhooks.h
> +++ b/gcc/targhooks.h
> @@ -173,6 +173,7 @@ extern bool default_addr_space_legitimate_address_p (machine_mode, rtx,
>  extern rtx default_addr_space_legitimize_address (rtx, rtx, machine_mode,
>                                                   addr_space_t);
>  extern bool default_addr_space_subset_p (addr_space_t, addr_space_t);
> +extern bool default_addr_space_zero_address_valid (addr_space_t);
>  extern rtx default_addr_space_convert (rtx, tree, tree);
>  extern unsigned int default_case_values_threshold (void);
>  extern bool default_have_conditional_execution (void);
> --
> 2.4.3
>