|

»

»

[PATCH] Fix fold_offsetof_1 (PR middle-end/41935)

View: New views

9 Messages — Rating Filter: Alert me

	[PATCH] Fix fold_offsetof_1 (PR middle-end/41935) by Jakub Jelinek :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message Hi! 2009-11-04 Jakub Jelinek <jakub@...> PR middle-end/41935 * c-common.c (fold_offsetof_1) <case ARRAY_REF>: Don't crash for VLAs or non-constant index, allow index one past the last element. * gcc.dg/pr41935.c: New test. * c-c++-common/builtin-offsetof.c (f0): Allow index one past the last element. * gcc.c-torture/execute/pr41935.c: New test. --- gcc/c-common.c.jj 2009-11-04 08:22:49.000000000 +0100 +++ gcc/c-common.c 2009-11-04 11:25:16.000000000 +0100 @@ -8398,14 +8398,22 @@ fold_offsetof_1 (tree expr, tree stop_re off = size_binop (MULT_EXPR, TYPE_SIZE_UNIT (TREE_TYPE (expr)), t); /* Check if the offset goes beyond the upper bound of the array. / - { - tree nelts = array_type_nelts (TREE_TYPE (TREE_OPERAND (expr, 0))); - HOST_WIDE_INT index = int_cst_value (t); - if (index > int_cst_value (nelts)) - warning (OPT_Warray_bounds, - "index %wd denotes an offset greater than size of %qT", - index, TREE_TYPE (TREE_OPERAND (expr, 0))); - } + if (code == PLUS_EXPR && TREE_CODE (t) == INTEGER_CST) + { + tree upbound = array_ref_up_bound (expr); + if (upbound != NULL_TREE + && TREE_CODE (upbound) == INTEGER_CST + && !tree_int_cst_equal (upbound, + TYPE_MAX_VALUE (TREE_TYPE (upbound)))) + { + upbound = size_binop (PLUS_EXPR, upbound, + build_int_cst (TREE_TYPE (upbound), 1)); + if (tree_int_cst_lt (upbound, t)) + warning (OPT_Warray_bounds, + "index %E denotes an offset greater than size of %qT", + t, TREE_TYPE (TREE_OPERAND (expr, 0))); + } + } break; case COMPOUND_EXPR: --- gcc/testsuite/gcc.dg/pr41935.c.jj 2009-11-04 10:38:56.000000000 +0100 +++ gcc/testsuite/gcc.dg/pr41935.c 2009-11-04 10:34:39.000000000 +0100 @@ -0,0 +1,25 @@ +/ PR middle-end/41935 / +/ { dg-do run } / +/ { dg-options "-O2" } / + +extern void abort (void); +struct A { int a; int b[10]; }; + +int +foo (struct A p) +{ + return __builtin_offsetof (struct A, b[p->a]); +} + +int +main () +{ + struct A a; + a.a = 7; + if (foo (&a) != 7 * sizeof (int) + __builtin_offsetof (struct A, b)) + abort (); + a.a = 2; + if (foo (&a) != 2 * sizeof (int) + __builtin_offsetof (struct A, b)) + abort (); + return 0; +} --- gcc/testsuite/c-c++-common/builtin-offsetof.c.jj 2009-11-04 08:15:30.000000000 +0100 +++ gcc/testsuite/c-c++-common/builtin-offsetof.c 2009-11-04 10:55:30.000000000 +0100 @@ -21,9 +21,9 @@ f0 () __builtin_offsetof(struct A, p[0]); // { dg-error "non constant address" } __builtin_offsetof(struct B, p[0]); // OK __builtin_offsetof(struct B, p[9]); // OK - __builtin_offsetof(struct B, p[10]); // { dg-warning "greater than size" } + __builtin_offsetof(struct B, p[10]); // OK + __builtin_offsetof(struct B, p[11]); // { dg-warning "greater than size" } __builtin_offsetof(struct B, a.p); // OK __builtin_offsetof(struct B, p[0]); // OK __builtin_offsetof(struct B, a.p[0]); // { dg-error "non constant address" } } - --- gcc/testsuite/gcc.c-torture/execute/pr41935.c.jj 2009-11-04 11:44:24.000000000 +0100 +++ gcc/testsuite/gcc.c-torture/execute/pr41935.c 2009-11-04 11:51:08.000000000 +0100 @@ -0,0 +1,25 @@ +/* PR middle-end/41935 / + +extern void abort (void); + +long int +foo (int n, int i, int j) +{ + typedef int T[n]; + struct S { int a; T b[n]; }; + return __builtin_offsetof (struct S, b[i][j]); +} + +int +main (void) +{ + typedef int T[5]; + struct S { int a; T b[5]; }; + if (foo (5, 2, 3) + != __builtin_offsetof (struct S, b) + (5 2 + 3) * sizeof (int)) + abort (); + if (foo (5, 5, 5) + != __builtin_offsetof (struct S, b) + (5 * 5 + 5) * sizeof (int)) + abort (); + return 0; +} Jakub
	Re: [PATCH] Fix fold_offsetof_1 (PR middle-end/41935) by Jakub Jelinek :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message Hi! On Wed, Nov 04, 2009 at 01:55:24PM +0100, Jakub Jelinek wrote: > 2009-11-04 Jakub Jelinek <jakub@...> > > PR middle-end/41935 > * c-common.c (fold_offsetof_1) <case ARRAY_REF>: Don't crash for VLAs > or non-constant index, allow index one past the last element. > > * gcc.dg/pr41935.c: New test. > * c-c++-common/builtin-offsetof.c (f0): Allow index one past the last > element. > * gcc.c-torture/execute/pr41935.c: New test. Oops, hit Send too early. The C FE apparently allows non-constant array indexes in offsetof member-designator (question is whether it should in pedantic mode), plus the array in the structure can be VLA as well, so assuming int_cst_value can be used is wrong. Also, IMHO we shouldn't warn with -Warray-bounds for offsetof with one past the last entry, as taking address of that is allowed in address arithmetics and offsetof doesn't dereference that in any way. Jakub
	Re: [PATCH] Fix fold_offsetof_1 (PR middle-end/41935) by Paolo Bonzini-6 :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message > Also, IMHO we shouldn't warn with -Warray-bounds for offsetof with > one past the last entry, as taking address of that is allowed in address > arithmetics and offsetof doesn't dereference that in any way. We also shouldn't warn about array bounds for the last entry of a struct, like typedef struct _VPD_IDENTIFICATION_DESCRIPTOR { UCHAR CodeSet : 4; UCHAR Reserved : 4; UCHAR IdentifierType : 4; UCHAR Association : 2; UCHAR Reserved2 : 2; UCHAR Reserved3; UCHAR IdentifierLength; UCHAR Identifier[1]; } VPD_IDENTIFICATION_DESCRIPTOR; typedef VPD_IDENTIFICATION_DESCRIPTOR *PVPD_IDENTIFICATION_DESCRIPTOR; ... return offsetof (VPD_IDENTIFICATION_DESCRIPTOR, Identifier[4]); (Saw that today in real-world code). Paolo
	Re: [PATCH] Fix fold_offsetof_1 (PR middle-end/41935) by Jason Merrill :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message On 11/04/2009 08:23 AM, Jakub Jelinek wrote: > Oops, hit Send too early. The C FE apparently allows non-constant array > indexes in offsetof member-designator Deliberately? > Also, IMHO we shouldn't warn with -Warray-bounds for offsetof with > one past the last entry, as taking address of that is allowed in address > arithmetics and offsetof doesn't dereference that in any way. Agreed. Jason
	Re: [PATCH] Fix fold_offsetof_1 (PR middle-end/41935) by Jakub Jelinek :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message On Wed, Nov 04, 2009 at 10:59:26AM -0500, Jason Merrill wrote: > On 11/04/2009 08:23 AM, Jakub Jelinek wrote: >> Oops, hit Send too early. The C FE apparently allows non-constant array >> indexes in offsetof member-designator > > Deliberately? No idea. Certainly when offsetof has been implemented as a macro it was allowed and clearly code in the wild uses it (otherwise we wouldn't get the bugreport). Jakub
	[PATCH] Fix fold_offsetof_1 (PR middle-end/41935, take 2) by Jakub Jelinek :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message On Wed, Nov 04, 2009 at 04:54:56PM +0100, Paolo Bonzini wrote: > >> Also, IMHO we shouldn't warn with -Warray-bounds for offsetof with >> one past the last entry, as taking address of that is allowed in address >> arithmetics and offsetof doesn't dereference that in any way. > > We also shouldn't warn about array bounds for the last entry of a > struct, like > > typedef struct _VPD_IDENTIFICATION_DESCRIPTOR { > UCHAR CodeSet : 4; > UCHAR Reserved : 4; > UCHAR IdentifierType : 4; > UCHAR Association : 2; > UCHAR Reserved2 : 2; > UCHAR Reserved3; > UCHAR IdentifierLength; > UCHAR Identifier[1]; > } VPD_IDENTIFICATION_DESCRIPTOR; > typedef VPD_IDENTIFICATION_DESCRIPTOR PVPD_IDENTIFICATION_DESCRIPTOR; > > ... > > return offsetof (VPD_IDENTIFICATION_DESCRIPTOR, Identifier[4]); > > (Saw that today in real-world code). Here is an updated patch that doesn't warn in that (and similar) case(s). Regarding the non-constants, for VL structures at least the bounds will be non-constant, and whether the FE allows or doesn't allow non-constant array indexes in offsetof should be something checked in the FE, rather than in code that is just trying to warn about out of bound array accesses. 2009-11-04 Jakub Jelinek <jakub@...> PR middle-end/41935 c-common.c (fold_offsetof_1) <case ARRAY_REF>: Don't crash for VLAs or non-constant index, allow index one past the last element and allow exceeding array bound in arrays that might be used as flexible array members. * gcc.dg/pr41935.c: New test. * c-c++-common/pr41935.c: New test. * c-c++-common/builtin-offsetof.c (f0): Allow index one past the last element. * gcc.c-torture/execute/pr41935.c: New test. --- gcc/c-common.c.jj 2009-11-04 18:36:24.000000000 +0100 +++ gcc/c-common.c 2009-11-04 20:35:45.000000000 +0100 @@ -8398,14 +8398,46 @@ fold_offsetof_1 (tree expr, tree stop_re off = size_binop (MULT_EXPR, TYPE_SIZE_UNIT (TREE_TYPE (expr)), t); /* Check if the offset goes beyond the upper bound of the array. / - { - tree nelts = array_type_nelts (TREE_TYPE (TREE_OPERAND (expr, 0))); - HOST_WIDE_INT index = int_cst_value (t); - if (index > int_cst_value (nelts)) - warning (OPT_Warray_bounds, - "index %wd denotes an offset greater than size of %qT", - index, TREE_TYPE (TREE_OPERAND (expr, 0))); - } + if (code == PLUS_EXPR && TREE_CODE (t) == INTEGER_CST) + { + tree upbound = array_ref_up_bound (expr); + if (upbound != NULL_TREE + && TREE_CODE (upbound) == INTEGER_CST + && !tree_int_cst_equal (upbound, + TYPE_MAX_VALUE (TREE_TYPE (upbound)))) + { + upbound = size_binop (PLUS_EXPR, upbound, + build_int_cst (TREE_TYPE (upbound), 1)); + if (tree_int_cst_lt (upbound, t)) + { + tree v; + + for (v = TREE_OPERAND (expr, 0); + TREE_CODE (v) == COMPONENT_REF; + v = TREE_OPERAND (v, 0)) + if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0))) + == RECORD_TYPE) + { + tree fld_chain = TREE_CHAIN (TREE_OPERAND (v, 1)); + for (; fld_chain; fld_chain = TREE_CHAIN (fld_chain)) + if (TREE_CODE (fld_chain) == FIELD_DECL) + break; + + if (fld_chain) + break; + } + / Don't warn if the array might be considered a poor + man's flexible array member with a very permissive + definition thereof. / + if (TREE_CODE (v) == ARRAY_REF + \|\| TREE_CODE (v) == COMPONENT_REF) + warning (OPT_Warray_bounds, + "index %E denotes an offset " + "greater than size of %qT", + t, TREE_TYPE (TREE_OPERAND (expr, 0))); + } + } + } break; case COMPOUND_EXPR: --- gcc/testsuite/gcc.dg/pr41935.c.jj 2009-11-04 19:04:37.000000000 +0100 +++ gcc/testsuite/gcc.dg/pr41935.c 2009-11-04 19:04:37.000000000 +0100 @@ -0,0 +1,25 @@ +/ PR middle-end/41935 / +/ { dg-do run } / +/ { dg-options "-O2" } / + +extern void abort (void); +struct A { int a; int b[10]; }; + +int +foo (struct A p) +{ + return __builtin_offsetof (struct A, b[p->a]); +} + +int +main () +{ + struct A a; + a.a = 7; + if (foo (&a) != 7 * sizeof (int) + __builtin_offsetof (struct A, b)) + abort (); + a.a = 2; + if (foo (&a) != 2 * sizeof (int) + __builtin_offsetof (struct A, b)) + abort (); + return 0; +} --- gcc/testsuite/c-c++-common/builtin-offsetof.c.jj 2009-11-04 18:36:08.000000000 +0100 +++ gcc/testsuite/c-c++-common/builtin-offsetof.c 2009-11-04 19:04:37.000000000 +0100 @@ -21,9 +21,9 @@ f0 () __builtin_offsetof(struct A, p[0]); // { dg-error "non constant address" } __builtin_offsetof(struct B, p[0]); // OK __builtin_offsetof(struct B, p[9]); // OK - __builtin_offsetof(struct B, p[10]); // { dg-warning "greater than size" } + __builtin_offsetof(struct B, p[10]); // OK + __builtin_offsetof(struct B, p[11]); // { dg-warning "greater than size" } __builtin_offsetof(struct B, a.p); // OK __builtin_offsetof(struct B, p[0]); // OK __builtin_offsetof(struct B, a.p[0]); // { dg-error "non constant address" } } - --- gcc/testsuite/gcc.c-torture/execute/pr41935.c.jj 2009-11-04 19:04:37.000000000 +0100 +++ gcc/testsuite/gcc.c-torture/execute/pr41935.c 2009-11-04 19:04:37.000000000 +0100 @@ -0,0 +1,25 @@ +/* PR middle-end/41935 / + +extern void abort (void); + +long int +foo (int n, int i, int j) +{ + typedef int T[n]; + struct S { int a; T b[n]; }; + return __builtin_offsetof (struct S, b[i][j]); +} + +int +main (void) +{ + typedef int T[5]; + struct S { int a; T b[5]; }; + if (foo (5, 2, 3) + != __builtin_offsetof (struct S, b) + (5 2 + 3) * sizeof (int)) + abort (); + if (foo (5, 5, 5) + != __builtin_offsetof (struct S, b) + (5 * 5 + 5) * sizeof (int)) + abort (); + return 0; +} --- gcc/testsuite/c-c++-common/pr41935.c.jj 2009-11-04 19:58:43.000000000 +0100 +++ gcc/testsuite/c-c++-common/pr41935.c 2009-11-04 20:06:49.000000000 +0100 @@ -0,0 +1,70 @@ +/* { dg-options "-Warray-bounds" } / +/ { dg-do compile } / + +struct A +{ + int i; + char p[1]; +}; + +struct B +{ + struct A a; + int i; +}; + +struct C +{ + int i; + struct A a; +}; + +union D +{ + char p[1]; + struct A a; + struct B b; + struct C c; +}; + +struct E +{ + int i; + union D d; +}; + +struct F +{ + union D d; + int i; +}; + +union G +{ + int i; + union D d; +}; + +void +f0 () +{ + __builtin_offsetof (struct A, p[4]); / OK / + __builtin_offsetof (struct B, a.p[4]); / { dg-warning "greater than size" } / + __builtin_offsetof (struct C, a.p[4]); / OK / + __builtin_offsetof (union D, p[4]); / OK / + __builtin_offsetof (union D, a.p[4]); / OK / + __builtin_offsetof (union D, b.a.p[4]); / { dg-warning "greater than size" } / + __builtin_offsetof (union D, c.a.p[4]); / OK / + __builtin_offsetof (struct E, d.p[4]); / OK / + __builtin_offsetof (struct E, d.a.p[4]); / OK / + __builtin_offsetof (struct E, d.b.a.p[4]); / { dg-warning "greater than size" } / + __builtin_offsetof (struct E, d.c.a.p[4]); / OK / + __builtin_offsetof (struct F, d.p[4]); / { dg-warning "greater than size" } / + __builtin_offsetof (struct F, d.a.p[4]); / { dg-warning "greater than size" } / + __builtin_offsetof (struct F, d.b.a.p[4]); / { dg-warning "greater than size" } / + __builtin_offsetof (struct F, d.c.a.p[4]); / { dg-warning "greater than size" } / + __builtin_offsetof (union G, d.p[4]); / OK / + __builtin_offsetof (union G, d.a.p[4]); / OK / + __builtin_offsetof (union G, d.b.a.p[4]); / { dg-warning "greater than size" } / + __builtin_offsetof (union G, d.c.a.p[4]); / OK */ +} Jakub
	Re: [PATCH] Fix fold_offsetof_1 (PR middle-end/41935, take 2) by Joseph S. Myers :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message On Wed, 4 Nov 2009, Jakub Jelinek wrote: > 2009-11-04 Jakub Jelinek <jakub@...> > > PR middle-end/41935 > * c-common.c (fold_offsetof_1) <case ARRAY_REF>: Don't crash for VLAs > or non-constant index, allow index one past the last element and > allow exceeding array bound in arrays that might be used as flexible > array members. > > * gcc.dg/pr41935.c: New test. > * c-c++-common/pr41935.c: New test. > * c-c++-common/builtin-offsetof.c (f0): Allow index one past the last > element. > * gcc.c-torture/execute/pr41935.c: New test. OK unless someone finds problems with the proposed patch in the next 24 hours. -- Joseph S. Myers joseph@...
	[4.4 PATCH] Fix fold_offsetof_1 (PR middle-end/41935) by Jakub Jelinek :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message On Thu, Nov 05, 2009 at 02:29:06AM +0000, Joseph S. Myers wrote: > On Wed, 4 Nov 2009, Jakub Jelinek wrote: > > > 2009-11-04 Jakub Jelinek <jakub@...> > > > > PR middle-end/41935 > > * c-common.c (fold_offsetof_1) <case ARRAY_REF>: Don't crash for VLAs > > or non-constant index, allow index one past the last element and > > allow exceeding array bound in arrays that might be used as flexible > > array members. > > > > * gcc.dg/pr41935.c: New test. > > * c-c++-common/pr41935.c: New test. > > * c-c++-common/builtin-offsetof.c (f0): Allow index one past the last > > element. > > * gcc.c-torture/execute/pr41935.c: New test. > > OK unless someone finds problems with the proposed patch in the next 24 > hours. Thanks. For 4.4 I think it would be better to revert this code and keep just the ICE fix. Warning with -Warray-bounds on out of bounds offsetof member-designators looks like a new feature to me rather than a regression fix, thus not appropriate to the release branch. Ok for 4.4? 2009-11-05 Jakub Jelinek <jakub@...> * c-common.c (fold_offsetof_1): Revert the recently added -Warray-bounds checking of offsetof arguments. * c-c++-common/builtin-offset.c: Renamed to ... * c-c++-common/builtin-offsetof.c: ... this. Don't expect a warning on offsetof (struct B, p[10]). --- gcc/c-common.c.jj 2009-11-04 08:27:18.000000000 +0100 +++ gcc/c-common.c 2009-11-05 14:41:25.000000000 +0100 @@ -7672,16 +7672,6 @@ fold_offsetof_1 (tree expr, tree stop_re } t = convert (sizetype, t); off = size_binop (MULT_EXPR, TYPE_SIZE_UNIT (TREE_TYPE (expr)), t); - - /* Check if the offset goes beyond the upper bound of the array. / - { - tree nelts = array_type_nelts (TREE_TYPE (TREE_OPERAND (expr, 0))); - HOST_WIDE_INT index = int_cst_value (t); - if (index > int_cst_value (nelts)) - warning (OPT_Warray_bounds, - "index %wd denotes an offset greater than size of %qT", - index, TREE_TYPE (TREE_OPERAND (expr, 0))); - } break; case COMPOUND_EXPR: --- gcc/testsuite/c-c++-common/builtin-offsetof.c.jj 2009-11-05 14:42:36.000000000 +0100 +++ gcc/testsuite/c-c++-common/builtin-offsetof.c 2009-11-05 14:42:50.000000000 +0100 @@ -0,0 +1,28 @@ +// Contributed by Dodji Seketeli <dodji@...> +// Origin PR c++/38699 +// { dg-options "-Warray-bounds" } +// { dg-do compile } + +struct A +{ + const char p; +}; + +struct B +{ + char p[10]; + struct A a; +}; + +void +f0 () +{ + __builtin_offsetof(struct A, p); // OK + __builtin_offsetof(struct A, p[0]); // { dg-error "non constant address" } + __builtin_offsetof(struct B, p[0]); // OK + __builtin_offsetof(struct B, p[9]); // OK + __builtin_offsetof(struct B, p[10]); // OK + __builtin_offsetof(struct B, a.p); // OK + __builtin_offsetof(struct B, p[0]); // OK + __builtin_offsetof(struct B, a.p[0]); // { dg-error "non constant address" } +} --- gcc/testsuite/c-c++-common/builtin-offset.c.jj 2009-11-03 23:37:24.000000000 +0100 +++ gcc/testsuite/c-c++-common/builtin-offset.c 2009-11-05 14:42:24.000000000 +0100 @@ -1,29 +0,0 @@ -// Contributed by Dodji Seketeli <dodji@...> -// Origin PR c++/38699 -// { dg-options "-Warray-bounds" } -// { dg-do compile } - -struct A -{ - const char *p; -}; - -struct B -{ - char p[10]; - struct A a; -}; - -void -f0 () -{ - __builtin_offsetof(struct A, p); // OK - __builtin_offsetof(struct A, p[0]); // { dg-error "non constant address" } - __builtin_offsetof(struct B, p[0]); // OK - __builtin_offsetof(struct B, p[9]); // OK - __builtin_offsetof(struct B, p[10]); // { dg-warning "greater than size" } - __builtin_offsetof(struct B, a.p); // OK - __builtin_offsetof(struct B, p[0]); // OK - __builtin_offsetof(struct B, a.p[0]); // { dg-error "non constant address" } -} - Jakub
	Re: [4.4 PATCH] Fix fold_offsetof_1 (PR middle-end/41935) by Jason Merrill :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message On 11/05/2009 08:47 AM, Jakub Jelinek wrote: > Thanks. For 4.4 I think it would be better to revert this code and keep > just the ICE fix. Warning with -Warray-bounds on out of bounds offsetof > member-designators looks like a new feature to me rather than a regression > fix, thus not appropriate to the release branch. > > Ok for 4.4? OK. Jason