[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 Jakub Jelinek changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #21 from Jakub Jelinek --- Fixed.
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 --- Comment #20 from prathamesh3492 at gcc dot gnu.org --- Author: prathamesh3492 Date: Fri Nov 25 08:03:51 2016 New Revision: 242858 URL: https://gcc.gnu.org/viewcvs?rev=242858&root=gcc&view=rev Log: 2016-11-25 Jakub Jelinek Prathamesh Kulkarni PR middle-end/78501 * tree-vrp.c (extract_range_basic): Check for ptrdiff_type_node to be non null and it's precision matches precision of lhs's type. Modified: trunk/gcc/ChangeLog trunk/gcc/tree-vrp.c
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 --- Comment #19 from prathamesh3492 at gcc dot gnu.org --- (In reply to Jakub Jelinek from comment #18) > Comment on attachment 40139 [details] > fix formatting of patch in comment 13 > > LGTM. Thanks, I have started bootstrap on x86_64-unknown-linux-gnu with --enable-languages=all,ada and will post it on patches list after the build completes. Thanks, Prathamesh
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 --- Comment #18 from Jakub Jelinek --- Comment on attachment 40139 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=40139 fix formatting of patch in comment 13 LGTM.
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 --- Comment #17 from prathamesh3492 at gcc dot gnu.org --- (In reply to Jakub Jelinek from comment #15) > For the tests, I think they would just pass on VMS. In one case you are > comparing if 0x7fffULL <= (unsigned int) something, that is for > 32-bit int clearly never true and should be folded. And in the other case, > you are assigning 32-bit unsigned value into 64-bit signed var and then > testing if it is negative, again, that is never true. Ah right, thanks for pointing it out! Regards, Prathamesh
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 --- Comment #16 from prathamesh3492 at gcc dot gnu.org --- Created attachment 40139 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=40139&action=edit fix formatting of patch in comment 13 Done formatting changes in this version. Does it look OK ? Thanks, Prathamesh
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 --- Comment #15 from Jakub Jelinek --- For the tests, I think they would just pass on VMS. In one case you are comparing if 0x7fffULL <= (unsigned int) something, that is for 32-bit int clearly never true and should be folded. And in the other case, you are assigning 32-bit unsigned value into 64-bit signed var and then testing if it is negative, again, that is never true.
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501
--- Comment #14 from Jakub Jelinek ---
Comment on attachment 40137
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=40137
check precision of ptrdiff_type_node and lhs type is equal
Formatting is wrong. && should not be at the end of line, and as the condition
doesn't fit on one line, each subcondition should be on a separate line.
Also, the outer {} look ugly. Perhaps:
case CFN_BUILT_IN_STRLEN:
if (tree lhs = gimple_call_lhs (stmt))
if (ptrdiff_type_node
&& TYPE_PRECISION (ptrdiff_type_node) == TYPE_PRECISION (TREE_TYPE
(lhs)))
{
...
return;
}
break;
?
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 --- Comment #13 from prathamesh3492 at gcc dot gnu.org --- Created attachment 40137 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=40137&action=edit check precision of ptrdiff_type_node and lhs type is equal Hi Jakub, Thanks for the suggestion. Does this patch (stage-1 built) look OK ? However the tests pr78153-1.c and pr78153-2.c are bound to fail if type_precision (ptrdiff_t) != type_precision (size_t). What effective target check should I add to skip the test for targets like VMS where the precisions would be different ? Thanks, Prathamesh
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 --- Comment #12 from Jakub Jelinek --- Or perhaps check ptrdiff_type_node != NULL && TYPE_PRECISION (ptrdiff_type_node) == TYPE_PRECISION (size_type_node) before trying to optimize this? I mean, on VMS it might be fine if strlen returns 0xfffe (maximum size_t minus 1), if the address space is 64-bit and so is ptrdiff_t. So the above would only optimize on sane targets. The other uses of ptrdiff_type_node in the middle-end, which need fixing anyway, would need something like your patch, but not sure if it is not a waste of time to compute it if the C/C++ FE will immediately override it anyway. So perhaps just compute it that way in the LTO FE? I mean, for the *printf warning/length stuff, those calls shouldn't appear in Ada/Go/Fortran code, they can in LTO or C-family. The gimple-ssa-sprintf.c code could just check if ptrdiff_type_node is non-NULL and if it is NULL, punt.
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 --- Comment #11 from prathamesh3492 at gcc dot gnu.org --- (In reply to Jakub Jelinek from comment #10) > Comment on attachment 40136 [details] > move ptrdiff_type_node initialization to build_common_tree_nodes > > So, what do you want to do for VRP in the vms case where size_t is unsigned > int (32-bit) and ptrdiff_t is long long int (64-bit)? Shall we use VARYING > range in that case, or 0 to SIZE_MAX / 2, something different? > What about the hypothetical case where size_t would be larger than ptrdiff_t? For CFN_BUILT_IN_STRLEN, we have: tree type = TREE_TYPE (gimple_call_lhs (stmt)); tree max = vrp_val_max (ptrdiff_type_node); wide_int wmax = wi::to_wide (max, TYPE_PRECISION (TREE_TYPE (max))); tree range_min = build_zero_cst (type); tree range_max = wide_int_to_tree (type, wmax - 1); If ptrdiff_t > size_t, I suppose max would be truncated to PTRDIFF_MAX / 2, Since the type of range_max is type of lhs ? In ptrdiff_t < size_t, ah it would be incorrect to use upper bound as PTRDIFF_MAX -1 since the range could be [0, SSIZE_MAX - 1] ? So instead of PTRDIFF_MAX, I guess it would be better to set range to [0, max(signed_type_for (type) - 1] which would cover both cases. Ah I see this is what you suggested in comment 4, thanks! However regardless of this bug, should we be moving ptrdiff_type_node to middle-end ? As pointed out above lto/lto-lang.c has: ptrdiff_type_node = integer_type_node, which is probably incorrect. Thanks, Prathamesh
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 --- Comment #10 from Jakub Jelinek --- Comment on attachment 40136 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=40136 move ptrdiff_type_node initialization to build_common_tree_nodes So, what do you want to do for VRP in the vms case where size_t is unsigned int (32-bit) and ptrdiff_t is long long int (64-bit)? Shall we use VARYING range in that case, or 0 to SIZE_MAX / 2, something different? What about the hypothetical case where size_t would be larger than ptrdiff_t?
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 --- Comment #9 from prathamesh3492 at gcc dot gnu.org --- Created attachment 40136 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=40136&action=edit move ptrdiff_type_node initialization to build_common_tree_nodes Hi Richard, Thanks for the suggestion. I have attached untested patch for moving ptrdiff_type_node initialization to the build_common_tree_nodes. Does it look OK ? Validation in progress. Thanks, Prathamesh
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 --- Comment #8 from Jakub Jelinek --- (In reply to Jakub Jelinek from comment #7) > case FMT_LEN_z: > dirtype = sign ? signed_type_for (size_type_node) : size_type_node; > break; > > case FMT_LEN_t: > dirtype = sign ? ptrdiff_type_node : unsigned_type_for > (ptrdiff_type_node); > break; Or maybe even better: dirtype = signed_or_unsigned_type_for (!sign, size_type_node); and dirtype = igned_or_unsigned_type_for (!sign, ptrdiff_type_node); sizeof (size_t) == sizeof (ptrdiff_t) can't be assumed. E.g. vms uses always 32-bit size_t, but either 32-bit or 64-bit ptrdiff_t.
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 --- Comment #7 from Jakub Jelinek --- (In reply to Richard Biener from comment #3) > (In reply to prathamesh3492 from comment #2) > > Hi, > > From the trace, it seems ptrdiff_type_node is set to NULL for ada ? > > Either we can guard setting range for CFN_BUILTIN_STRLEN by checking > > ptrtdiff_type_node is non null, or make vrp_val_max return NULL if type is > > NULL. > > I am trying to reproduce on x86_64-unknown-linux-gnu. > > > > Thanks, > > Prathamesh > > Looks like gimple-ssa-sprintf.c also unconditionally uses it. > > I suppose we should move its initialization to build_common_tree_nodes, > it's built from the PTRDIFF_TYPE target macro. See how size_type_node is > built. > Also move unsigned_ptrdiff_type_node. The way gimple-ssa-sprintf.c uses it looks just wrong to me. case FMT_LEN_z: dirtype = sign ? ptrdiff_type_node : size_type_node; break; case FMT_LEN_t: dirtype = sign ? ptrdiff_type_node : size_type_node; break; From what I can see in POSIX, it should be case FMT_LEN_z: dirtype = sign ? signed_type_for (size_type_node) : size_type_node; break; case FMT_LEN_t: dirtype = sign ? ptrdiff_type_node : unsigned_type_for (ptrdiff_type_node); break;
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 --- Comment #6 from Richard Biener --- (In reply to Jakub Jelinek from comment #4) > Even for Fortran and other non-C FEs I think ptrdiff_type_node isn't what > you expect it to be. Wouldn't it be better to use signed_type_for (type) as > the type instead (where type is the type of strlen's lhs, i.e. usually > size_t)? As ptrdiff_type_node is appearantly used (otherwise we'd not have the LTO variant) it should be a middle-end defined type (it's defined by the target via PTRDIFF_TYPE).
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 --- Comment #5 from Richard Biener --- Ugh, and lto does lto/lto-lang.c: ptrdiff_type_node = integer_type_node; (bogus, possibly causes wrong-code if it's ever used, make sure to remove that init as well)
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 --- Comment #4 from Jakub Jelinek --- Even for Fortran and other non-C FEs I think ptrdiff_type_node isn't what you expect it to be. Wouldn't it be better to use signed_type_for (type) as the type instead (where type is the type of strlen's lhs, i.e. usually size_t)?
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 --- Comment #3 from Richard Biener --- (In reply to prathamesh3492 from comment #2) > Hi, > From the trace, it seems ptrdiff_type_node is set to NULL for ada ? > Either we can guard setting range for CFN_BUILTIN_STRLEN by checking > ptrtdiff_type_node is non null, or make vrp_val_max return NULL if type is > NULL. > I am trying to reproduce on x86_64-unknown-linux-gnu. > > Thanks, > Prathamesh Looks like gimple-ssa-sprintf.c also unconditionally uses it. I suppose we should move its initialization to build_common_tree_nodes, it's built from the PTRDIFF_TYPE target macro. See how size_type_node is built. Also move unsigned_ptrdiff_type_node.
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 --- Comment #2 from prathamesh3492 at gcc dot gnu.org --- Hi, From the trace, it seems ptrdiff_type_node is set to NULL for ada ? Either we can guard setting range for CFN_BUILTIN_STRLEN by checking ptrtdiff_type_node is non null, or make vrp_val_max return NULL if type is NULL. I am trying to reproduce on x86_64-unknown-linux-gnu. Thanks, Prathamesh
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 Dominique d'Humieres changed: What|Removed |Added Target|*-*-solaris2.* |*-*-solaris2.* ||x86_64-apple-darwin16 Status|UNCONFIRMED |NEW Last reconfirmed||2016-11-24 Host|*-*-solaris2.* |*-*-solaris2.* ||x86_64-apple-darwin16 Ever confirmed|0 |1 Build|*-*-solaris2.* |*-*-solaris2.* ||x86_64-apple-darwin16 --- Comment #1 from Dominique d'Humieres --- Also seen on x86_64-apple-darwin16.
[Bug middle-end/78501] [7 regression] SEGV in vrp_val_max
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78501 Rainer Orth changed: What|Removed |Added Target Milestone|--- |7.0
