On Tue, Aug 19, 2014 at 7:20 AM, lin zuojian <manjian2...@gmail.com> wrote:
> Hi,
> Here patch v2. Move the function as Andrew instructed.
>
> * tree-inline.c (setup_one_parameter): Add strict aliasing check.
> * c-family/c-common.c (strict_aliasing_warning): Move to alias.c.
> * c-family/c-common.h (strict_aliasing_warning): Move to tree.h.
> * alias.c (strict_aliasing_warning): New function moved from
> c-family/c-common.c.
> * tree.h (strict_aliasing_warning): New function declaration moved from
> c-family/c-common.h.
> * testsuite/gcc.dg/Wstrict-aliasing-inline.C: New test.
Generally I don't think we want to expand the use of the IMHO broken
strict_aliasing_warning code.
We should be able to do better after some constant/forward propagation,
scanning for MEM_REFs operating on decls and accessing those with
an alias set that is not a subset of the set of the decl. Like simply
warn as part of the tree-ssa-forwprop.c sweep (maybe only for those
MEM_REFs we simplified to avoid duplicates, but then also watch
for CCP doing the same).
Richard.
> ---
> gcc/ChangeLog | 11 +++
> gcc/alias.c | 78
> ++++++++++++++++++++++
> gcc/c-family/c-common.c | 78
> ----------------------
> gcc/c-family/c-common.h | 1 -
> .../gcc.dg/Wstrict-aliasing-inline-parameter.C | 42 ++++++++++++
> gcc/tree-inline.c | 4 ++
> gcc/tree.h | 2 +
> 7 files changed, 137 insertions(+), 79 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/Wstrict-aliasing-inline-parameter.C
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 8ccdde2..6514313 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,14 @@
> +2014-08-19 Lin Zuojian <manjian2...@gmail.com>
> +
> + * tree-inline.c (setup_one_parameter): Add strict aliasing check.
> + * c-family/c-common.c (strict_aliasing_warning): Move to alias.c.
> + * c-family/c-common.h (strict_aliasing_warning): Move to tree.h.
> + * alias.c (strict_aliasing_warning): New function moved from
> + c-family/c-common.c.
> + * tree.h (strict_aliasing_warning): New function declaration moved
> from
> + c-family/c-common.h.
> + * testsuite/gcc.dg/Wstrict-aliasing-inline.C: New test.
> +
> 2014-08-19 David Malcolm <dmalc...@redhat.com>
>
> * basic-block.h (BB_HEAD): Convert to a function. Strengthen the
> diff --git a/gcc/alias.c b/gcc/alias.c
> index 39df09b..8496435 100644
> --- a/gcc/alias.c
> +++ b/gcc/alias.c
> @@ -3044,4 +3044,82 @@ end_alias_analysis (void)
> sbitmap_free (reg_known_equiv_p);
> }
>
> +/* Print a warning about casts that might indicate violation
> + of strict aliasing rules if -Wstrict-aliasing is used and
> + strict aliasing mode is in effect. OTYPE is the original
> + TREE_TYPE of EXPR, and TYPE the type we're casting to. */
> +
> +bool
> +strict_aliasing_warning (tree otype, tree type, tree expr)
> +{
> + /* Strip pointer conversion chains and get to the correct original type.
> */
> + STRIP_NOPS (expr);
> + otype = TREE_TYPE (expr);
> +
> + if (!(flag_strict_aliasing
> + && POINTER_TYPE_P (type)
> + && POINTER_TYPE_P (otype)
> + && !VOID_TYPE_P (TREE_TYPE (type)))
> + /* If the type we are casting to is a ref-all pointer
> + dereferencing it is always valid. */
> + || TYPE_REF_CAN_ALIAS_ALL (type))
> + return false;
> +
> + if ((warn_strict_aliasing > 1) && TREE_CODE (expr) == ADDR_EXPR
> + && (DECL_P (TREE_OPERAND (expr, 0))
> + || handled_component_p (TREE_OPERAND (expr, 0))))
> + {
> + /* Casting the address of an object to non void pointer. Warn
> + if the cast breaks type based aliasing. */
> + if (!COMPLETE_TYPE_P (TREE_TYPE (type)) && warn_strict_aliasing == 2)
> + {
> + warning (OPT_Wstrict_aliasing, "type-punning to incomplete type "
> + "might break strict-aliasing rules");
> + return true;
> + }
> + else
> + {
> + /* warn_strict_aliasing >= 3. This includes the default (3).
> + Only warn if the cast is dereferenced immediately. */
> + alias_set_type set1 =
> + get_alias_set (TREE_TYPE (TREE_OPERAND (expr, 0)));
> + alias_set_type set2 = get_alias_set (TREE_TYPE (type));
> +
> + if (set1 != set2 && set2 != 0
> + && (set1 == 0 || !alias_sets_conflict_p (set1, set2)))
> + {
> + warning (OPT_Wstrict_aliasing, "dereferencing type-punned "
> + "pointer will break strict-aliasing rules");
> + return true;
> + }
> + else if (warn_strict_aliasing == 2
> + && !alias_sets_must_conflict_p (set1, set2))
> + {
> + warning (OPT_Wstrict_aliasing, "dereferencing type-punned "
> + "pointer might break strict-aliasing rules");
> + return true;
> + }
> + }
> + }
> + else
> + if ((warn_strict_aliasing == 1) && !VOID_TYPE_P (TREE_TYPE (otype)))
> + {
> + /* At this level, warn for any conversions, even if an address is
> + not taken in the same statement. This will likely produce many
> + false positives, but could be useful to pinpoint problems that
> + are not revealed at higher levels. */
> + alias_set_type set1 = get_alias_set (TREE_TYPE (otype));
> + alias_set_type set2 = get_alias_set (TREE_TYPE (type));
> + if (!COMPLETE_TYPE_P (type)
> + || !alias_sets_must_conflict_p (set1, set2))
> + {
> + warning (OPT_Wstrict_aliasing, "dereferencing type-punned "
> + "pointer might break strict-aliasing rules");
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> #include "gt-alias.h"
> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> index acc9a20..bc4f420 100644
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -1846,84 +1846,6 @@ warn_if_unused_value (const_tree exp, location_t locus)
> }
>
>
> -/* Print a warning about casts that might indicate violation
> - of strict aliasing rules if -Wstrict-aliasing is used and
> - strict aliasing mode is in effect. OTYPE is the original
> - TREE_TYPE of EXPR, and TYPE the type we're casting to. */
> -
> -bool
> -strict_aliasing_warning (tree otype, tree type, tree expr)
> -{
> - /* Strip pointer conversion chains and get to the correct original type.
> */
> - STRIP_NOPS (expr);
> - otype = TREE_TYPE (expr);
> -
> - if (!(flag_strict_aliasing
> - && POINTER_TYPE_P (type)
> - && POINTER_TYPE_P (otype)
> - && !VOID_TYPE_P (TREE_TYPE (type)))
> - /* If the type we are casting to is a ref-all pointer
> - dereferencing it is always valid. */
> - || TYPE_REF_CAN_ALIAS_ALL (type))
> - return false;
> -
> - if ((warn_strict_aliasing > 1) && TREE_CODE (expr) == ADDR_EXPR
> - && (DECL_P (TREE_OPERAND (expr, 0))
> - || handled_component_p (TREE_OPERAND (expr, 0))))
> - {
> - /* Casting the address of an object to non void pointer. Warn
> - if the cast breaks type based aliasing. */
> - if (!COMPLETE_TYPE_P (TREE_TYPE (type)) && warn_strict_aliasing == 2)
> - {
> - warning (OPT_Wstrict_aliasing, "type-punning to incomplete type "
> - "might break strict-aliasing rules");
> - return true;
> - }
> - else
> - {
> - /* warn_strict_aliasing >= 3. This includes the default (3).
> - Only warn if the cast is dereferenced immediately. */
> - alias_set_type set1 =
> - get_alias_set (TREE_TYPE (TREE_OPERAND (expr, 0)));
> - alias_set_type set2 = get_alias_set (TREE_TYPE (type));
> -
> - if (set1 != set2 && set2 != 0
> - && (set1 == 0 || !alias_sets_conflict_p (set1, set2)))
> - {
> - warning (OPT_Wstrict_aliasing, "dereferencing type-punned "
> - "pointer will break strict-aliasing rules");
> - return true;
> - }
> - else if (warn_strict_aliasing == 2
> - && !alias_sets_must_conflict_p (set1, set2))
> - {
> - warning (OPT_Wstrict_aliasing, "dereferencing type-punned "
> - "pointer might break strict-aliasing rules");
> - return true;
> - }
> - }
> - }
> - else
> - if ((warn_strict_aliasing == 1) && !VOID_TYPE_P (TREE_TYPE (otype)))
> - {
> - /* At this level, warn for any conversions, even if an address is
> - not taken in the same statement. This will likely produce many
> - false positives, but could be useful to pinpoint problems that
> - are not revealed at higher levels. */
> - alias_set_type set1 = get_alias_set (TREE_TYPE (otype));
> - alias_set_type set2 = get_alias_set (TREE_TYPE (type));
> - if (!COMPLETE_TYPE_P (type)
> - || !alias_sets_must_conflict_p (set1, set2))
> - {
> - warning (OPT_Wstrict_aliasing, "dereferencing type-punned "
> - "pointer might break strict-aliasing rules");
> - return true;
> - }
> - }
> -
> - return false;
> -}
> -
> /* Warn about memset (&a, 0, sizeof (&a)); and similar mistakes with
> sizeof as last operand of certain builtins. */
>
> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> index 26aaee2..1a10038 100644
> --- a/gcc/c-family/c-common.h
> +++ b/gcc/c-family/c-common.h
> @@ -770,7 +770,6 @@ extern void binary_op_error (location_t, enum tree_code,
> tree, tree);
> extern tree fix_string_type (tree);
> extern void constant_expression_warning (tree);
> extern void constant_expression_error (tree);
> -extern bool strict_aliasing_warning (tree, tree, tree);
> extern void sizeof_pointer_memaccess_warning (location_t *, tree,
> vec<tree, va_gc> *, tree *,
> bool (*) (tree, tree));
> diff --git a/gcc/testsuite/gcc.dg/Wstrict-aliasing-inline-parameter.C
> b/gcc/testsuite/gcc.dg/Wstrict-aliasing-inline-parameter.C
> new file mode 100644
> index 0000000..9a667be
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/Wstrict-aliasing-inline-parameter.C
> @@ -0,0 +1,42 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wstrict-aliasing=10 -fstrict-aliasing" } */
> +
> +struct A
> +{
> + int a;
> + int b;
> + int c;
> +};
> +
> +static inline int hash2(const unsigned short* change2, int len)
> +{
> + int result = 0;
> + for(int i = 0; i < len; ++i) {
> +
> + result += change2[i];
> + result ^= result << 11;
> + result += result >> 17;
> +
> + // Force "avalanching" of final 31 bits.
> + result ^= result << 3;
> + result += result >> 5;
> + result ^= result << 2;
> + result += result >> 15;
> + result ^= result << 10;
> + }
> + return result;
> +}
> +
> +static inline int hash(const void* change1, int len)
> +{
> + return hash2(static_cast<const unsigned short*>(change1), len / 2);
> +}
> +
> +
> +int foo(int a, int b, int c)
> +{
> + struct A a_struct = {a, b, c};
> + return hash(&a_struct, sizeof(struct A));
> +}
> +
> +/* { dg-message "dereferencing type-punned pointer will break
> strict-aliasing rules" } */
> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> index b6ecaa4..95e745c 100644
> --- a/gcc/tree-inline.c
> +++ b/gcc/tree-inline.c
> @@ -2913,6 +2913,10 @@ setup_one_parameter (copy_body_data *id, tree p, tree
> value, tree fn,
> }
> }
>
> + if (warn_strict_aliasing > 2)
> + if (strict_aliasing_warning (TREE_TYPE (rhs), TREE_TYPE(p), rhs))
> + warning (OPT_Wstrict_aliasing, "during inlining function %s into
> function %s", fndecl_name(fn), function_name(cfun));
> +
> /* Make an equivalent VAR_DECL. Note that we must NOT remap the type
> here since the type of this decl must be visible to the calling
> function. */
> diff --git a/gcc/tree.h b/gcc/tree.h
> index e000e4e..e7c4318 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -4788,4 +4788,6 @@ extern tree get_inner_reference (tree, HOST_WIDE_INT *,
> HOST_WIDE_INT *,
> EXP, an ARRAY_REF or an ARRAY_RANGE_REF. */
> extern tree array_ref_low_bound (tree);
>
> +extern bool strict_aliasing_warning (tree, tree, tree);
> +
> #endif /* GCC_TREE_H */
> --
> 1.9.1
>
> ---
> Lin Zuojian
