On 2023-01-26 11:20, Qing Zhao wrote:
Hi, Siddhesh,
Thanks a lot for this patch, after -fstrict-flex-array functionality has been
added into GCC,
I think that making the tree-object-size to have consistent behavior with
flex arrays is a
valuable and natural work that need to be added.
I also like the comments you added into tree-object-size.cc, making the code
much easier to be understood.
Minor comments below:
On Dec 21, 2022, at 5:25 PM, Siddhesh Poyarekar <siddh...@gotplt.org> wrote:
The tree object size pass tries to fail when it detects a flex array in
the struct, but it ends up doing the right thing only when the flex
array is in the outermost struct. For nested cases (such as arrays
nested in a union or an inner struct), it ends up taking whatever value
the flex array is declared with, using zero for the standard flex array,
i.e. [].
Rework subobject size computation to make it more consistent across the
board, honoring -fstrict-flex-arrays. With this change, any array at
the end of the struct will end up causing __bos to use the allocated
value of the outer object, bailing out in the maximum case when it can't
find it. In the minimum case, it will return the subscript value or the
allocated value of the outer object, whichever is larger.
I see from the changes in the testing case, there are the following major
changes for the existing behavior (can be show with the testing case)
****For non-nested structures:
struct A
{
char a[10];
int b;
char c[10];
};
1. The Minimum size of the reference to the subobject that is a trailing array of a
structure is changed from “0” to “sizeof the subobject"
- if (__builtin_object_size (&p->c, 3) != 0)
+ if (__builtin_object_size (&p->c, 3) != 10)
****For nested structures:
struct D
{
int i;
struct D1
{
char b;
char a[10];
} j;
};
2. The Maximum size of the reference to the subobject that is a trailing array of
the inner structure is changed from “sizeof the subobject” to “-1"
- if (__builtin_object_size (&d->j.a[3], 1) != sizeof (d->j.a) - 3)
+ if (__builtin_object_size (&d->j.a[3], 1) != (size_t) -1)
.
3. The Minimum size of the reference to the subobject that is a trailing array of
the inner structure is changed from “0” to “sizeof the subobject"
- if (__builtin_object_size ((char *) &e->j[0], 3) != 0)
+ if (__builtin_object_size ((char *) &e->j[0], 3) != sizeof (e->j))
All of the above is correct, thanks for the high level review!
I think that all the above changes are good. My only concern is, for the change
of the Minimum size of the reference to the subobject that is a trailing array
(the above case 1 and 3), will there be any negtive impact on the existing
application that use it?
I doubt it, because the 0 return value for minimum object size is
essentially a failure to determine minimum object size, i.e. a special
value. This change can be interpreted as succeeding to get the minimum
object size in more cases.
Likewise for the maximum object size change, the change can be
interpreted as failing to get the maximum object size in more cases.
Does that sound reasonable?
+ /* If the subobject size cannot be easily inferred or is smaller than
+ the whole size, just use the whole size. */
Should the above comment be:
+ /* If the subobject size cannot be easily inferred or is larger than
+ the whole size, just use the whole size. */
if (! TYPE_SIZE_UNIT (TREE_TYPE (var))
|| ! tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (var)))
|| (pt_var_size && TREE_CODE (pt_var_size) == INTEGER_CST
&& tree_int_cst_lt (pt_var_size,
TYPE_SIZE_UNIT (TREE_TYPE (var)))))
var = pt_var;
Oops, yes indeed, fixed in my copy.
Thanks,
Sid
