| Summary: | incorrect "array subscript is below/above array bounds" diagnostic | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Robert Schiele <rschiele> |
| Component: | middle-end | Assignee: | Not yet assigned to anyone <unassigned> |
| Status: | RESOLVED WORKSFORME | ||
| Severity: | minor | CC: | jakub, msebor |
| Priority: | P3 | Keywords: | diagnostic |
| Version: | 5.0 | ||
| Target Milestone: | --- | ||
| See Also: | https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63477 | ||
| Host: | Target: | ||
| Build: | Known to work: | 5.4.0, 6.4.0, 7.1.0, 8.0 | |
| Known to fail: | Last reconfirmed: | ||
| Bug Depends on: | |||
| Bug Blocks: | 56456 | ||
I think it is just fine to diagnose this. I still see the warning with today's top of trunk (GCC 4.8) but I agree that some sort of a diagnostic here seems helpful. It would be nice if the warning could distinguish between cases where the index definitely is out of bounds and those where it could be but it's not 100% certain. I haven't looked at the implementation to see if this is feasible. It would also be helpful to have clarity (both for users and GCC developers) on what diagnostics are meant to be issued under what circumstances. Based on some other warnings (-Wuninitialized and -Wmaybe-uninitialized) it seems reasonable to expect -Warray-bounds to only be issued when an index is definitely out of bounds (and not when it could be). Just like with pr63477, I cannot reproduce the warning with this test case in GCC 8.0 anymore (despite what I said in comment #2), or 7-branch, or 6-branch, or even 5-branch. Just like in pr63477, bisection also points to r220157 (gcc 5.0.0) as the fix. So I'll assume that what I saw in comment #2 was some transient fluke and resolve this as worksforsome (based on comment #1 and #2 saying it would make sense to diagnose this). |
I found a case where gcc diagnostic "array subscript is below/above array bounds" is wrong in my opinion. I simplified the test case as much as possible and got it down to: typedef struct { int a[2]; int b; } c; int f(c *d, int e) { int r = 0; int p; int i; switch(e) { case 0: i = 0; break; default: i = -1; break; }; p = d->a[i]; if (p > 0) r = 0; else { switch(e) { case 0: r = 1; break; } } return r; } The basic idea of the function f was that the parameter e is only allowed to get a certain set of permitted values. In that example the set was reduced to only 0 being the permitted value (which obviously does no longer make a lot of sense but the original code with a larger set did show the same problem). Based on that value a specific action is taken in the first switch statement and a default case is given, which sets the value i to an invalid one (-1). I agree this is not particularly intelligent error handling but was found with that pattern in real code.) Later this value i is used for indexing an array. This is diagnosed by gcc to be below array bounds (or above for a value too high), which would obviously make sense for (i == -1) but on the other hand I don't see any reason gcc should assume this default case to ever happen. Further analysis reveals that this behavior appeared in the development phase of gcc 4.8 with this commit: commit 78b7a67520737f2e029383dd5a89ba8c1c4a3ef9 Author: steven <steven@138bc75d-0d04-0410-961f-82ee72b054a4> Date: Mon Jul 2 18:50:51 2012 +0000 gcc/ * stmt.c (emit_case_bit_tests): Remove. (expand_case): Remove expand_switch_using_bit_tests_p code. * tree-switch-conversion.c (hoist_edge_and_branch_if_true): New. (MAX_CASE_BIT_TESTS): Moved from stmt.c to here. (lshift_cheap_p): Likewise. (expand_switch_using_bit_tests_p): Likewise. (struct case_bit_test): Likewise. (case_bit_test_cmp): Likewise. (emit_case_bit_tests): New implementation for GIMPLE. (gen_inbound_check): Do not release post-dominator info here. (process_switch): Reorder code. Expand as bit tests if it looks like a win. (do_switchconv): Release post-dominator info here if something changed. (struct gimple_opt_pass): Verify more. * tree.h (expand_switch_using_bit_tests_p): Remove prototype. testsuite/ * gcc.dg/tree-ssa/pr36881.c: Fix test case to not expand as bit tests. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@189173 138bc75d-0d04-0410-961f-82ee72b054a4 This obviously does not necessarily mean that it was introduced here since this could have been just the trigger for that. Another interesting observation is that if I replace the second, not directly related switch statement with an if statement with the same semantics the obscure diagnostic goes away. It seems that this problem is independent of the architecture but I actually tested this on arm-*-linux-gnueabihf and i686-*-linux-gnu. Do people here agree this is a bug in diagnostics or do people think that it is legitimate to assume that the default case actually can happen?