From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1930) id 061FA3857BAF; Tue, 24 May 2022 22:20:18 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 061FA3857BAF MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Martin Sebor To: gcc-cvs@gcc.gnu.org Subject: [gcc r13-750] PR middle-end/105604 - ICE: in tree_to_shwi with vla in struct and sprintf X-Act-Checkin: gcc X-Git-Author: Martin Sebor X-Git-Refname: refs/heads/master X-Git-Oldrev: 1189c03859cefef4fc4fd44d57eb3d4d3348b562 X-Git-Newrev: 10d1986aee47c592f903527bb68546efc557735d Message-Id: <20220524222018.061FA3857BAF@sourceware.org> Date: Tue, 24 May 2022 22:20:18 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 May 2022 22:20:18 -0000 https://gcc.gnu.org/g:10d1986aee47c592f903527bb68546efc557735d commit r13-750-g10d1986aee47c592f903527bb68546efc557735d Author: Martin Sebor Date: Tue May 24 16:01:12 2022 -0600 PR middle-end/105604 - ICE: in tree_to_shwi with vla in struct and sprintf gcc/ChangeLog: PR middle-end/105604 * gimple-ssa-sprintf.cc (set_aggregate_size_and_offset): Add comments. (get_origin_and_offset_r): Remove null handling. Handle variable array sizes. (get_origin_and_offset): Handle null argument here. Simplify. (alias_offset): Update comment. * pointer-query.cc (field_at_offset): Update comment. Handle members of variable-length types. gcc/testsuite/ChangeLog: PR middle-end/105604 * gcc.dg/Wrestrict-24.c: New test. * gcc.dg/Wrestrict-25.c: New test. * gcc.dg/Wrestrict-26.c: New test. Co-authored-by: Richard Biener Diff: --- gcc/gimple-ssa-sprintf.cc | 77 +++++++++-------- gcc/pointer-query.cc | 12 ++- gcc/testsuite/gcc.dg/Wrestrict-24.c | 35 ++++++++ gcc/testsuite/gcc.dg/Wrestrict-25.c | 165 ++++++++++++++++++++++++++++++++++++ gcc/testsuite/gcc.dg/Wrestrict-26.c | 114 +++++++++++++++++++++++++ 5 files changed, 368 insertions(+), 35 deletions(-) diff --git a/gcc/gimple-ssa-sprintf.cc b/gcc/gimple-ssa-sprintf.cc index 8202129667e..6bd27302213 100644 --- a/gcc/gimple-ssa-sprintf.cc +++ b/gcc/gimple-ssa-sprintf.cc @@ -2232,8 +2232,9 @@ format_character (const directive &dir, tree arg, pointer_query &ptr_qry) } /* If TYPE is an array or struct or union, increment *FLDOFF by the starting - offset of the member that *OFF point into and set *FLDSIZE to its size - in bytes and decrement *OFF by the same. Otherwise do nothing. */ + offset of the member that *OFF points into if one can be determined and + set *FLDSIZE to its size in bytes and decrement *OFF by the same. + Otherwise do nothing. */ static void set_aggregate_size_and_offset (tree type, HOST_WIDE_INT *fldoff, @@ -2249,9 +2250,9 @@ set_aggregate_size_and_offset (tree type, HOST_WIDE_INT *fldoff, if (array_elt_at_offset (type, *off, &index, &arrsize)) { *fldoff += index; - *off -= index; *fldsize = arrsize; } + /* Otherwise leave *FLDOFF et al. unchanged. */ } else if (RECORD_OR_UNION_TYPE_P (type)) { @@ -2269,11 +2270,12 @@ set_aggregate_size_and_offset (tree type, HOST_WIDE_INT *fldoff, *fldoff += index; *off -= index; } + /* Otherwise leave *FLDOFF et al. unchanged. */ } } -/* For an expression X of pointer type, recursively try to find the same - origin (object or pointer) as Y it references and return such a Y. +/* For an expression X of pointer type, recursively try to find its origin + (either object DECL or pointer such as PARM_DECL) Y and return such a Y. When X refers to an array element or struct member, set *FLDOFF to the offset of the element or member from the beginning of the "most derived" object and *FLDSIZE to its size. When nonnull, set *OFF to @@ -2284,9 +2286,6 @@ static tree get_origin_and_offset_r (tree x, HOST_WIDE_INT *fldoff, HOST_WIDE_INT *fldsize, HOST_WIDE_INT *off) { - if (!x) - return NULL_TREE; - HOST_WIDE_INT sizebuf = -1; if (!fldsize) fldsize = &sizebuf; @@ -2308,23 +2307,33 @@ get_origin_and_offset_r (tree x, HOST_WIDE_INT *fldoff, HOST_WIDE_INT *fldsize, case ARRAY_REF: { - tree offset = TREE_OPERAND (x, 1); - HOST_WIDE_INT idx = (tree_fits_uhwi_p (offset) - ? tree_to_uhwi (offset) : HOST_WIDE_INT_MAX); + tree sub = TREE_OPERAND (x, 1); + unsigned HOST_WIDE_INT idx = + tree_fits_uhwi_p (sub) ? tree_to_uhwi (sub) : HOST_WIDE_INT_MAX; - tree eltype = TREE_TYPE (x); - if (TREE_CODE (eltype) == INTEGER_TYPE) + tree elsz = array_ref_element_size (x); + unsigned HOST_WIDE_INT elbytes = + tree_fits_shwi_p (elsz) ? tree_to_shwi (elsz) : HOST_WIDE_INT_MAX; + + unsigned HOST_WIDE_INT byteoff = idx * elbytes; + + if (byteoff < HOST_WIDE_INT_MAX + && elbytes < HOST_WIDE_INT_MAX + && byteoff / elbytes == idx) { + /* For in-bounds constant offsets into constant-sized arrays + bump up *OFF, and for what's likely arrays or structs of + arrays, also *FLDOFF, as necessary. */ if (off) - *off = idx; + *off += byteoff; + if (elbytes > 1) + *fldoff += byteoff; } - else if (idx < HOST_WIDE_INT_MAX) - *fldoff += idx * int_size_in_bytes (eltype); else - *fldoff = idx; + *fldoff = HOST_WIDE_INT_MAX; x = TREE_OPERAND (x, 0); - return get_origin_and_offset_r (x, fldoff, fldsize, nullptr); + return get_origin_and_offset_r (x, fldoff, fldsize, off); } case MEM_REF: @@ -2350,8 +2359,14 @@ get_origin_and_offset_r (tree x, HOST_WIDE_INT *fldoff, HOST_WIDE_INT *fldsize, case COMPONENT_REF: { + tree foff = component_ref_field_offset (x); tree fld = TREE_OPERAND (x, 1); - *fldoff += int_byte_position (fld); + if (!tree_fits_shwi_p (foff) + || !tree_fits_shwi_p (DECL_FIELD_BIT_OFFSET (fld))) + return x; + *fldoff += (tree_to_shwi (foff) + + (tree_to_shwi (DECL_FIELD_BIT_OFFSET (fld)) + / BITS_PER_UNIT)); get_origin_and_offset_r (fld, fldoff, fldsize, off); x = TREE_OPERAND (x, 0); @@ -2411,30 +2426,25 @@ get_origin_and_offset_r (tree x, HOST_WIDE_INT *fldoff, HOST_WIDE_INT *fldsize, return x; } -/* Nonrecursive version of the above. */ +/* Nonrecursive version of the above. + The function never returns null unless X is null to begin with. */ static tree get_origin_and_offset (tree x, HOST_WIDE_INT *fldoff, HOST_WIDE_INT *off, HOST_WIDE_INT *fldsize = nullptr) { + if (!x) + return NULL_TREE; + HOST_WIDE_INT sizebuf; if (!fldsize) fldsize = &sizebuf; + /* Invalidate *FLDSIZE. */ *fldsize = -1; + *fldoff = *off = 0; - *fldoff = *off = *fldsize = 0; - tree orig = get_origin_and_offset_r (x, fldoff, fldsize, off); - if (!orig) - return NULL_TREE; - - if (!*fldoff && *off == *fldsize) - { - *fldoff = *off; - *off = 0; - } - - return orig; + return get_origin_and_offset_r (x, fldoff, fldsize, off); } /* If ARG refers to the same (sub)object or array element as described @@ -2454,7 +2464,8 @@ alias_offset (tree arg, HOST_WIDE_INT *arg_size, return HOST_WIDE_INT_MIN; /* The two arguments may refer to the same object. If they both refer - to a struct member, see if the members are one and the same. */ + to a struct member, see if the members are one and the same. If so, + return the offset into the member. */ HOST_WIDE_INT arg_off = 0, arg_fld = 0; tree arg_orig = get_origin_and_offset (arg, &arg_fld, &arg_off, arg_size); diff --git a/gcc/pointer-query.cc b/gcc/pointer-query.cc index 67c25503f46..ae561731216 100644 --- a/gcc/pointer-query.cc +++ b/gcc/pointer-query.cc @@ -2448,9 +2448,13 @@ field_at_offset (tree type, tree start_after, HOST_WIDE_INT off, /* The offset of FLD within its immediately enclosing structure. */ HOST_WIDE_INT fldpos = next_pos < 0 ? int_byte_position (fld) : next_pos; + tree typesize = TYPE_SIZE_UNIT (fldtype); + if (typesize && TREE_CODE (typesize) != INTEGER_CST) + /* Bail if FLD is a variable length member. */ + return NULL_TREE; + /* If the size is not available the field is a flexible array member. Treat this case as success. */ - tree typesize = TYPE_SIZE_UNIT (fldtype); HOST_WIDE_INT fldsize = (tree_fits_uhwi_p (typesize) ? tree_to_uhwi (typesize) : off); @@ -2464,7 +2468,11 @@ field_at_offset (tree type, tree start_after, HOST_WIDE_INT off, { /* If OFF is equal to the offset of the next field continue to it and skip the array/struct business below. */ - next_pos = int_byte_position (next_fld); + tree pos = byte_position (next_fld); + if (!tree_fits_shwi_p (pos)) + /* Bail if NEXT_FLD is a variable length member. */ + return NULL_TREE; + next_pos = tree_to_shwi (pos); *nextoff = *fldoff + next_pos; if (*nextoff == off && TREE_CODE (type) != UNION_TYPE) continue; diff --git a/gcc/testsuite/gcc.dg/Wrestrict-24.c b/gcc/testsuite/gcc.dg/Wrestrict-24.c new file mode 100644 index 00000000000..d224d80f87a --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wrestrict-24.c @@ -0,0 +1,35 @@ +/* PR tree-optimization/105604 - ICE: in tree_to_shwi with vla in struct + and sprintf + { dg-do compile } + { dg-options "-O2 -Wall -Wrestrict" } */ + +extern int sprintf (char*, const char*, ...); + +extern void* sink (void*, ...); + +struct { + long users; + long size; + char *data; +} * main_trans; + +void *main___trans_tmp_1; + +int users = 0; + +void test (void) +{ + struct { + long users; + long size; + char *data; + int links[users]; + char buf[]; + } *trans = sink (0); + + trans->data = trans->buf; + main___trans_tmp_1 = trans; + main_trans = main___trans_tmp_1; + sprintf (main_trans->data, "test"); + sink (main_trans->data); +} diff --git a/gcc/testsuite/gcc.dg/Wrestrict-25.c b/gcc/testsuite/gcc.dg/Wrestrict-25.c new file mode 100644 index 00000000000..a15f56d7424 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wrestrict-25.c @@ -0,0 +1,165 @@ +/* PR tree-optimization/105604 - ICE: in tree_to_shwi with vla in struct + and sprintf + { dg-do compile } + { dg-options "-O2 -Wall -Wrestrict" } */ + +extern int sprintf (char*, const char*, ...); + +void* sink (void*); + + +void sprintf_S_a8_an_bn (int n, int i, int j) +{ + struct { + char a8[8], an[n], bn[n]; + } *p = sink (0); + + { + char *d = p->a8 + i; + char *s = p->a8; + sprintf (d, "%s", s); // { dg-warning "argument 3 may overlap" } + sink (p); + } + + { + char *d = p->a8; + char *s = p->a8 + j; + sprintf (d, "%s", s); // { dg-warning "argument 3 may overlap" } + sink (p); + } + + { + char *d = p->a8 + i; + char *s = p->a8 + j; + sprintf (d, "%s", s); // { dg-warning "argument 3 may overlap" } + sink (p); + } + + { + char *d = p->a8 + i; + char *s = p->an; + sprintf (d, "%s", s); + sink (p); + } + + { + char *d = p->a8; + char *s = p->an + j; + sprintf (d, "%s", s); + sink (p); + } + + { + char *d = p->a8 + i; + char *s = p->an + j; + sprintf (d, "%s", s); + sink (p); + } + + { + /* The IL makes it impossible to rule out an overlap between + p->a8 + i and p->bn + i so the "may overlap" warning triggers. */ + char *d = p->a8 + i; + char *s = p->bn; + sprintf (d, "%s", s); // { dg-bogus "-Wrestrict" "pr??????" { xfail *-*-* } } + sink (p); + } + + { + char *d = p->a8; + char *s = p->bn + j; + sprintf (d, "%s", s); // { dg-bogus "-Wrestrict" "pr??????" { xfail *-*-* } } + sink (p); + } + + { + char *d = p->a8 + i; + char *s = p->bn + j; + sprintf (d, "%s", s); // { dg-bogus "-Wrestrict" "pr??????" { xfail *-*-* } } + sink (p); + } + + { + char *d = p->an + i; + char *s = p->bn; + sprintf (d, "%s", s); + sink (p); + } + + { + char *d = p->an; + char *s = p->bn + j; + sprintf (d, "%s", s); + sink (p); + } + + { + char *d = p->an + i; + char *s = p->bn + j; + sprintf (d, "%s", s); + sink (p); + } + + { + char *d = p->an + i; + char *s = p->a8; + sprintf (d, "%s", s); + sink (p); + } + + { + char *d = p->an; + char *s = p->a8 + j; + sprintf (d, "%s", s); + sink (p); + } + + { + char *d = p->an + i; + char *s = p->a8 + j; + sprintf (d, "%s", s); + sink (p); + } + + { + char *d = p->bn + i; + char *s = p->a8; + sprintf (d, "%s", s); // { dg-bogus "-Wrestrict" "pr??????" { xfail *-*-* } } + sink (p); + } + + { + char *d = p->bn; + char *s = p->a8 + j; + sprintf (d, "%s", s); // { dg-bogus "-Wrestrict" "pr??????" { xfail *-*-* } } + sink (p); + } + + { + char *d = p->bn + i; + char *s = p->a8 + j; + sprintf (d, "%s", s); // { dg-bogus "-Wrestrict" "pr??????" { xfail *-*-* } } + sink (p); + } + + { + char *d = p->bn + i; + char *s = p->bn; + sprintf (d, "%s", s); // { dg-warning "may overlap" } + sink (p); + } + + { + char *d = p->bn; + char *s = p->bn + j; + sprintf (d, "%s", s); // { dg-warning "may overlap" } + sink (p); + } + + { + char *d = p->bn + i; + char *s = p->bn + j; + sprintf (d, "%s", s); // { dg-warning "may overlap" } + sink (p); + } +} diff --git a/gcc/testsuite/gcc.dg/Wrestrict-26.c b/gcc/testsuite/gcc.dg/Wrestrict-26.c new file mode 100644 index 00000000000..a10c426a081 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wrestrict-26.c @@ -0,0 +1,114 @@ +/* Verify that sprintf calls with arrays or struct of arrays don't + cause -Wrestrict false positives. + { dg-do compile } + { dg-options "-O2 -Wall -Wrestrict -ftrack-macro-expansion=0" } */ + +#define sprintf(d, f, ...) (sprintf (d, f, __VA_ARGS__), sink (d)) + +extern void sink (void*, ...); +extern int (sprintf) (char*, const char*, ...); + +extern char ca[][2][8]; + +void test_array_of_arrays (void) +{ + sprintf (ca[0][0], "%s", ca[0][0]); // { dg-warning "-Wrestrict" } + sprintf (ca[0][0], "%s", ca[0][1]); + sprintf (ca[0][0], "%s", ca[1][0]); + sprintf (ca[0][0], "%s", ca[1][1]); + + sprintf (ca[0][1], "%s", ca[0][0]); + sprintf (ca[0][1], "%s", ca[0][1]); // { dg-warning "-Wrestrict" } + sprintf (ca[0][1], "%s", ca[1][0]); + sprintf (ca[0][1], "%s", ca[1][1]); + + sprintf (ca[1][0], "%s", ca[0][0]); + sprintf (ca[1][0], "%s", ca[0][1]); + sprintf (ca[1][0], "%s", ca[1][0]); // { dg-warning "-Wrestrict" } + sprintf (ca[1][0], "%s", ca[1][1]); + + sprintf (ca[1][1], "%s", ca[0][0]); + sprintf (ca[1][1], "%s", ca[0][1]); + sprintf (ca[1][1], "%s", ca[1][0]); + sprintf (ca[1][1], "%s", ca[1][1]); // { dg-warning "-Wrestrict" } +} + + +struct A +{ + char a[2][2][8]; + char b[2][2][8]; + char c[2][2][8]; +}; + +extern struct A aa[][2]; + +void test_array_of_structs (void) +{ + // Verify that calls with the same elements of the same array trigger + // warnings as expected. + sprintf (aa[0][0].a[0][0], "%s", aa[0][0].a[0][0]); // { dg-warning "-Wrestrict" } + sprintf (aa[0][0].a[0][1], "%s", aa[0][0].a[0][1]); // { dg-warning "-Wrestrict" } + sprintf (aa[0][0].a[1][0], "%s", aa[0][0].a[1][0]); // { dg-warning "-Wrestrict" } + sprintf (aa[0][0].a[1][1], "%s", aa[0][0].a[1][1]); // { dg-warning "-Wrestrict" } + sprintf (aa[0][1].a[0][0], "%s", aa[0][1].a[0][0]); // { dg-warning "-Wrestrict" } + sprintf (aa[0][1].a[0][1], "%s", aa[0][1].a[0][1]); // { dg-warning "-Wrestrict" } + sprintf (aa[0][1].a[1][0], "%s", aa[0][1].a[1][0]); // { dg-warning "-Wrestrict" } + sprintf (aa[0][1].a[1][1], "%s", aa[0][1].a[1][1]); // { dg-warning "-Wrestrict" } + sprintf (aa[1][0].a[0][0], "%s", aa[1][0].a[0][0]); // { dg-warning "-Wrestrict" } + sprintf (aa[1][0].a[0][1], "%s", aa[1][0].a[0][1]); // { dg-warning "-Wrestrict" } + sprintf (aa[1][0].a[1][0], "%s", aa[1][0].a[1][0]); // { dg-warning "-Wrestrict" } + sprintf (aa[1][0].a[1][1], "%s", aa[1][0].a[1][1]); // { dg-warning "-Wrestrict" } + sprintf (aa[1][1].a[0][0], "%s", aa[1][1].a[0][0]); // { dg-warning "-Wrestrict" } + sprintf (aa[1][1].a[0][1], "%s", aa[1][1].a[0][1]); // { dg-warning "-Wrestrict" } + sprintf (aa[1][1].a[1][0], "%s", aa[1][1].a[1][0]); // { dg-warning "-Wrestrict" } + sprintf (aa[1][1].a[1][1], "%s", aa[1][1].a[1][1]); // { dg-warning "-Wrestrict" } + +#define NOWARN() + + // Exhaustively verify that calls with different elements of the same + // array don't cause false positives. +#undef NOWARN +#define NOWARN(D, S) \ + sprintf (D[0][0], "%s", S[0][0]); \ + sprintf (D[0][0], "%s", S[0][1]); \ + sprintf (D[0][0], "%s", S[1][0]); \ + sprintf (D[0][0], "%s", S[1][1]); \ + sprintf (D[0][1], "%s", S[0][0]); \ + sprintf (D[0][1], "%s", S[0][1]); \ + sprintf (D[0][1], "%s", S[1][0]); \ + sprintf (D[0][1], "%s", S[1][1]); \ + sprintf (D[1][0], "%s", S[0][0]); \ + sprintf (D[1][0], "%s", S[0][1]); \ + sprintf (D[1][0], "%s", S[1][0]); \ + sprintf (D[1][0], "%s", S[1][1]); \ + sprintf (D[1][1], "%s", S[0][0]); \ + sprintf (D[1][1], "%s", S[0][1]); \ + sprintf (D[1][1], "%s", S[1][0]); \ + sprintf (D[1][1], "%s", S[1][1]) + + NOWARN (aa[0][0].a, aa[0][1].a); + NOWARN (aa[0][0].a, aa[1][0].a); + NOWARN (aa[0][0].a, aa[1][1].a); + + NOWARN (aa[0][1].a, aa[0][0].a); + NOWARN (aa[0][1].a, aa[1][0].a); + NOWARN (aa[0][1].a, aa[1][1].a); + + NOWARN (aa[1][0].a, aa[0][0].a); + NOWARN (aa[1][0].a, aa[0][1].a); + NOWARN (aa[1][0].a, aa[1][1].a); + +#define NOWARN_MEM(M1, M2) \ + NOWARN (aa[0][0].M1, aa[0][0].M2); \ + NOWARN (aa[0][0].M1, aa[0][1].M2); \ + NOWARN (aa[0][0].M1, aa[1][0].M2); \ + NOWARN (aa[0][0].M1, aa[1][1].M2) + + NOWARN_MEM (a, b); + NOWARN_MEM (a, c); + NOWARN_MEM (b, a); + NOWARN_MEM (b, c); + NOWARN_MEM (c, a); + NOWARN_MEM (c, b); +}