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....@gmail.com>: > 2016-12-20 17:44 GMT+03:00 Alexander Ivchenko <aivch...@gmail.com>: >> 2016-11-26 0:28 GMT+03:00 Ilya Enkovich <enkovich....@gmail.com>: >>> 2016-11-25 15:47 GMT+03:00 Alexander Ivchenko <aivch...@gmail.com>: >>>> Hi, >>>> >>>> The patch below addresses PR68270. could you please take a look? >>>> >>>> 2016-11-25 Alexander Ivchenko <aivch...@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)