* [PATCH] Tweak array_at_struct_end_p
@ 2017-05-04 9:07 Richard Biener
2017-05-05 15:19 ` Christophe Lyon
0 siblings, 1 reply; 2+ messages in thread
From: Richard Biener @ 2017-05-04 9:07 UTC (permalink / raw)
To: gcc-patches
The following picks the changes suggested as followup for PR80533
that do not cause the warning regression on accessing a [0] array.
Additionally the patch removes the unnecessary allow_compref of the
function.
The question whether we want to allow an array to extend into
padding still stands. This patch allows it for C99 flex arrays
(but not pre-C99 GNU extension [0] due to the above warning
regression, also not for [1] or larger arrays we treat as flex arrays
when we can't see an underlying decl).
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
Richard.
2017-05-04 Richard Biener <rguenther@suse.de>
* tree.c (array_at_struct_end_p): Handle arrays at struct
end with flexarrays more conservatively. Refactor and treat
arrays of arrays or aggregates more strict. Fix
VIEW_CONVERT_EXPR handling. Remove allow_compref argument.
* tree.c (array_at_struct_end_p): Adjust prototype.
* emit-rtl.c (set_mem_attributes_minus_bitpos): Adjust.
* gimple-fold.c (get_range_strlen): Likewise.
* tree-chkp.c (chkp_may_narrow_to_field): Likewise.
Index: gcc/tree.c
===================================================================
--- gcc/tree.c (revision 247542)
+++ gcc/tree.c (working copy)
@@ -13227,18 +13235,26 @@ array_ref_up_bound (tree exp)
return NULL_TREE;
}
-/* Returns true if REF is an array reference to an array at the end of
- a structure. If this is the case, the array may be allocated larger
- than its upper bound implies. When ALLOW_COMPREF is true considers
- REF when it's a COMPONENT_REF in addition ARRAY_REF and
- ARRAY_RANGE_REF. */
+/* Returns true if REF is an array reference or a component reference
+ to an array at the end of a structure.
+ If this is the case, the array may be allocated larger
+ than its upper bound implies. */
bool
-array_at_struct_end_p (tree ref, bool allow_compref)
+array_at_struct_end_p (tree ref)
{
- if (TREE_CODE (ref) != ARRAY_REF
- && TREE_CODE (ref) != ARRAY_RANGE_REF
- && (!allow_compref || TREE_CODE (ref) != COMPONENT_REF))
+ tree atype;
+
+ if (TREE_CODE (ref) == ARRAY_REF
+ || TREE_CODE (ref) == ARRAY_RANGE_REF)
+ {
+ atype = TREE_TYPE (TREE_OPERAND (ref, 0));
+ ref = TREE_OPERAND (ref, 0);
+ }
+ else if (TREE_CODE (ref) == COMPONENT_REF
+ && TREE_CODE (TREE_TYPE (TREE_OPERAND (ref, 1))) == ARRAY_TYPE)
+ atype = TREE_TYPE (TREE_OPERAND (ref, 1));
+ else
return false;
while (handled_component_p (ref))
@@ -13246,19 +13262,42 @@ array_at_struct_end_p (tree ref, bool al
/* If the reference chain contains a component reference to a
non-union type and there follows another field the reference
is not at the end of a structure. */
- if (TREE_CODE (ref) == COMPONENT_REF
- && TREE_CODE (TREE_TYPE (TREE_OPERAND (ref, 0))) == RECORD_TYPE)
+ if (TREE_CODE (ref) == COMPONENT_REF)
{
- tree nextf = DECL_CHAIN (TREE_OPERAND (ref, 1));
- while (nextf && TREE_CODE (nextf) != FIELD_DECL)
- nextf = DECL_CHAIN (nextf);
- if (nextf)
- return false;
+ if (TREE_CODE (TREE_TYPE (TREE_OPERAND (ref, 0))) == RECORD_TYPE)
+ {
+ tree nextf = DECL_CHAIN (TREE_OPERAND (ref, 1));
+ while (nextf && TREE_CODE (nextf) != FIELD_DECL)
+ nextf = DECL_CHAIN (nextf);
+ if (nextf)
+ return false;
+ }
}
+ /* If we have a multi-dimensional array we do not consider
+ a non-innermost dimension as flex array if the whole
+ multi-dimensional array is at struct end.
+ Same for an array of aggregates with a trailing array
+ member. */
+ else if (TREE_CODE (ref) == ARRAY_REF)
+ return false;
+ else if (TREE_CODE (ref) == ARRAY_RANGE_REF)
+ ;
+ /* If we view an underlying object as sth else then what we
+ gathered up to now is what we have to rely on. */
+ else if (TREE_CODE (ref) == VIEW_CONVERT_EXPR)
+ break;
+ else
+ gcc_unreachable ();
ref = TREE_OPERAND (ref, 0);
}
+ /* The array now is at struct end. Treat flexible arrays as
+ always subject to extend, even into just padding constrained by
+ an underlying decl. */
+ if (! TYPE_SIZE (atype))
+ return true;
+
tree size = NULL;
if (TREE_CODE (ref) == MEM_REF
Index: gcc/tree.h
===================================================================
--- gcc/tree.h (revision 247542)
+++ gcc/tree.h (working copy)
@@ -4885,12 +4885,10 @@ extern tree array_ref_up_bound (tree);
EXP, an ARRAY_REF or an ARRAY_RANGE_REF. */
extern tree array_ref_low_bound (tree);
-/* Returns true if REF is an array reference to an array at the end of
- a structure. If this is the case, the array may be allocated larger
- than its upper bound implies. When second argument is true considers
- REF when it's a COMPONENT_REF in addition ARRAY_REF and
- ARRAY_RANGE_REF. */
-extern bool array_at_struct_end_p (tree, bool = false);
+/* Returns true if REF is an array reference or a component reference
+ to an array at the end of a structure. If this is the case, the array
+ may be allocated larger than its upper bound implies. */
+extern bool array_at_struct_end_p (tree);
/* Return a tree representing the offset, in bytes, of the field referenced
by EXP. This does not include any offset in DECL_FIELD_BIT_OFFSET. */
Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c (revision 247542)
+++ gcc/emit-rtl.c (working copy)
@@ -1957,7 +1957,7 @@ set_mem_attributes_minus_bitpos (rtx ref
|| (TREE_CODE (t2) == COMPONENT_REF
/* For trailing arrays t2 doesn't have a size that
covers all valid accesses. */
- && ! array_at_struct_end_p (t, false)))
+ && ! array_at_struct_end_p (t)))
{
attrs.expr = t2;
attrs.offset_known_p = false;
Index: gcc/gimple-fold.c
===================================================================
--- gcc/gimple-fold.c (revision 247542)
+++ gcc/gimple-fold.c (working copy)
@@ -1235,7 +1224,7 @@ get_range_strlen (tree arg, tree length[
the NUL.
Set *FLEXP to true if the array whose bound is being
used is at the end of a struct. */
- if (array_at_struct_end_p (arg, true))
+ if (array_at_struct_end_p (arg))
*flexp = true;
arg = TREE_OPERAND (arg, 1);
Index: gcc/tree-chkp.c
===================================================================
--- gcc/tree-chkp.c (revision 247542)
+++ gcc/tree-chkp.c (working copy)
@@ -3277,7 +3277,7 @@ chkp_may_narrow_to_field (tree ref, tree
return DECL_SIZE (field) && TREE_CODE (DECL_SIZE (field)) == INTEGER_CST
&& tree_to_uhwi (DECL_SIZE (field)) != 0
&& !(flag_chkp_flexible_struct_trailing_arrays
- && array_at_struct_end_p (ref, true))
+ && array_at_struct_end_p (ref))
&& (!DECL_FIELD_OFFSET (field)
|| TREE_CODE (DECL_FIELD_OFFSET (field)) == INTEGER_CST)
&& (!DECL_FIELD_BIT_OFFSET (field)
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] Tweak array_at_struct_end_p
2017-05-04 9:07 [PATCH] Tweak array_at_struct_end_p Richard Biener
@ 2017-05-05 15:19 ` Christophe Lyon
0 siblings, 0 replies; 2+ messages in thread
From: Christophe Lyon @ 2017-05-05 15:19 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
Hi,
On 4 May 2017 at 11:07, Richard Biener <rguenther@suse.de> wrote:
>
> The following picks the changes suggested as followup for PR80533
> that do not cause the warning regression on accessing a [0] array.
>
> Additionally the patch removes the unnecessary allow_compref of the
> function.
>
> The question whether we want to allow an array to extend into
> padding still stands. This patch allows it for C99 flex arrays
> (but not pre-C99 GNU extension [0] due to the above warning
> regression, also not for [1] or larger arrays we treat as flex arrays
> when we can't see an underlying decl).
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
>
> Richard.
>
> 2017-05-04 Richard Biener <rguenther@suse.de>
>
> * tree.c (array_at_struct_end_p): Handle arrays at struct
> end with flexarrays more conservatively. Refactor and treat
> arrays of arrays or aggregates more strict. Fix
> VIEW_CONVERT_EXPR handling. Remove allow_compref argument.
> * tree.c (array_at_struct_end_p): Adjust prototype.
> * emit-rtl.c (set_mem_attributes_minus_bitpos): Adjust.
> * gimple-fold.c (get_range_strlen): Likewise.
> * tree-chkp.c (chkp_may_narrow_to_field): Likewise.
>
Since this patch was committed (r247581), I've noticed regressions
on arm-none-linux-gnueabihf:
- PASS now FAIL [PASS => FAIL]:
Executed from: gfortran.dg/dg.exp
gfortran.dg/alloc_comp_auto_array_2.f90 -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions (test for
excess errors)
gfortran.dg/alloc_comp_auto_array_2.f90 -O3 -g (test for excess errors)
Christophe
> Index: gcc/tree.c
> ===================================================================
> --- gcc/tree.c (revision 247542)
> +++ gcc/tree.c (working copy)
> @@ -13227,18 +13235,26 @@ array_ref_up_bound (tree exp)
> return NULL_TREE;
> }
>
> -/* Returns true if REF is an array reference to an array at the end of
> - a structure. If this is the case, the array may be allocated larger
> - than its upper bound implies. When ALLOW_COMPREF is true considers
> - REF when it's a COMPONENT_REF in addition ARRAY_REF and
> - ARRAY_RANGE_REF. */
> +/* Returns true if REF is an array reference or a component reference
> + to an array at the end of a structure.
> + If this is the case, the array may be allocated larger
> + than its upper bound implies. */
>
> bool
> -array_at_struct_end_p (tree ref, bool allow_compref)
> +array_at_struct_end_p (tree ref)
> {
> - if (TREE_CODE (ref) != ARRAY_REF
> - && TREE_CODE (ref) != ARRAY_RANGE_REF
> - && (!allow_compref || TREE_CODE (ref) != COMPONENT_REF))
> + tree atype;
> +
> + if (TREE_CODE (ref) == ARRAY_REF
> + || TREE_CODE (ref) == ARRAY_RANGE_REF)
> + {
> + atype = TREE_TYPE (TREE_OPERAND (ref, 0));
> + ref = TREE_OPERAND (ref, 0);
> + }
> + else if (TREE_CODE (ref) == COMPONENT_REF
> + && TREE_CODE (TREE_TYPE (TREE_OPERAND (ref, 1))) == ARRAY_TYPE)
> + atype = TREE_TYPE (TREE_OPERAND (ref, 1));
> + else
> return false;
>
> while (handled_component_p (ref))
> @@ -13246,19 +13262,42 @@ array_at_struct_end_p (tree ref, bool al
> /* If the reference chain contains a component reference to a
> non-union type and there follows another field the reference
> is not at the end of a structure. */
> - if (TREE_CODE (ref) == COMPONENT_REF
> - && TREE_CODE (TREE_TYPE (TREE_OPERAND (ref, 0))) == RECORD_TYPE)
> + if (TREE_CODE (ref) == COMPONENT_REF)
> {
> - tree nextf = DECL_CHAIN (TREE_OPERAND (ref, 1));
> - while (nextf && TREE_CODE (nextf) != FIELD_DECL)
> - nextf = DECL_CHAIN (nextf);
> - if (nextf)
> - return false;
> + if (TREE_CODE (TREE_TYPE (TREE_OPERAND (ref, 0))) == RECORD_TYPE)
> + {
> + tree nextf = DECL_CHAIN (TREE_OPERAND (ref, 1));
> + while (nextf && TREE_CODE (nextf) != FIELD_DECL)
> + nextf = DECL_CHAIN (nextf);
> + if (nextf)
> + return false;
> + }
> }
> + /* If we have a multi-dimensional array we do not consider
> + a non-innermost dimension as flex array if the whole
> + multi-dimensional array is at struct end.
> + Same for an array of aggregates with a trailing array
> + member. */
> + else if (TREE_CODE (ref) == ARRAY_REF)
> + return false;
> + else if (TREE_CODE (ref) == ARRAY_RANGE_REF)
> + ;
> + /* If we view an underlying object as sth else then what we
> + gathered up to now is what we have to rely on. */
> + else if (TREE_CODE (ref) == VIEW_CONVERT_EXPR)
> + break;
> + else
> + gcc_unreachable ();
>
> ref = TREE_OPERAND (ref, 0);
> }
>
> + /* The array now is at struct end. Treat flexible arrays as
> + always subject to extend, even into just padding constrained by
> + an underlying decl. */
> + if (! TYPE_SIZE (atype))
> + return true;
> +
> tree size = NULL;
>
> if (TREE_CODE (ref) == MEM_REF
> Index: gcc/tree.h
> ===================================================================
> --- gcc/tree.h (revision 247542)
> +++ gcc/tree.h (working copy)
> @@ -4885,12 +4885,10 @@ extern tree array_ref_up_bound (tree);
> EXP, an ARRAY_REF or an ARRAY_RANGE_REF. */
> extern tree array_ref_low_bound (tree);
>
> -/* Returns true if REF is an array reference to an array at the end of
> - a structure. If this is the case, the array may be allocated larger
> - than its upper bound implies. When second argument is true considers
> - REF when it's a COMPONENT_REF in addition ARRAY_REF and
> - ARRAY_RANGE_REF. */
> -extern bool array_at_struct_end_p (tree, bool = false);
> +/* Returns true if REF is an array reference or a component reference
> + to an array at the end of a structure. If this is the case, the array
> + may be allocated larger than its upper bound implies. */
> +extern bool array_at_struct_end_p (tree);
>
> /* Return a tree representing the offset, in bytes, of the field referenced
> by EXP. This does not include any offset in DECL_FIELD_BIT_OFFSET. */
> Index: gcc/emit-rtl.c
> ===================================================================
> --- gcc/emit-rtl.c (revision 247542)
> +++ gcc/emit-rtl.c (working copy)
> @@ -1957,7 +1957,7 @@ set_mem_attributes_minus_bitpos (rtx ref
> || (TREE_CODE (t2) == COMPONENT_REF
> /* For trailing arrays t2 doesn't have a size that
> covers all valid accesses. */
> - && ! array_at_struct_end_p (t, false)))
> + && ! array_at_struct_end_p (t)))
> {
> attrs.expr = t2;
> attrs.offset_known_p = false;
> Index: gcc/gimple-fold.c
> ===================================================================
> --- gcc/gimple-fold.c (revision 247542)
> +++ gcc/gimple-fold.c (working copy)
> @@ -1235,7 +1224,7 @@ get_range_strlen (tree arg, tree length[
> the NUL.
> Set *FLEXP to true if the array whose bound is being
> used is at the end of a struct. */
> - if (array_at_struct_end_p (arg, true))
> + if (array_at_struct_end_p (arg))
> *flexp = true;
>
> arg = TREE_OPERAND (arg, 1);
> Index: gcc/tree-chkp.c
> ===================================================================
> --- gcc/tree-chkp.c (revision 247542)
> +++ gcc/tree-chkp.c (working copy)
> @@ -3277,7 +3277,7 @@ chkp_may_narrow_to_field (tree ref, tree
> return DECL_SIZE (field) && TREE_CODE (DECL_SIZE (field)) == INTEGER_CST
> && tree_to_uhwi (DECL_SIZE (field)) != 0
> && !(flag_chkp_flexible_struct_trailing_arrays
> - && array_at_struct_end_p (ref, true))
> + && array_at_struct_end_p (ref))
> && (!DECL_FIELD_OFFSET (field)
> || TREE_CODE (DECL_FIELD_OFFSET (field)) == INTEGER_CST)
> && (!DECL_FIELD_BIT_OFFSET (field)
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-05-05 15:13 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-04 9:07 [PATCH] Tweak array_at_struct_end_p Richard Biener
2017-05-05 15:19 ` Christophe Lyon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).