Pointer Bounds Checker and trailing arrays (PR68270)

Pointer Bounds Checker and trailing arrays (PR68270)

[Alexander Ivchenko]

Hi,

The patch below addresses PR68270. could you please take a look?

2016-11-25  Alexander Ivchenko  <aivchenk@gmail.com>

       * c-family/c.opt (flag_chkp_flexible_struct_trailing_arrays):
       Add new option.
       * tree-chkp.c (chkp_parse_array_and_component_ref): Forbid
       narrowing when chkp_parse_array_and_component_ref is used and
       the ARRAY_REF points to an array in the end of the struct.



diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 7d8a726..e45d6a2 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1166,6 +1166,11 @@ C ObjC C++ ObjC++ LTO RejectNegative Report
Var(flag_chkp_narrow_to_innermost_ar
 Forces Pointer Bounds Checker to use bounds of the innermost arrays in case of
 nested static arryas access.  By default outermost array is used.

+fchkp-flexible-struct-trailing-arrays
+C ObjC C++ ObjC++ LTO RejectNegative Report
Var(flag_chkp_flexible_struct_trailing_arrays)
+Allow Pointer Bounds Checker to treat all trailing arrays in structures as
+possibly flexible.
+
 fchkp-optimize
 C ObjC C++ ObjC++ LTO Report Var(flag_chkp_optimize) Init(-1)
 Allow Pointer Bounds Checker optimizations.  By default allowed
diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
index 2769682..40f99c3 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -3425,7 +3425,9 @@ chkp_parse_array_and_component_ref (tree node, tree *ptr,
   if (flag_chkp_narrow_bounds
       && !flag_chkp_narrow_to_innermost_arrray
       && (!last_comp
-  || chkp_may_narrow_to_field (TREE_OPERAND (last_comp, 1))))
+  || (chkp_may_narrow_to_field (TREE_OPERAND (last_comp, 1))
+      && !(flag_chkp_flexible_struct_trailing_arrays
+   && array_at_struct_end_p (var)))))
     {
       comp_to_narrow = last_comp;
       break;

Re: Pointer Bounds Checker and trailing arrays (PR68270)

[Ilya Enkovich]

2016-11-25 15:47 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
> Hi,
>
> The patch below addresses PR68270. could you please take a look?
>
> 2016-11-25  Alexander Ivchenko  <aivchenk@gmail.com>
>
>        * c-family/c.opt (flag_chkp_flexible_struct_trailing_arrays):
>        Add new option.
>        * tree-chkp.c (chkp_parse_array_and_component_ref): Forbid
>        narrowing when chkp_parse_array_and_component_ref is used and
>        the ARRAY_REF points to an array in the end of the struct.
>
>
>
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index 7d8a726..e45d6a2 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -1166,6 +1166,11 @@ C ObjC C++ ObjC++ LTO RejectNegative Report
> Var(flag_chkp_narrow_to_innermost_ar
>  Forces Pointer Bounds Checker to use bounds of the innermost arrays in case of
>  nested static arryas access.  By default outermost array is used.
>
> +fchkp-flexible-struct-trailing-arrays
> +C ObjC C++ ObjC++ LTO RejectNegative Report
> Var(flag_chkp_flexible_struct_trailing_arrays)
> +Allow Pointer Bounds Checker to treat all trailing arrays in structures as
> +possibly flexible.

Words 'allow' and 'possibly' are confusing here. This option is about to force
checker to do something, not to give him a choice.

New option has to be documented in invoke.texi. It would also be nice to reflect
changes on GCC MPX wiki page.

We have an attribute to change compiler behavior when this option is not set.
But we have no way to make exceptions when this option is used. Should we
add one?

> +
>  fchkp-optimize
>  C ObjC C++ ObjC++ LTO Report Var(flag_chkp_optimize) Init(-1)
>  Allow Pointer Bounds Checker optimizations.  By default allowed
> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
> index 2769682..40f99c3 100644
> --- a/gcc/tree-chkp.c
> +++ b/gcc/tree-chkp.c
> @@ -3425,7 +3425,9 @@ chkp_parse_array_and_component_ref (tree node, tree *ptr,
>    if (flag_chkp_narrow_bounds
>        && !flag_chkp_narrow_to_innermost_arrray
>        && (!last_comp
> -  || chkp_may_narrow_to_field (TREE_OPERAND (last_comp, 1))))
> +  || (chkp_may_narrow_to_field (TREE_OPERAND (last_comp, 1))
> +      && !(flag_chkp_flexible_struct_trailing_arrays
> +   && array_at_struct_end_p (var)))))

This is incorrect place for fix. Consider code

struct S {
  int a;
  int b[10];
};

struct S s;
int *p = s.b;

Here you need to compute bounds for p and you want your option to take effect
but in this case you won't event reach your new check because there is no
ARRAY_REF. And even if we change it to

int *p = s.b[5];

then it still would be narrowed because s.b would still be written
into 'comp_to_narrow'
variable. Correct place for fix is in chkp_may_narrow_to_field.

Also you should consider fchkp-narrow-to-innermost-arrray option. Should it
be more powerfull or not? I think fchkp-narrow-to-innermost-arrray shouldn't
narrow to variable sized fields. BTW looks like right now bnd_variable_size
attribute is ignored by fchkp-narrow-to-innermost-arrray. This is another
problem and may be fixed in another patch though.

Also patch lacks tests for various situations (with option and without, with
ARRAY_REF and without). In case of new attribute and fix for
fchkp-narrow-to-innermost-arrray behavior additional tests are required.

--
Ilya

>      {
>        comp_to_narrow = last_comp;
>        break;

Re: Pointer Bounds Checker and trailing arrays (PR68270)

[Alexander Ivchenko]

2016-11-26 0:28 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
> 2016-11-25 15:47 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
>> Hi,
>>
>> The patch below addresses PR68270. could you please take a look?
>>
>> 2016-11-25  Alexander Ivchenko  <aivchenk@gmail.com>
>>
>>        * c-family/c.opt (flag_chkp_flexible_struct_trailing_arrays):
>>        Add new option.
>>        * tree-chkp.c (chkp_parse_array_and_component_ref): Forbid
>>        narrowing when chkp_parse_array_and_component_ref is used and
>>        the ARRAY_REF points to an array in the end of the struct.
>>
>>
>>
>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>> index 7d8a726..e45d6a2 100644
>> --- a/gcc/c-family/c.opt
>> +++ b/gcc/c-family/c.opt
>> @@ -1166,6 +1166,11 @@ C ObjC C++ ObjC++ LTO RejectNegative Report
>> Var(flag_chkp_narrow_to_innermost_ar
>>  Forces Pointer Bounds Checker to use bounds of the innermost arrays in case of
>>  nested static arryas access.  By default outermost array is used.
>>
>> +fchkp-flexible-struct-trailing-arrays
>> +C ObjC C++ ObjC++ LTO RejectNegative Report
>> Var(flag_chkp_flexible_struct_trailing_arrays)
>> +Allow Pointer Bounds Checker to treat all trailing arrays in structures as
>> +possibly flexible.
>
> Words 'allow' and 'possibly' are confusing here. This option is about to force
> checker to do something, not to give him a choice.

Fixed

> New option has to be documented in invoke.texi. It would also be nice to reflect
> changes on GCC MPX wiki page.

Done
> We have an attribute to change compiler behavior when this option is not set.
> But we have no way to make exceptions when this option is used. Should we
> add one?
Something like "bnd_fixed_size" ? Could work. Although the customer
request did not mention the need for that.
Can I add it in a separate patch?


>> +
>>  fchkp-optimize
>>  C ObjC C++ ObjC++ LTO Report Var(flag_chkp_optimize) Init(-1)
>>  Allow Pointer Bounds Checker optimizations.  By default allowed
>> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
>> index 2769682..40f99c3 100644
>> --- a/gcc/tree-chkp.c
>> +++ b/gcc/tree-chkp.c
>> @@ -3425,7 +3425,9 @@ chkp_parse_array_and_component_ref (tree node, tree *ptr,
>>    if (flag_chkp_narrow_bounds
>>        && !flag_chkp_narrow_to_innermost_arrray
>>        && (!last_comp
>> -  || chkp_may_narrow_to_field (TREE_OPERAND (last_comp, 1))))
>> +  || (chkp_may_narrow_to_field (TREE_OPERAND (last_comp, 1))
>> +      && !(flag_chkp_flexible_struct_trailing_arrays
>> +   && array_at_struct_end_p (var)))))
>
> This is incorrect place for fix. Consider code
>
> struct S {
>   int a;
>   int b[10];
> };
>
> struct S s;
> int *p = s.b;
>
> Here you need to compute bounds for p and you want your option to take effect
> but in this case you won't event reach your new check because there is no
> ARRAY_REF. And even if we change it to
>
> int *p = s.b[5];
>
> then it still would be narrowed because s.b would still be written
> into 'comp_to_narrow'
> variable. Correct place for fix is in chkp_may_narrow_to_field.

Done

> Also you should consider fchkp-narrow-to-innermost-arrray option. Should it
> be more powerfull or not? I think fchkp-narrow-to-innermost-arrray shouldn't
> narrow to variable sized fields. BTW looks like right now bnd_variable_size
> attribute is ignored by fchkp-narrow-to-innermost-arrray. This is another
> problem and may be fixed in another patch though.
The way code works in chkp_parse_array_and_component_ref seems to be
exactly like you say:  fchkp-narrow-to-innermost-arrray won't narrow
to variable sized fields. I will create a separate bug for
bnd_variable_size+ fchkp-narrow-to-innermost-arrray.

> Also patch lacks tests for various situations (with option and without, with
> ARRAY_REF and without). In case of new attribute and fix for
> fchkp-narrow-to-innermost-arrray behavior additional tests are required.
I added three tests for flag_chkp_flexible_struct_trailing_arrays



The patch is below:

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 2d47d54..21ad6aa 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1188,7 +1188,13 @@ narrowing is on, field bounds are used.
Otherwise full object bounds are used.
 fchkp-narrow-to-innermost-array
 C ObjC C++ ObjC++ LTO RejectNegative Report
Var(flag_chkp_narrow_to_innermost_arrray)
 Forces Pointer Bounds Checker to use bounds of the innermost arrays in case of
-nested static arryas access.  By default outermost array is used.
+nested static arrays access.  By default outermost array is used.
+
+fchkp-flexible-struct-trailing-arrays
+C ObjC C++ ObjC++ LTO RejectNegative Report
Var(flag_chkp_flexible_struct_trailing_arrays)
+Forces Pointer Bounds Checker to treat all trailing arrays in structures as
+possibly flexible.  By default only arrays fields with zero length or that are
+marked with attribute bnd_variable_size are treated as flexible.

 fchkp-optimize
 C ObjC C++ ObjC++ LTO Report Var(flag_chkp_optimize) Init(-1)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 034ae98..2372c22a 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10886,6 +10886,13 @@ Forces Pointer Bounds Checker to use narrowed
bounds for the address of the
 first field in the structure.  By default a pointer to the first field has
 the same bounds as a pointer to the whole structure.

+@item -fchkp-flexible-struct-trailing-arrays
+@opindex fchkp-flexible-struct-trailing-arrays
+@opindex fno-chkp-flexible-struct-trailing-arrays
+Forces Pointer Bounds Checker to treat all trailing arrays in structures as
+possibly flexible.  By default only arrays fields with zero length or that are
+marked with attribute bnd_variable_size are treated as flexible.
+
 @item -fchkp-narrow-to-innermost-array
 @opindex fchkp-narrow-to-innermost-array
 @opindex fno-chkp-narrow-to-innermost-array
diff --git a/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-lbv.c
b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-lbv.c
new file mode 100644
index 0000000..9739920
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-lbv.c
@@ -0,0 +1,29 @@
+/* { dg-do run } */
+/* { dg-shouldfail "bounds violation" } */
+/* { dg-options "-fcheck-pointer-bounds -mmpx
-fchkp-flexible-struct-trailing-arrays" } */
+
+
+#define SHOULDFAIL
+
+#include "mpx-check.h"
+
+struct S
+{
+  int a;
+  int p[10];
+};
+
+int rd (int *p, int i)
+{
+  int res = p[i];
+  printf ("%d\n", res);
+  return res;
+}
+
+int mpx_test (int argc, const char **argv)
+{
+  struct S *s = (struct S *)alloca (sizeof(struct S) + sizeof (int)*100);
+  rd (s->p, -2);
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-nov.c
b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-nov.c
new file mode 100644
index 0000000..f5c8f95
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-nov.c
@@ -0,0 +1,29 @@
+/* { dg-do run } */
+/* { dg-options "-fcheck-pointer-bounds -mmpx
-fchkp-flexible-struct-trailing-arrays" } */
+
+
+#include "mpx-check.h"
+
+struct S
+{
+  int a;
+  int p[10];
+};
+
+int rd (int *p, int i)
+{
+  int res = p[i];
+  printf ("%d\n", res);
+  return res;
+}
+
+int mpx_test (int argc, const char **argv)
+{
+  struct S *s = (struct S *)alloca (sizeof(struct S) + sizeof (int)*100);
+  rd (s->p, 0);
+  rd (s->p, 99);
+  s->p[0];
+  s->p[99];
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-ubv.c
b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-ubv.c
new file mode 100644
index 0000000..8385a5a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-ubv.c
@@ -0,0 +1,29 @@
+/* { dg-do run } */
+/* { dg-shouldfail "bounds violation" } */
+/* { dg-options "-fcheck-pointer-bounds -mmpx
-fchkp-flexible-struct-trailing-arrays" } */
+
+
+#define SHOULDFAIL
+
+#include "mpx-check.h"
+
+struct S
+{
+  int a;
+  int p[10];
+};
+
+int rd (int *p, int i)
+{
+  int res = p[i];
+  printf ("%d\n", res);
+  return res;
+}
+
+int mpx_test (int argc, const char **argv)
+{
+  struct S *s = (struct S *)alloca (sizeof(struct S) + sizeof (int)*100);
+  rd (s->p, 110);
+
+  return 0;
+}
diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
index 2769682..6c5e541 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -3272,6 +3272,8 @@ chkp_may_narrow_to_field (tree field)
 {
   return DECL_SIZE (field) && TREE_CODE (DECL_SIZE (field)) == INTEGER_CST
     && tree_to_uhwi (DECL_SIZE (field)) != 0
+    && (!flag_chkp_flexible_struct_trailing_arrays
+ || DECL_CHAIN (field))
     && (!DECL_FIELD_OFFSET (field)
  || TREE_CODE (DECL_FIELD_OFFSET (field)) == INTEGER_CST)
     && (!DECL_FIELD_BIT_OFFSET (field)

Re: Pointer Bounds Checker and trailing arrays (PR68270)

[Ilya Enkovich]

2016-12-20 17:44 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
> 2016-11-26 0:28 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
>> 2016-11-25 15:47 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
>>> Hi,
>>>
>>> The patch below addresses PR68270. could you please take a look?
>>>
>>> 2016-11-25  Alexander Ivchenko  <aivchenk@gmail.com>
>>>
>>>        * c-family/c.opt (flag_chkp_flexible_struct_trailing_arrays):
>>>        Add new option.
>>>        * tree-chkp.c (chkp_parse_array_and_component_ref): Forbid
>>>        narrowing when chkp_parse_array_and_component_ref is used and
>>>        the ARRAY_REF points to an array in the end of the struct.
>>>
>>>
>>>
>>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>>> index 7d8a726..e45d6a2 100644
>>> --- a/gcc/c-family/c.opt
>>> +++ b/gcc/c-family/c.opt
>>> @@ -1166,6 +1166,11 @@ C ObjC C++ ObjC++ LTO RejectNegative Report
>>> Var(flag_chkp_narrow_to_innermost_ar
>>>  Forces Pointer Bounds Checker to use bounds of the innermost arrays in case of
>>>  nested static arryas access.  By default outermost array is used.
>>>
>>> +fchkp-flexible-struct-trailing-arrays
>>> +C ObjC C++ ObjC++ LTO RejectNegative Report
>>> Var(flag_chkp_flexible_struct_trailing_arrays)
>>> +Allow Pointer Bounds Checker to treat all trailing arrays in structures as
>>> +possibly flexible.
>>
>> Words 'allow' and 'possibly' are confusing here. This option is about to force
>> checker to do something, not to give him a choice.
>
> Fixed
>
>> New option has to be documented in invoke.texi. It would also be nice to reflect
>> changes on GCC MPX wiki page.
>
> Done
>> We have an attribute to change compiler behavior when this option is not set.
>> But we have no way to make exceptions when this option is used. Should we
>> add one?
> Something like "bnd_fixed_size" ? Could work. Although the customer
> request did not mention the need for that.
> Can I add it in a separate patch?
>

Yes.

>
>>> +
>>>  fchkp-optimize
>>>  C ObjC C++ ObjC++ LTO Report Var(flag_chkp_optimize) Init(-1)
>>>  Allow Pointer Bounds Checker optimizations.  By default allowed
>>> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
>>> index 2769682..40f99c3 100644
>>> --- a/gcc/tree-chkp.c
>>> +++ b/gcc/tree-chkp.c
>>> @@ -3425,7 +3425,9 @@ chkp_parse_array_and_component_ref (tree node, tree *ptr,
>>>    if (flag_chkp_narrow_bounds
>>>        && !flag_chkp_narrow_to_innermost_arrray
>>>        && (!last_comp
>>> -  || chkp_may_narrow_to_field (TREE_OPERAND (last_comp, 1))))
>>> +  || (chkp_may_narrow_to_field (TREE_OPERAND (last_comp, 1))
>>> +      && !(flag_chkp_flexible_struct_trailing_arrays
>>> +   && array_at_struct_end_p (var)))))
>>
>> This is incorrect place for fix. Consider code
>>
>> struct S {
>>   int a;
>>   int b[10];
>> };
>>
>> struct S s;
>> int *p = s.b;
>>
>> Here you need to compute bounds for p and you want your option to take effect
>> but in this case you won't event reach your new check because there is no
>> ARRAY_REF. And even if we change it to
>>
>> int *p = s.b[5];
>>
>> then it still would be narrowed because s.b would still be written
>> into 'comp_to_narrow'
>> variable. Correct place for fix is in chkp_may_narrow_to_field.
>
> Done
>
>> Also you should consider fchkp-narrow-to-innermost-arrray option. Should it
>> be more powerfull or not? I think fchkp-narrow-to-innermost-arrray shouldn't
>> narrow to variable sized fields. BTW looks like right now bnd_variable_size
>> attribute is ignored by fchkp-narrow-to-innermost-arrray. This is another
>> problem and may be fixed in another patch though.
> The way code works in chkp_parse_array_and_component_ref seems to be
> exactly like you say:  fchkp-narrow-to-innermost-arrray won't narrow
> to variable sized fields. I will create a separate bug for
> bnd_variable_size+ fchkp-narrow-to-innermost-arrray.
>
>> Also patch lacks tests for various situations (with option and without, with
>> ARRAY_REF and without). In case of new attribute and fix for
>> fchkp-narrow-to-innermost-arrray behavior additional tests are required.
> I added three tests for flag_chkp_flexible_struct_trailing_arrays
>
>
>
> The patch is below:
>
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index 2d47d54..21ad6aa 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -1188,7 +1188,13 @@ narrowing is on, field bounds are used.
> Otherwise full object bounds are used.
>  fchkp-narrow-to-innermost-array
>  C ObjC C++ ObjC++ LTO RejectNegative Report
> Var(flag_chkp_narrow_to_innermost_arrray)
>  Forces Pointer Bounds Checker to use bounds of the innermost arrays in case of
> -nested static arryas access.  By default outermost array is used.
> +nested static arrays access.  By default outermost array is used.
> +
> +fchkp-flexible-struct-trailing-arrays
> +C ObjC C++ ObjC++ LTO RejectNegative Report
> Var(flag_chkp_flexible_struct_trailing_arrays)
> +Forces Pointer Bounds Checker to treat all trailing arrays in structures as
> +possibly flexible.  By default only arrays fields with zero length or that are
> +marked with attribute bnd_variable_size are treated as flexible.
>
>  fchkp-optimize
>  C ObjC C++ ObjC++ LTO Report Var(flag_chkp_optimize) Init(-1)
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 034ae98..2372c22a 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -10886,6 +10886,13 @@ Forces Pointer Bounds Checker to use narrowed
> bounds for the address of the
>  first field in the structure.  By default a pointer to the first field has
>  the same bounds as a pointer to the whole structure.
>
> +@item -fchkp-flexible-struct-trailing-arrays
> +@opindex fchkp-flexible-struct-trailing-arrays
> +@opindex fno-chkp-flexible-struct-trailing-arrays
> +Forces Pointer Bounds Checker to treat all trailing arrays in structures as
> +possibly flexible.  By default only arrays fields with zero length or that are
> +marked with attribute bnd_variable_size are treated as flexible.
> +
>  @item -fchkp-narrow-to-innermost-array
>  @opindex fchkp-narrow-to-innermost-array
>  @opindex fno-chkp-narrow-to-innermost-array
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-lbv.c
> b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-lbv.c
> new file mode 100644
> index 0000000..9739920
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-lbv.c
> @@ -0,0 +1,29 @@
> +/* { dg-do run } */
> +/* { dg-shouldfail "bounds violation" } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx
> -fchkp-flexible-struct-trailing-arrays" } */
> +
> +
> +#define SHOULDFAIL
> +
> +#include "mpx-check.h"
> +
> +struct S
> +{
> +  int a;
> +  int p[10];
> +};
> +
> +int rd (int *p, int i)
> +{
> +  int res = p[i];
> +  printf ("%d\n", res);
> +  return res;
> +}
> +
> +int mpx_test (int argc, const char **argv)
> +{
> +  struct S *s = (struct S *)alloca (sizeof(struct S) + sizeof (int)*100);
> +  rd (s->p, -2);
> +
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-nov.c
> b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-nov.c
> new file mode 100644
> index 0000000..f5c8f95
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-nov.c
> @@ -0,0 +1,29 @@
> +/* { dg-do run } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx
> -fchkp-flexible-struct-trailing-arrays" } */
> +
> +
> +#include "mpx-check.h"
> +
> +struct S
> +{
> +  int a;
> +  int p[10];
> +};
> +
> +int rd (int *p, int i)
> +{
> +  int res = p[i];
> +  printf ("%d\n", res);
> +  return res;
> +}
> +
> +int mpx_test (int argc, const char **argv)
> +{
> +  struct S *s = (struct S *)alloca (sizeof(struct S) + sizeof (int)*100);
> +  rd (s->p, 0);
> +  rd (s->p, 99);
> +  s->p[0];
> +  s->p[99];
> +
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-ubv.c
> b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-ubv.c
> new file mode 100644
> index 0000000..8385a5a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-ubv.c
> @@ -0,0 +1,29 @@
> +/* { dg-do run } */
> +/* { dg-shouldfail "bounds violation" } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx
> -fchkp-flexible-struct-trailing-arrays" } */
> +
> +
> +#define SHOULDFAIL
> +
> +#include "mpx-check.h"
> +
> +struct S
> +{
> +  int a;
> +  int p[10];
> +};
> +
> +int rd (int *p, int i)
> +{
> +  int res = p[i];
> +  printf ("%d\n", res);
> +  return res;
> +}
> +
> +int mpx_test (int argc, const char **argv)
> +{
> +  struct S *s = (struct S *)alloca (sizeof(struct S) + sizeof (int)*100);
> +  rd (s->p, 110);
> +
> +  return 0;
> +}
> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
> index 2769682..6c5e541 100644
> --- a/gcc/tree-chkp.c
> +++ b/gcc/tree-chkp.c
> @@ -3272,6 +3272,8 @@ chkp_may_narrow_to_field (tree field)
>  {
>    return DECL_SIZE (field) && TREE_CODE (DECL_SIZE (field)) == INTEGER_CST
>      && tree_to_uhwi (DECL_SIZE (field)) != 0
> +    && (!flag_chkp_flexible_struct_trailing_arrays
> + || DECL_CHAIN (field))

What if it's not array?

Thanks,
Ilya

>      && (!DECL_FIELD_OFFSET (field)
>   || TREE_CODE (DECL_FIELD_OFFSET (field)) == INTEGER_CST)
>      && (!DECL_FIELD_BIT_OFFSET (field)

Re: Pointer Bounds Checker and trailing arrays (PR68270)

[Alexander Ivchenko]

Right.. here is this updated chunk (otherwise no difference in the patch)

diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
index 2769682..6c7862c 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -3272,6 +3272,9 @@ chkp_may_narrow_to_field (tree field)
 {
   return DECL_SIZE (field) && TREE_CODE (DECL_SIZE (field)) == INTEGER_CST
     && tree_to_uhwi (DECL_SIZE (field)) != 0
+    && !(flag_chkp_flexible_struct_trailing_arrays
+ && TREE_CODE(TREE_TYPE(field)) == ARRAY_TYPE
+ && !DECL_CHAIN (field))
     && (!DECL_FIELD_OFFSET (field)
  || TREE_CODE (DECL_FIELD_OFFSET (field)) == INTEGER_CST)
     && (!DECL_FIELD_BIT_OFFSET (field)

2016-12-21 21:00 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
> 2016-12-20 17:44 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
>> 2016-11-26 0:28 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
>>> 2016-11-25 15:47 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
>>>> Hi,
>>>>
>>>> The patch below addresses PR68270. could you please take a look?
>>>>
>>>> 2016-11-25  Alexander Ivchenko  <aivchenk@gmail.com>
>>>>
>>>>        * c-family/c.opt (flag_chkp_flexible_struct_trailing_arrays):
>>>>        Add new option.
>>>>        * tree-chkp.c (chkp_parse_array_and_component_ref): Forbid
>>>>        narrowing when chkp_parse_array_and_component_ref is used and
>>>>        the ARRAY_REF points to an array in the end of the struct.
>>>>
>>>>
>>>>
>>>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>>>> index 7d8a726..e45d6a2 100644
>>>> --- a/gcc/c-family/c.opt
>>>> +++ b/gcc/c-family/c.opt
>>>> @@ -1166,6 +1166,11 @@ C ObjC C++ ObjC++ LTO RejectNegative Report
>>>> Var(flag_chkp_narrow_to_innermost_ar
>>>>  Forces Pointer Bounds Checker to use bounds of the innermost arrays in case of
>>>>  nested static arryas access.  By default outermost array is used.
>>>>
>>>> +fchkp-flexible-struct-trailing-arrays
>>>> +C ObjC C++ ObjC++ LTO RejectNegative Report
>>>> Var(flag_chkp_flexible_struct_trailing_arrays)
>>>> +Allow Pointer Bounds Checker to treat all trailing arrays in structures as
>>>> +possibly flexible.
>>>
>>> Words 'allow' and 'possibly' are confusing here. This option is about to force
>>> checker to do something, not to give him a choice.
>>
>> Fixed
>>
>>> New option has to be documented in invoke.texi. It would also be nice to reflect
>>> changes on GCC MPX wiki page.
>>
>> Done
>>> We have an attribute to change compiler behavior when this option is not set.
>>> But we have no way to make exceptions when this option is used. Should we
>>> add one?
>> Something like "bnd_fixed_size" ? Could work. Although the customer
>> request did not mention the need for that.
>> Can I add it in a separate patch?
>>
>
> Yes.
>
>>
>>>> +
>>>>  fchkp-optimize
>>>>  C ObjC C++ ObjC++ LTO Report Var(flag_chkp_optimize) Init(-1)
>>>>  Allow Pointer Bounds Checker optimizations.  By default allowed
>>>> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
>>>> index 2769682..40f99c3 100644
>>>> --- a/gcc/tree-chkp.c
>>>> +++ b/gcc/tree-chkp.c
>>>> @@ -3425,7 +3425,9 @@ chkp_parse_array_and_component_ref (tree node, tree *ptr,
>>>>    if (flag_chkp_narrow_bounds
>>>>        && !flag_chkp_narrow_to_innermost_arrray
>>>>        && (!last_comp
>>>> -  || chkp_may_narrow_to_field (TREE_OPERAND (last_comp, 1))))
>>>> +  || (chkp_may_narrow_to_field (TREE_OPERAND (last_comp, 1))
>>>> +      && !(flag_chkp_flexible_struct_trailing_arrays
>>>> +   && array_at_struct_end_p (var)))))
>>>
>>> This is incorrect place for fix. Consider code
>>>
>>> struct S {
>>>   int a;
>>>   int b[10];
>>> };
>>>
>>> struct S s;
>>> int *p = s.b;
>>>
>>> Here you need to compute bounds for p and you want your option to take effect
>>> but in this case you won't event reach your new check because there is no
>>> ARRAY_REF. And even if we change it to
>>>
>>> int *p = s.b[5];
>>>
>>> then it still would be narrowed because s.b would still be written
>>> into 'comp_to_narrow'
>>> variable. Correct place for fix is in chkp_may_narrow_to_field.
>>
>> Done
>>
>>> Also you should consider fchkp-narrow-to-innermost-arrray option. Should it
>>> be more powerfull or not? I think fchkp-narrow-to-innermost-arrray shouldn't
>>> narrow to variable sized fields. BTW looks like right now bnd_variable_size
>>> attribute is ignored by fchkp-narrow-to-innermost-arrray. This is another
>>> problem and may be fixed in another patch though.
>> The way code works in chkp_parse_array_and_component_ref seems to be
>> exactly like you say:  fchkp-narrow-to-innermost-arrray won't narrow
>> to variable sized fields. I will create a separate bug for
>> bnd_variable_size+ fchkp-narrow-to-innermost-arrray.
>>
>>> Also patch lacks tests for various situations (with option and without, with
>>> ARRAY_REF and without). In case of new attribute and fix for
>>> fchkp-narrow-to-innermost-arrray behavior additional tests are required.
>> I added three tests for flag_chkp_flexible_struct_trailing_arrays
>>
>>
>>
>> The patch is below:
>>
>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>> index 2d47d54..21ad6aa 100644
>> --- a/gcc/c-family/c.opt
>> +++ b/gcc/c-family/c.opt
>> @@ -1188,7 +1188,13 @@ narrowing is on, field bounds are used.
>> Otherwise full object bounds are used.
>>  fchkp-narrow-to-innermost-array
>>  C ObjC C++ ObjC++ LTO RejectNegative Report
>> Var(flag_chkp_narrow_to_innermost_arrray)
>>  Forces Pointer Bounds Checker to use bounds of the innermost arrays in case of
>> -nested static arryas access.  By default outermost array is used.
>> +nested static arrays access.  By default outermost array is used.
>> +
>> +fchkp-flexible-struct-trailing-arrays
>> +C ObjC C++ ObjC++ LTO RejectNegative Report
>> Var(flag_chkp_flexible_struct_trailing_arrays)
>> +Forces Pointer Bounds Checker to treat all trailing arrays in structures as
>> +possibly flexible.  By default only arrays fields with zero length or that are
>> +marked with attribute bnd_variable_size are treated as flexible.
>>
>>  fchkp-optimize
>>  C ObjC C++ ObjC++ LTO Report Var(flag_chkp_optimize) Init(-1)
>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>> index 034ae98..2372c22a 100644
>> --- a/gcc/doc/invoke.texi
>> +++ b/gcc/doc/invoke.texi
>> @@ -10886,6 +10886,13 @@ Forces Pointer Bounds Checker to use narrowed
>> bounds for the address of the
>>  first field in the structure.  By default a pointer to the first field has
>>  the same bounds as a pointer to the whole structure.
>>
>> +@item -fchkp-flexible-struct-trailing-arrays
>> +@opindex fchkp-flexible-struct-trailing-arrays
>> +@opindex fno-chkp-flexible-struct-trailing-arrays
>> +Forces Pointer Bounds Checker to treat all trailing arrays in structures as
>> +possibly flexible.  By default only arrays fields with zero length or that are
>> +marked with attribute bnd_variable_size are treated as flexible.
>> +
>>  @item -fchkp-narrow-to-innermost-array
>>  @opindex fchkp-narrow-to-innermost-array
>>  @opindex fno-chkp-narrow-to-innermost-array
>> diff --git a/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-lbv.c
>> b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-lbv.c
>> new file mode 100644
>> index 0000000..9739920
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-lbv.c
>> @@ -0,0 +1,29 @@
>> +/* { dg-do run } */
>> +/* { dg-shouldfail "bounds violation" } */
>> +/* { dg-options "-fcheck-pointer-bounds -mmpx
>> -fchkp-flexible-struct-trailing-arrays" } */
>> +
>> +
>> +#define SHOULDFAIL
>> +
>> +#include "mpx-check.h"
>> +
>> +struct S
>> +{
>> +  int a;
>> +  int p[10];
>> +};
>> +
>> +int rd (int *p, int i)
>> +{
>> +  int res = p[i];
>> +  printf ("%d\n", res);
>> +  return res;
>> +}
>> +
>> +int mpx_test (int argc, const char **argv)
>> +{
>> +  struct S *s = (struct S *)alloca (sizeof(struct S) + sizeof (int)*100);
>> +  rd (s->p, -2);
>> +
>> +  return 0;
>> +}
>> diff --git a/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-nov.c
>> b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-nov.c
>> new file mode 100644
>> index 0000000..f5c8f95
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-nov.c
>> @@ -0,0 +1,29 @@
>> +/* { dg-do run } */
>> +/* { dg-options "-fcheck-pointer-bounds -mmpx
>> -fchkp-flexible-struct-trailing-arrays" } */
>> +
>> +
>> +#include "mpx-check.h"
>> +
>> +struct S
>> +{
>> +  int a;
>> +  int p[10];
>> +};
>> +
>> +int rd (int *p, int i)
>> +{
>> +  int res = p[i];
>> +  printf ("%d\n", res);
>> +  return res;
>> +}
>> +
>> +int mpx_test (int argc, const char **argv)
>> +{
>> +  struct S *s = (struct S *)alloca (sizeof(struct S) + sizeof (int)*100);
>> +  rd (s->p, 0);
>> +  rd (s->p, 99);
>> +  s->p[0];
>> +  s->p[99];
>> +
>> +  return 0;
>> +}
>> diff --git a/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-ubv.c
>> b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-ubv.c
>> new file mode 100644
>> index 0000000..8385a5a
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-ubv.c
>> @@ -0,0 +1,29 @@
>> +/* { dg-do run } */
>> +/* { dg-shouldfail "bounds violation" } */
>> +/* { dg-options "-fcheck-pointer-bounds -mmpx
>> -fchkp-flexible-struct-trailing-arrays" } */
>> +
>> +
>> +#define SHOULDFAIL
>> +
>> +#include "mpx-check.h"
>> +
>> +struct S
>> +{
>> +  int a;
>> +  int p[10];
>> +};
>> +
>> +int rd (int *p, int i)
>> +{
>> +  int res = p[i];
>> +  printf ("%d\n", res);
>> +  return res;
>> +}
>> +
>> +int mpx_test (int argc, const char **argv)
>> +{
>> +  struct S *s = (struct S *)alloca (sizeof(struct S) + sizeof (int)*100);
>> +  rd (s->p, 110);
>> +
>> +  return 0;
>> +}
>> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
>> index 2769682..6c5e541 100644
>> --- a/gcc/tree-chkp.c
>> +++ b/gcc/tree-chkp.c
>> @@ -3272,6 +3272,8 @@ chkp_may_narrow_to_field (tree field)
>>  {
>>    return DECL_SIZE (field) && TREE_CODE (DECL_SIZE (field)) == INTEGER_CST
>>      && tree_to_uhwi (DECL_SIZE (field)) != 0
>> +    && (!flag_chkp_flexible_struct_trailing_arrays
>> + || DECL_CHAIN (field))
>
> What if it's not array?
>
> Thanks,
> Ilya
>
>>      && (!DECL_FIELD_OFFSET (field)
>>   || TREE_CODE (DECL_FIELD_OFFSET (field)) == INTEGER_CST)
>>      && (!DECL_FIELD_BIT_OFFSET (field)

Re: Pointer Bounds Checker and trailing arrays (PR68270)

[Ilya Enkovich]

2016-12-21 22:18 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
> Right.. here is this updated chunk (otherwise no difference in the patch)
>
> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
> index 2769682..6c7862c 100644
> --- a/gcc/tree-chkp.c
> +++ b/gcc/tree-chkp.c
> @@ -3272,6 +3272,9 @@ chkp_may_narrow_to_field (tree field)
>  {
>    return DECL_SIZE (field) && TREE_CODE (DECL_SIZE (field)) == INTEGER_CST
>      && tree_to_uhwi (DECL_SIZE (field)) != 0
> +    && !(flag_chkp_flexible_struct_trailing_arrays
> + && TREE_CODE(TREE_TYPE(field)) == ARRAY_TYPE
> + && !DECL_CHAIN (field))
>      && (!DECL_FIELD_OFFSET (field)
>   || TREE_CODE (DECL_FIELD_OFFSET (field)) == INTEGER_CST)
>      && (!DECL_FIELD_BIT_OFFSET (field)

OK.

>
> 2016-12-21 21:00 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
>> 2016-12-20 17:44 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
>>> 2016-11-26 0:28 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
>>>> 2016-11-25 15:47 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
>>>>> Hi,
>>>>>
>>>>> The patch below addresses PR68270. could you please take a look?
>>>>>
>>>>> 2016-11-25  Alexander Ivchenko  <aivchenk@gmail.com>
>>>>>
>>>>>        * c-family/c.opt (flag_chkp_flexible_struct_trailing_arrays):
>>>>>        Add new option.
>>>>>        * tree-chkp.c (chkp_parse_array_and_component_ref): Forbid
>>>>>        narrowing when chkp_parse_array_and_component_ref is used and
>>>>>        the ARRAY_REF points to an array in the end of the struct.
>>>>>
>>>>>
>>>>>
>>>>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>>>>> index 7d8a726..e45d6a2 100644
>>>>> --- a/gcc/c-family/c.opt
>>>>> +++ b/gcc/c-family/c.opt
>>>>> @@ -1166,6 +1166,11 @@ C ObjC C++ ObjC++ LTO RejectNegative Report
>>>>> Var(flag_chkp_narrow_to_innermost_ar
>>>>>  Forces Pointer Bounds Checker to use bounds of the innermost arrays in case of
>>>>>  nested static arryas access.  By default outermost array is used.
>>>>>
>>>>> +fchkp-flexible-struct-trailing-arrays
>>>>> +C ObjC C++ ObjC++ LTO RejectNegative Report
>>>>> Var(flag_chkp_flexible_struct_trailing_arrays)
>>>>> +Allow Pointer Bounds Checker to treat all trailing arrays in structures as
>>>>> +possibly flexible.
>>>>
>>>> Words 'allow' and 'possibly' are confusing here. This option is about to force
>>>> checker to do something, not to give him a choice.
>>>
>>> Fixed
>>>
>>>> New option has to be documented in invoke.texi. It would also be nice to reflect
>>>> changes on GCC MPX wiki page.
>>>
>>> Done
>>>> We have an attribute to change compiler behavior when this option is not set.
>>>> But we have no way to make exceptions when this option is used. Should we
>>>> add one?
>>> Something like "bnd_fixed_size" ? Could work. Although the customer
>>> request did not mention the need for that.
>>> Can I add it in a separate patch?
>>>
>>
>> Yes.
>>
>>>
>>>>> +
>>>>>  fchkp-optimize
>>>>>  C ObjC C++ ObjC++ LTO Report Var(flag_chkp_optimize) Init(-1)
>>>>>  Allow Pointer Bounds Checker optimizations.  By default allowed
>>>>> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
>>>>> index 2769682..40f99c3 100644
>>>>> --- a/gcc/tree-chkp.c
>>>>> +++ b/gcc/tree-chkp.c
>>>>> @@ -3425,7 +3425,9 @@ chkp_parse_array_and_component_ref (tree node, tree *ptr,
>>>>>    if (flag_chkp_narrow_bounds
>>>>>        && !flag_chkp_narrow_to_innermost_arrray
>>>>>        && (!last_comp
>>>>> -  || chkp_may_narrow_to_field (TREE_OPERAND (last_comp, 1))))
>>>>> +  || (chkp_may_narrow_to_field (TREE_OPERAND (last_comp, 1))
>>>>> +      && !(flag_chkp_flexible_struct_trailing_arrays
>>>>> +   && array_at_struct_end_p (var)))))
>>>>
>>>> This is incorrect place for fix. Consider code
>>>>
>>>> struct S {
>>>>   int a;
>>>>   int b[10];
>>>> };
>>>>
>>>> struct S s;
>>>> int *p = s.b;
>>>>
>>>> Here you need to compute bounds for p and you want your option to take effect
>>>> but in this case you won't event reach your new check because there is no
>>>> ARRAY_REF. And even if we change it to
>>>>
>>>> int *p = s.b[5];
>>>>
>>>> then it still would be narrowed because s.b would still be written
>>>> into 'comp_to_narrow'
>>>> variable. Correct place for fix is in chkp_may_narrow_to_field.
>>>
>>> Done
>>>
>>>> Also you should consider fchkp-narrow-to-innermost-arrray option. Should it
>>>> be more powerfull or not? I think fchkp-narrow-to-innermost-arrray shouldn't
>>>> narrow to variable sized fields. BTW looks like right now bnd_variable_size
>>>> attribute is ignored by fchkp-narrow-to-innermost-arrray. This is another
>>>> problem and may be fixed in another patch though.
>>> The way code works in chkp_parse_array_and_component_ref seems to be
>>> exactly like you say:  fchkp-narrow-to-innermost-arrray won't narrow
>>> to variable sized fields. I will create a separate bug for
>>> bnd_variable_size+ fchkp-narrow-to-innermost-arrray.
>>>
>>>> Also patch lacks tests for various situations (with option and without, with
>>>> ARRAY_REF and without). In case of new attribute and fix for
>>>> fchkp-narrow-to-innermost-arrray behavior additional tests are required.
>>> I added three tests for flag_chkp_flexible_struct_trailing_arrays
>>>
>>>
>>>
>>> The patch is below:
>>>
>>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>>> index 2d47d54..21ad6aa 100644
>>> --- a/gcc/c-family/c.opt
>>> +++ b/gcc/c-family/c.opt
>>> @@ -1188,7 +1188,13 @@ narrowing is on, field bounds are used.
>>> Otherwise full object bounds are used.
>>>  fchkp-narrow-to-innermost-array
>>>  C ObjC C++ ObjC++ LTO RejectNegative Report
>>> Var(flag_chkp_narrow_to_innermost_arrray)
>>>  Forces Pointer Bounds Checker to use bounds of the innermost arrays in case of
>>> -nested static arryas access.  By default outermost array is used.
>>> +nested static arrays access.  By default outermost array is used.
>>> +
>>> +fchkp-flexible-struct-trailing-arrays
>>> +C ObjC C++ ObjC++ LTO RejectNegative Report

I also noticed this part conflicts with documentation which describes
both positive and negative flag versions. I think we should allow
negative version.

Patch is OK with that change and proper testing.

Thanks,
Ilya

>>> Var(flag_chkp_flexible_struct_trailing_arrays)
>>> +Forces Pointer Bounds Checker to treat all trailing arrays in structures as
>>> +possibly flexible.  By default only arrays fields with zero length or that are
>>> +marked with attribute bnd_variable_size are treated as flexible.
>>>
>>>  fchkp-optimize
>>>  C ObjC C++ ObjC++ LTO Report Var(flag_chkp_optimize) Init(-1)
>>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>>> index 034ae98..2372c22a 100644
>>> --- a/gcc/doc/invoke.texi
>>> +++ b/gcc/doc/invoke.texi
>>> @@ -10886,6 +10886,13 @@ Forces Pointer Bounds Checker to use narrowed
>>> bounds for the address of the
>>>  first field in the structure.  By default a pointer to the first field has
>>>  the same bounds as a pointer to the whole structure.
>>>
>>> +@item -fchkp-flexible-struct-trailing-arrays
>>> +@opindex fchkp-flexible-struct-trailing-arrays
>>> +@opindex fno-chkp-flexible-struct-trailing-arrays
>>> +Forces Pointer Bounds Checker to treat all trailing arrays in structures as
>>> +possibly flexible.  By default only arrays fields with zero length or that are
>>> +marked with attribute bnd_variable_size are treated as flexible.
>>> +
>>>  @item -fchkp-narrow-to-innermost-array
>>>  @opindex fchkp-narrow-to-innermost-array
>>>  @opindex fno-chkp-narrow-to-innermost-array
>>> diff --git a/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-lbv.c
>>> b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-lbv.c
>>> new file mode 100644
>>> index 0000000..9739920
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-lbv.c
>>> @@ -0,0 +1,29 @@
>>> +/* { dg-do run } */
>>> +/* { dg-shouldfail "bounds violation" } */
>>> +/* { dg-options "-fcheck-pointer-bounds -mmpx
>>> -fchkp-flexible-struct-trailing-arrays" } */
>>> +
>>> +
>>> +#define SHOULDFAIL
>>> +
>>> +#include "mpx-check.h"
>>> +
>>> +struct S
>>> +{
>>> +  int a;
>>> +  int p[10];
>>> +};
>>> +
>>> +int rd (int *p, int i)
>>> +{
>>> +  int res = p[i];
>>> +  printf ("%d\n", res);
>>> +  return res;
>>> +}
>>> +
>>> +int mpx_test (int argc, const char **argv)
>>> +{
>>> +  struct S *s = (struct S *)alloca (sizeof(struct S) + sizeof (int)*100);
>>> +  rd (s->p, -2);
>>> +
>>> +  return 0;
>>> +}
>>> diff --git a/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-nov.c
>>> b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-nov.c
>>> new file mode 100644
>>> index 0000000..f5c8f95
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-nov.c
>>> @@ -0,0 +1,29 @@
>>> +/* { dg-do run } */
>>> +/* { dg-options "-fcheck-pointer-bounds -mmpx
>>> -fchkp-flexible-struct-trailing-arrays" } */
>>> +
>>> +
>>> +#include "mpx-check.h"
>>> +
>>> +struct S
>>> +{
>>> +  int a;
>>> +  int p[10];
>>> +};
>>> +
>>> +int rd (int *p, int i)
>>> +{
>>> +  int res = p[i];
>>> +  printf ("%d\n", res);
>>> +  return res;
>>> +}
>>> +
>>> +int mpx_test (int argc, const char **argv)
>>> +{
>>> +  struct S *s = (struct S *)alloca (sizeof(struct S) + sizeof (int)*100);
>>> +  rd (s->p, 0);
>>> +  rd (s->p, 99);
>>> +  s->p[0];
>>> +  s->p[99];
>>> +
>>> +  return 0;
>>> +}
>>> diff --git a/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-ubv.c
>>> b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-ubv.c
>>> new file mode 100644
>>> index 0000000..8385a5a
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-ubv.c
>>> @@ -0,0 +1,29 @@
>>> +/* { dg-do run } */
>>> +/* { dg-shouldfail "bounds violation" } */
>>> +/* { dg-options "-fcheck-pointer-bounds -mmpx
>>> -fchkp-flexible-struct-trailing-arrays" } */
>>> +
>>> +
>>> +#define SHOULDFAIL
>>> +
>>> +#include "mpx-check.h"
>>> +
>>> +struct S
>>> +{
>>> +  int a;
>>> +  int p[10];
>>> +};
>>> +
>>> +int rd (int *p, int i)
>>> +{
>>> +  int res = p[i];
>>> +  printf ("%d\n", res);
>>> +  return res;
>>> +}
>>> +
>>> +int mpx_test (int argc, const char **argv)
>>> +{
>>> +  struct S *s = (struct S *)alloca (sizeof(struct S) + sizeof (int)*100);
>>> +  rd (s->p, 110);
>>> +
>>> +  return 0;
>>> +}
>>> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
>>> index 2769682..6c5e541 100644
>>> --- a/gcc/tree-chkp.c
>>> +++ b/gcc/tree-chkp.c
>>> @@ -3272,6 +3272,8 @@ chkp_may_narrow_to_field (tree field)
>>>  {
>>>    return DECL_SIZE (field) && TREE_CODE (DECL_SIZE (field)) == INTEGER_CST
>>>      && tree_to_uhwi (DECL_SIZE (field)) != 0
>>> +    && (!flag_chkp_flexible_struct_trailing_arrays
>>> + || DECL_CHAIN (field))
>>
>> What if it's not array?
>>
>> Thanks,
>> Ilya
>>
>>>      && (!DECL_FIELD_OFFSET (field)
>>>   || TREE_CODE (DECL_FIELD_OFFSET (field)) == INTEGER_CST)
>>>      && (!DECL_FIELD_BIT_OFFSET (field)

Re: Pointer Bounds Checker and trailing arrays (PR68270)

[Alexander Ivchenko]

Committed as r243936.

Thank you,
Alexander

2016-12-22 2:47 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
> 2016-12-21 22:18 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
>> Right.. here is this updated chunk (otherwise no difference in the patch)
>>
>> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
>> index 2769682..6c7862c 100644
>> --- a/gcc/tree-chkp.c
>> +++ b/gcc/tree-chkp.c
>> @@ -3272,6 +3272,9 @@ chkp_may_narrow_to_field (tree field)
>>  {
>>    return DECL_SIZE (field) && TREE_CODE (DECL_SIZE (field)) == INTEGER_CST
>>      && tree_to_uhwi (DECL_SIZE (field)) != 0
>> +    && !(flag_chkp_flexible_struct_trailing_arrays
>> + && TREE_CODE(TREE_TYPE(field)) == ARRAY_TYPE
>> + && !DECL_CHAIN (field))
>>      && (!DECL_FIELD_OFFSET (field)
>>   || TREE_CODE (DECL_FIELD_OFFSET (field)) == INTEGER_CST)
>>      && (!DECL_FIELD_BIT_OFFSET (field)
>
> OK.
>
>>
>> 2016-12-21 21:00 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
>>> 2016-12-20 17:44 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
>>>> 2016-11-26 0:28 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
>>>>> 2016-11-25 15:47 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
>>>>>> Hi,
>>>>>>
>>>>>> The patch below addresses PR68270. could you please take a look?
>>>>>>
>>>>>> 2016-11-25  Alexander Ivchenko  <aivchenk@gmail.com>
>>>>>>
>>>>>>        * c-family/c.opt (flag_chkp_flexible_struct_trailing_arrays):
>>>>>>        Add new option.
>>>>>>        * tree-chkp.c (chkp_parse_array_and_component_ref): Forbid
>>>>>>        narrowing when chkp_parse_array_and_component_ref is used and
>>>>>>        the ARRAY_REF points to an array in the end of the struct.
>>>>>>
>>>>>>
>>>>>>
>>>>>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>>>>>> index 7d8a726..e45d6a2 100644
>>>>>> --- a/gcc/c-family/c.opt
>>>>>> +++ b/gcc/c-family/c.opt
>>>>>> @@ -1166,6 +1166,11 @@ C ObjC C++ ObjC++ LTO RejectNegative Report
>>>>>> Var(flag_chkp_narrow_to_innermost_ar
>>>>>>  Forces Pointer Bounds Checker to use bounds of the innermost arrays in case of
>>>>>>  nested static arryas access.  By default outermost array is used.
>>>>>>
>>>>>> +fchkp-flexible-struct-trailing-arrays
>>>>>> +C ObjC C++ ObjC++ LTO RejectNegative Report
>>>>>> Var(flag_chkp_flexible_struct_trailing_arrays)
>>>>>> +Allow Pointer Bounds Checker to treat all trailing arrays in structures as
>>>>>> +possibly flexible.
>>>>>
>>>>> Words 'allow' and 'possibly' are confusing here. This option is about to force
>>>>> checker to do something, not to give him a choice.
>>>>
>>>> Fixed
>>>>
>>>>> New option has to be documented in invoke.texi. It would also be nice to reflect
>>>>> changes on GCC MPX wiki page.
>>>>
>>>> Done
>>>>> We have an attribute to change compiler behavior when this option is not set.
>>>>> But we have no way to make exceptions when this option is used. Should we
>>>>> add one?
>>>> Something like "bnd_fixed_size" ? Could work. Although the customer
>>>> request did not mention the need for that.
>>>> Can I add it in a separate patch?
>>>>
>>>
>>> Yes.
>>>
>>>>
>>>>>> +
>>>>>>  fchkp-optimize
>>>>>>  C ObjC C++ ObjC++ LTO Report Var(flag_chkp_optimize) Init(-1)
>>>>>>  Allow Pointer Bounds Checker optimizations.  By default allowed
>>>>>> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
>>>>>> index 2769682..40f99c3 100644
>>>>>> --- a/gcc/tree-chkp.c
>>>>>> +++ b/gcc/tree-chkp.c
>>>>>> @@ -3425,7 +3425,9 @@ chkp_parse_array_and_component_ref (tree node, tree *ptr,
>>>>>>    if (flag_chkp_narrow_bounds
>>>>>>        && !flag_chkp_narrow_to_innermost_arrray
>>>>>>        && (!last_comp
>>>>>> -  || chkp_may_narrow_to_field (TREE_OPERAND (last_comp, 1))))
>>>>>> +  || (chkp_may_narrow_to_field (TREE_OPERAND (last_comp, 1))
>>>>>> +      && !(flag_chkp_flexible_struct_trailing_arrays
>>>>>> +   && array_at_struct_end_p (var)))))
>>>>>
>>>>> This is incorrect place for fix. Consider code
>>>>>
>>>>> struct S {
>>>>>   int a;
>>>>>   int b[10];
>>>>> };
>>>>>
>>>>> struct S s;
>>>>> int *p = s.b;
>>>>>
>>>>> Here you need to compute bounds for p and you want your option to take effect
>>>>> but in this case you won't event reach your new check because there is no
>>>>> ARRAY_REF. And even if we change it to
>>>>>
>>>>> int *p = s.b[5];
>>>>>
>>>>> then it still would be narrowed because s.b would still be written
>>>>> into 'comp_to_narrow'
>>>>> variable. Correct place for fix is in chkp_may_narrow_to_field.
>>>>
>>>> Done
>>>>
>>>>> Also you should consider fchkp-narrow-to-innermost-arrray option. Should it
>>>>> be more powerfull or not? I think fchkp-narrow-to-innermost-arrray shouldn't
>>>>> narrow to variable sized fields. BTW looks like right now bnd_variable_size
>>>>> attribute is ignored by fchkp-narrow-to-innermost-arrray. This is another
>>>>> problem and may be fixed in another patch though.
>>>> The way code works in chkp_parse_array_and_component_ref seems to be
>>>> exactly like you say:  fchkp-narrow-to-innermost-arrray won't narrow
>>>> to variable sized fields. I will create a separate bug for
>>>> bnd_variable_size+ fchkp-narrow-to-innermost-arrray.
>>>>
>>>>> Also patch lacks tests for various situations (with option and without, with
>>>>> ARRAY_REF and without). In case of new attribute and fix for
>>>>> fchkp-narrow-to-innermost-arrray behavior additional tests are required.
>>>> I added three tests for flag_chkp_flexible_struct_trailing_arrays
>>>>
>>>>
>>>>
>>>> The patch is below:
>>>>
>>>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>>>> index 2d47d54..21ad6aa 100644
>>>> --- a/gcc/c-family/c.opt
>>>> +++ b/gcc/c-family/c.opt
>>>> @@ -1188,7 +1188,13 @@ narrowing is on, field bounds are used.
>>>> Otherwise full object bounds are used.
>>>>  fchkp-narrow-to-innermost-array
>>>>  C ObjC C++ ObjC++ LTO RejectNegative Report
>>>> Var(flag_chkp_narrow_to_innermost_arrray)
>>>>  Forces Pointer Bounds Checker to use bounds of the innermost arrays in case of
>>>> -nested static arryas access.  By default outermost array is used.
>>>> +nested static arrays access.  By default outermost array is used.
>>>> +
>>>> +fchkp-flexible-struct-trailing-arrays
>>>> +C ObjC C++ ObjC++ LTO RejectNegative Report
>
> I also noticed this part conflicts with documentation which describes
> both positive and negative flag versions. I think we should allow
> negative version.
>
> Patch is OK with that change and proper testing.
>
> Thanks,
> Ilya
>
>>>> Var(flag_chkp_flexible_struct_trailing_arrays)
>>>> +Forces Pointer Bounds Checker to treat all trailing arrays in structures as
>>>> +possibly flexible.  By default only arrays fields with zero length or that are
>>>> +marked with attribute bnd_variable_size are treated as flexible.
>>>>
>>>>  fchkp-optimize
>>>>  C ObjC C++ ObjC++ LTO Report Var(flag_chkp_optimize) Init(-1)
>>>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>>>> index 034ae98..2372c22a 100644
>>>> --- a/gcc/doc/invoke.texi
>>>> +++ b/gcc/doc/invoke.texi
>>>> @@ -10886,6 +10886,13 @@ Forces Pointer Bounds Checker to use narrowed
>>>> bounds for the address of the
>>>>  first field in the structure.  By default a pointer to the first field has
>>>>  the same bounds as a pointer to the whole structure.
>>>>
>>>> +@item -fchkp-flexible-struct-trailing-arrays
>>>> +@opindex fchkp-flexible-struct-trailing-arrays
>>>> +@opindex fno-chkp-flexible-struct-trailing-arrays
>>>> +Forces Pointer Bounds Checker to treat all trailing arrays in structures as
>>>> +possibly flexible.  By default only arrays fields with zero length or that are
>>>> +marked with attribute bnd_variable_size are treated as flexible.
>>>> +
>>>>  @item -fchkp-narrow-to-innermost-array
>>>>  @opindex fchkp-narrow-to-innermost-array
>>>>  @opindex fno-chkp-narrow-to-innermost-array
>>>> diff --git a/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-lbv.c
>>>> b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-lbv.c
>>>> new file mode 100644
>>>> index 0000000..9739920
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-lbv.c
>>>> @@ -0,0 +1,29 @@
>>>> +/* { dg-do run } */
>>>> +/* { dg-shouldfail "bounds violation" } */
>>>> +/* { dg-options "-fcheck-pointer-bounds -mmpx
>>>> -fchkp-flexible-struct-trailing-arrays" } */
>>>> +
>>>> +
>>>> +#define SHOULDFAIL
>>>> +
>>>> +#include "mpx-check.h"
>>>> +
>>>> +struct S
>>>> +{
>>>> +  int a;
>>>> +  int p[10];
>>>> +};
>>>> +
>>>> +int rd (int *p, int i)
>>>> +{
>>>> +  int res = p[i];
>>>> +  printf ("%d\n", res);
>>>> +  return res;
>>>> +}
>>>> +
>>>> +int mpx_test (int argc, const char **argv)
>>>> +{
>>>> +  struct S *s = (struct S *)alloca (sizeof(struct S) + sizeof (int)*100);
>>>> +  rd (s->p, -2);
>>>> +
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-nov.c
>>>> b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-nov.c
>>>> new file mode 100644
>>>> index 0000000..f5c8f95
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-nov.c
>>>> @@ -0,0 +1,29 @@
>>>> +/* { dg-do run } */
>>>> +/* { dg-options "-fcheck-pointer-bounds -mmpx
>>>> -fchkp-flexible-struct-trailing-arrays" } */
>>>> +
>>>> +
>>>> +#include "mpx-check.h"
>>>> +
>>>> +struct S
>>>> +{
>>>> +  int a;
>>>> +  int p[10];
>>>> +};
>>>> +
>>>> +int rd (int *p, int i)
>>>> +{
>>>> +  int res = p[i];
>>>> +  printf ("%d\n", res);
>>>> +  return res;
>>>> +}
>>>> +
>>>> +int mpx_test (int argc, const char **argv)
>>>> +{
>>>> +  struct S *s = (struct S *)alloca (sizeof(struct S) + sizeof (int)*100);
>>>> +  rd (s->p, 0);
>>>> +  rd (s->p, 99);
>>>> +  s->p[0];
>>>> +  s->p[99];
>>>> +
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-ubv.c
>>>> b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-ubv.c
>>>> new file mode 100644
>>>> index 0000000..8385a5a
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-ubv.c
>>>> @@ -0,0 +1,29 @@
>>>> +/* { dg-do run } */
>>>> +/* { dg-shouldfail "bounds violation" } */
>>>> +/* { dg-options "-fcheck-pointer-bounds -mmpx
>>>> -fchkp-flexible-struct-trailing-arrays" } */
>>>> +
>>>> +
>>>> +#define SHOULDFAIL
>>>> +
>>>> +#include "mpx-check.h"
>>>> +
>>>> +struct S
>>>> +{
>>>> +  int a;
>>>> +  int p[10];
>>>> +};
>>>> +
>>>> +int rd (int *p, int i)
>>>> +{
>>>> +  int res = p[i];
>>>> +  printf ("%d\n", res);
>>>> +  return res;
>>>> +}
>>>> +
>>>> +int mpx_test (int argc, const char **argv)
>>>> +{
>>>> +  struct S *s = (struct S *)alloca (sizeof(struct S) + sizeof (int)*100);
>>>> +  rd (s->p, 110);
>>>> +
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
>>>> index 2769682..6c5e541 100644
>>>> --- a/gcc/tree-chkp.c
>>>> +++ b/gcc/tree-chkp.c
>>>> @@ -3272,6 +3272,8 @@ chkp_may_narrow_to_field (tree field)
>>>>  {
>>>>    return DECL_SIZE (field) && TREE_CODE (DECL_SIZE (field)) == INTEGER_CST
>>>>      && tree_to_uhwi (DECL_SIZE (field)) != 0
>>>> +    && (!flag_chkp_flexible_struct_trailing_arrays
>>>> + || DECL_CHAIN (field))
>>>
>>> What if it's not array?
>>>
>>> Thanks,
>>> Ilya
>>>
>>>>      && (!DECL_FIELD_OFFSET (field)
>>>>   || TREE_CODE (DECL_FIELD_OFFSET (field)) == INTEGER_CST)
>>>>      && (!DECL_FIELD_BIT_OFFSET (field)

Re: Pointer Bounds Checker and trailing arrays (PR68270)

[Richard Biener]

On Thu, Dec 22, 2016 at 12:47 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2016-12-21 22:18 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
>> Right.. here is this updated chunk (otherwise no difference in the patch)
>>
>> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
>> index 2769682..6c7862c 100644
>> --- a/gcc/tree-chkp.c
>> +++ b/gcc/tree-chkp.c
>> @@ -3272,6 +3272,9 @@ chkp_may_narrow_to_field (tree field)
>>  {
>>    return DECL_SIZE (field) && TREE_CODE (DECL_SIZE (field)) == INTEGER_CST
>>      && tree_to_uhwi (DECL_SIZE (field)) != 0
>> +    && !(flag_chkp_flexible_struct_trailing_arrays
>> + && TREE_CODE(TREE_TYPE(field)) == ARRAY_TYPE
>> + && !DECL_CHAIN (field))
>>      && (!DECL_FIELD_OFFSET (field)
>>   || TREE_CODE (DECL_FIELD_OFFSET (field)) == INTEGER_CST)
>>      && (!DECL_FIELD_BIT_OFFSET (field)
>
> OK.

There is array_at_struct_end_p which has a better implementation for
this (the above
considers a TYPE_DECL after the array a field).

Richard.

>>
>> 2016-12-21 21:00 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
>>> 2016-12-20 17:44 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
>>>> 2016-11-26 0:28 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
>>>>> 2016-11-25 15:47 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
>>>>>> Hi,
>>>>>>
>>>>>> The patch below addresses PR68270. could you please take a look?
>>>>>>
>>>>>> 2016-11-25  Alexander Ivchenko  <aivchenk@gmail.com>
>>>>>>
>>>>>>        * c-family/c.opt (flag_chkp_flexible_struct_trailing_arrays):
>>>>>>        Add new option.
>>>>>>        * tree-chkp.c (chkp_parse_array_and_component_ref): Forbid
>>>>>>        narrowing when chkp_parse_array_and_component_ref is used and
>>>>>>        the ARRAY_REF points to an array in the end of the struct.
>>>>>>
>>>>>>
>>>>>>
>>>>>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>>>>>> index 7d8a726..e45d6a2 100644
>>>>>> --- a/gcc/c-family/c.opt
>>>>>> +++ b/gcc/c-family/c.opt
>>>>>> @@ -1166,6 +1166,11 @@ C ObjC C++ ObjC++ LTO RejectNegative Report
>>>>>> Var(flag_chkp_narrow_to_innermost_ar
>>>>>>  Forces Pointer Bounds Checker to use bounds of the innermost arrays in case of
>>>>>>  nested static arryas access.  By default outermost array is used.
>>>>>>
>>>>>> +fchkp-flexible-struct-trailing-arrays
>>>>>> +C ObjC C++ ObjC++ LTO RejectNegative Report
>>>>>> Var(flag_chkp_flexible_struct_trailing_arrays)
>>>>>> +Allow Pointer Bounds Checker to treat all trailing arrays in structures as
>>>>>> +possibly flexible.
>>>>>
>>>>> Words 'allow' and 'possibly' are confusing here. This option is about to force
>>>>> checker to do something, not to give him a choice.
>>>>
>>>> Fixed
>>>>
>>>>> New option has to be documented in invoke.texi. It would also be nice to reflect
>>>>> changes on GCC MPX wiki page.
>>>>
>>>> Done
>>>>> We have an attribute to change compiler behavior when this option is not set.
>>>>> But we have no way to make exceptions when this option is used. Should we
>>>>> add one?
>>>> Something like "bnd_fixed_size" ? Could work. Although the customer
>>>> request did not mention the need for that.
>>>> Can I add it in a separate patch?
>>>>
>>>
>>> Yes.
>>>
>>>>
>>>>>> +
>>>>>>  fchkp-optimize
>>>>>>  C ObjC C++ ObjC++ LTO Report Var(flag_chkp_optimize) Init(-1)
>>>>>>  Allow Pointer Bounds Checker optimizations.  By default allowed
>>>>>> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
>>>>>> index 2769682..40f99c3 100644
>>>>>> --- a/gcc/tree-chkp.c
>>>>>> +++ b/gcc/tree-chkp.c
>>>>>> @@ -3425,7 +3425,9 @@ chkp_parse_array_and_component_ref (tree node, tree *ptr,
>>>>>>    if (flag_chkp_narrow_bounds
>>>>>>        && !flag_chkp_narrow_to_innermost_arrray
>>>>>>        && (!last_comp
>>>>>> -  || chkp_may_narrow_to_field (TREE_OPERAND (last_comp, 1))))
>>>>>> +  || (chkp_may_narrow_to_field (TREE_OPERAND (last_comp, 1))
>>>>>> +      && !(flag_chkp_flexible_struct_trailing_arrays
>>>>>> +   && array_at_struct_end_p (var)))))
>>>>>
>>>>> This is incorrect place for fix. Consider code
>>>>>
>>>>> struct S {
>>>>>   int a;
>>>>>   int b[10];
>>>>> };
>>>>>
>>>>> struct S s;
>>>>> int *p = s.b;
>>>>>
>>>>> Here you need to compute bounds for p and you want your option to take effect
>>>>> but in this case you won't event reach your new check because there is no
>>>>> ARRAY_REF. And even if we change it to
>>>>>
>>>>> int *p = s.b[5];
>>>>>
>>>>> then it still would be narrowed because s.b would still be written
>>>>> into 'comp_to_narrow'
>>>>> variable. Correct place for fix is in chkp_may_narrow_to_field.
>>>>
>>>> Done
>>>>
>>>>> Also you should consider fchkp-narrow-to-innermost-arrray option. Should it
>>>>> be more powerfull or not? I think fchkp-narrow-to-innermost-arrray shouldn't
>>>>> narrow to variable sized fields. BTW looks like right now bnd_variable_size
>>>>> attribute is ignored by fchkp-narrow-to-innermost-arrray. This is another
>>>>> problem and may be fixed in another patch though.
>>>> The way code works in chkp_parse_array_and_component_ref seems to be
>>>> exactly like you say:  fchkp-narrow-to-innermost-arrray won't narrow
>>>> to variable sized fields. I will create a separate bug for
>>>> bnd_variable_size+ fchkp-narrow-to-innermost-arrray.
>>>>
>>>>> Also patch lacks tests for various situations (with option and without, with
>>>>> ARRAY_REF and without). In case of new attribute and fix for
>>>>> fchkp-narrow-to-innermost-arrray behavior additional tests are required.
>>>> I added three tests for flag_chkp_flexible_struct_trailing_arrays
>>>>
>>>>
>>>>
>>>> The patch is below:
>>>>
>>>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>>>> index 2d47d54..21ad6aa 100644
>>>> --- a/gcc/c-family/c.opt
>>>> +++ b/gcc/c-family/c.opt
>>>> @@ -1188,7 +1188,13 @@ narrowing is on, field bounds are used.
>>>> Otherwise full object bounds are used.
>>>>  fchkp-narrow-to-innermost-array
>>>>  C ObjC C++ ObjC++ LTO RejectNegative Report
>>>> Var(flag_chkp_narrow_to_innermost_arrray)
>>>>  Forces Pointer Bounds Checker to use bounds of the innermost arrays in case of
>>>> -nested static arryas access.  By default outermost array is used.
>>>> +nested static arrays access.  By default outermost array is used.
>>>> +
>>>> +fchkp-flexible-struct-trailing-arrays
>>>> +C ObjC C++ ObjC++ LTO RejectNegative Report
>
> I also noticed this part conflicts with documentation which describes
> both positive and negative flag versions. I think we should allow
> negative version.
>
> Patch is OK with that change and proper testing.
>
> Thanks,
> Ilya
>
>>>> Var(flag_chkp_flexible_struct_trailing_arrays)
>>>> +Forces Pointer Bounds Checker to treat all trailing arrays in structures as
>>>> +possibly flexible.  By default only arrays fields with zero length or that are
>>>> +marked with attribute bnd_variable_size are treated as flexible.
>>>>
>>>>  fchkp-optimize
>>>>  C ObjC C++ ObjC++ LTO Report Var(flag_chkp_optimize) Init(-1)
>>>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>>>> index 034ae98..2372c22a 100644
>>>> --- a/gcc/doc/invoke.texi
>>>> +++ b/gcc/doc/invoke.texi
>>>> @@ -10886,6 +10886,13 @@ Forces Pointer Bounds Checker to use narrowed
>>>> bounds for the address of the
>>>>  first field in the structure.  By default a pointer to the first field has
>>>>  the same bounds as a pointer to the whole structure.
>>>>
>>>> +@item -fchkp-flexible-struct-trailing-arrays
>>>> +@opindex fchkp-flexible-struct-trailing-arrays
>>>> +@opindex fno-chkp-flexible-struct-trailing-arrays
>>>> +Forces Pointer Bounds Checker to treat all trailing arrays in structures as
>>>> +possibly flexible.  By default only arrays fields with zero length or that are
>>>> +marked with attribute bnd_variable_size are treated as flexible.
>>>> +
>>>>  @item -fchkp-narrow-to-innermost-array
>>>>  @opindex fchkp-narrow-to-innermost-array
>>>>  @opindex fno-chkp-narrow-to-innermost-array
>>>> diff --git a/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-lbv.c
>>>> b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-lbv.c
>>>> new file mode 100644
>>>> index 0000000..9739920
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-lbv.c
>>>> @@ -0,0 +1,29 @@
>>>> +/* { dg-do run } */
>>>> +/* { dg-shouldfail "bounds violation" } */
>>>> +/* { dg-options "-fcheck-pointer-bounds -mmpx
>>>> -fchkp-flexible-struct-trailing-arrays" } */
>>>> +
>>>> +
>>>> +#define SHOULDFAIL
>>>> +
>>>> +#include "mpx-check.h"
>>>> +
>>>> +struct S
>>>> +{
>>>> +  int a;
>>>> +  int p[10];
>>>> +};
>>>> +
>>>> +int rd (int *p, int i)
>>>> +{
>>>> +  int res = p[i];
>>>> +  printf ("%d\n", res);
>>>> +  return res;
>>>> +}
>>>> +
>>>> +int mpx_test (int argc, const char **argv)
>>>> +{
>>>> +  struct S *s = (struct S *)alloca (sizeof(struct S) + sizeof (int)*100);
>>>> +  rd (s->p, -2);
>>>> +
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-nov.c
>>>> b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-nov.c
>>>> new file mode 100644
>>>> index 0000000..f5c8f95
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-nov.c
>>>> @@ -0,0 +1,29 @@
>>>> +/* { dg-do run } */
>>>> +/* { dg-options "-fcheck-pointer-bounds -mmpx
>>>> -fchkp-flexible-struct-trailing-arrays" } */
>>>> +
>>>> +
>>>> +#include "mpx-check.h"
>>>> +
>>>> +struct S
>>>> +{
>>>> +  int a;
>>>> +  int p[10];
>>>> +};
>>>> +
>>>> +int rd (int *p, int i)
>>>> +{
>>>> +  int res = p[i];
>>>> +  printf ("%d\n", res);
>>>> +  return res;
>>>> +}
>>>> +
>>>> +int mpx_test (int argc, const char **argv)
>>>> +{
>>>> +  struct S *s = (struct S *)alloca (sizeof(struct S) + sizeof (int)*100);
>>>> +  rd (s->p, 0);
>>>> +  rd (s->p, 99);
>>>> +  s->p[0];
>>>> +  s->p[99];
>>>> +
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-ubv.c
>>>> b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-ubv.c
>>>> new file mode 100644
>>>> index 0000000..8385a5a
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/i386/mpx/vla-trailing-1-ubv.c
>>>> @@ -0,0 +1,29 @@
>>>> +/* { dg-do run } */
>>>> +/* { dg-shouldfail "bounds violation" } */
>>>> +/* { dg-options "-fcheck-pointer-bounds -mmpx
>>>> -fchkp-flexible-struct-trailing-arrays" } */
>>>> +
>>>> +
>>>> +#define SHOULDFAIL
>>>> +
>>>> +#include "mpx-check.h"
>>>> +
>>>> +struct S
>>>> +{
>>>> +  int a;
>>>> +  int p[10];
>>>> +};
>>>> +
>>>> +int rd (int *p, int i)
>>>> +{
>>>> +  int res = p[i];
>>>> +  printf ("%d\n", res);
>>>> +  return res;
>>>> +}
>>>> +
>>>> +int mpx_test (int argc, const char **argv)
>>>> +{
>>>> +  struct S *s = (struct S *)alloca (sizeof(struct S) + sizeof (int)*100);
>>>> +  rd (s->p, 110);
>>>> +
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
>>>> index 2769682..6c5e541 100644
>>>> --- a/gcc/tree-chkp.c
>>>> +++ b/gcc/tree-chkp.c
>>>> @@ -3272,6 +3272,8 @@ chkp_may_narrow_to_field (tree field)
>>>>  {
>>>>    return DECL_SIZE (field) && TREE_CODE (DECL_SIZE (field)) == INTEGER_CST
>>>>      && tree_to_uhwi (DECL_SIZE (field)) != 0
>>>> +    && (!flag_chkp_flexible_struct_trailing_arrays
>>>> + || DECL_CHAIN (field))
>>>
>>> What if it's not array?
>>>
>>> Thanks,
>>> Ilya
>>>
>>>>      && (!DECL_FIELD_OFFSET (field)
>>>>   || TREE_CODE (DECL_FIELD_OFFSET (field)) == INTEGER_CST)
>>>>      && (!DECL_FIELD_BIT_OFFSET (field)