Opened 12 years ago
Closed 11 years ago
#1984 closed defect (fixed)
geography_btree.c: array subscript is above array bounds [-Warray-bounds]
Reported by: | strk | Owned by: | pramsey |
---|---|---|---|
Priority: | medium | Milestone: | PostGIS 2.1.0 |
Component: | postgis | Version: | master |
Keywords: | history | Cc: |
Description
With gcc 4.6.3:
geography_btree.c: In function ‘geography_lt’: geography_btree.c:38:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:38:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c: In function ‘geography_le’: geography_btree.c:38:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:38:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c: In function ‘geography_gt’: geography_btree.c:38:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:38:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c: In function ‘geography_ge’: geography_btree.c:38:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:38:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c: In function ‘geography_eq’: geography_btree.c:38:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:38:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c: In function ‘geography_cmp’: geography_btree.c:38:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:38:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:39:33: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:9: warning: array subscript is above array bounds [-Warray-bounds] geography_btree.c:40:33: warning: array subscript is above array bounds [-Warray-bounds]
Worth checking
Change History (11)
comment:1 by , 12 years ago
comment:2 by , 12 years ago
Yes, could just convert the macros to use pointer arithmetic instead of subscripts.
comment:3 by , 12 years ago
Milestone: | PostGIS 2.0.2 → PostGIS 2.1.0 |
---|
sounds too dangerous to fix. Push back if you feel otherwise.
comment:4 by , 12 years ago
The code itself is perfectly legal and good and well tested, it's just a compiler warning. Changing the code is dangerous, since it works fine. Using some other C construct to express the intent might quiet the warning on the newer gcc compilers but break our compilation on other ones. Definitely a 2.1 thing.
comment:5 by , 12 years ago
Milestone: | PostGIS 2.1.0 → PostGIS Future |
---|
How about we make this a PostGIS Future thing
comment:6 by , 11 years ago
Okay - pointer/cast workaround committed to trunk. Feel free to close out in a couple of weeks assuming we've had no complaints.
comment:7 by , 11 years ago
Keywords: | history added |
---|---|
Milestone: | PostGIS Future → PostGIS 2.1.0 |
committed at r11508
this really should have waited till we branched and trunk became 2.2 (oh well).
comment:8 by , 11 years ago
robe: I think we should ship a 2.1.0 that builds with no compiler warnings, really
comment:9 by , 11 years ago
my qualm is not about that as we already called beta and we had 7 months to do this before beta was called. This looks to me like something that may fix compiler warnings and cause other issues so really should have undergone more testing before it was shoved in at the last minute.
comment:10 by , 11 years ago
I appreciate that we probably should have done this earlier during the cycle, but then again the excessive warnings have unmasked at least one potential crash bug that we should fix.
In terms of risk, I'd say it's reasonably low: it's two self-contained macros that are used in several critical places during the index code (and it's really just changing the arithmetic to use direct pointer arithmetic with no logic change) and so if I made a mistake it would likely crash fairly quickly. I also introduced deliberate errors to both the pre- and post- patch version and confirmed that the regression tests failed in exactly the same way by diffing the files involved so it's already had a pretty good work out.
Apologies if you feel slightly put out by this, but I genuinely feel that the benefits will far outweigh the drawbacks in this particular case.
comment:11 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
okay this is done right. I think s.
Ok the issue here is GIDX→c being defined as a 1-element array, while we know it's of variable size instead.
One way to fix this would be changing its definition from:
To:
But doing so would change sizeof(GIDX) to be one float less than possibly expected by client code. This may actually simplify some code, but doesn't look like we have much testing for that (nothing in the testsuite fails with the above changes).
Another way could be doing the cast from within the GIDX_* macros which are using subscripts. Probably safer.