[SERVER-51231] suspicious long long to double conversions in bounds checks Created: 30/Sep/20 Updated: 27/Oct/23 Resolved: 23/May/22 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Billy Donahue | Assignee: | Billy Donahue |
| Resolution: | Gone away | Votes: | 0 |
| Labels: | internal-sa-move | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||
| Operating System: | ALL | ||||||||||||||||
| Steps To Reproduce: | buildscripts/scons.py --ssl --variables-files=etc/scons/xcode_macosx.vars --libc++ --detect-odr-violations --dbg=on --opt=off --ninja --build-tools=next VARIANT_DIR=ninja generate-ninja Emits warnings treated as errors. |
||||||||||||||||
| Sprint: | Service Arch 2022-2-21, Service Arch 2022-03-07, Service Arch 2022-03-21, Service Arch 2022-04-04, Service Arch 2022-04-18, Service Arch 2022-05-02, Service Arch 2022-05-16, Service Arch 2022-05-30 | ||||||||||||||||
| Participants: | |||||||||||||||||
| Story Points: | 2 | ||||||||||||||||
| Description |
|
(found by XCode12 clang warnings which appear to be true positives) We have several scenarios where double values are compared to LONG_LONG_MAX to determine whether they are representable. The problem is LONG_LONG_MAX (2^63 - 1) doesn't cleanly map to any double, and it could be rounded up to the double representation of LONG_LONG_MAX+1, (2^63), before comparison. There could be valuable bugs here, as double values near LONG_LONG_MAX will appear unrepresentable to coerce and formatting functions when they actually are fine, or vice versa, depending on the code and relops in question. We have unit tests with the same bug, including safe_num_test, so it may look like these converters are coming up with the right answer and passing unit tests when they aren't actually correct. summation_test.cpp etc don't cover the highest values of the long long range very well and have the same casting errors. It would be nice to see if we could craft some custom-built tests that fail because of these casts. I haven't tried that. Not for review yet, but this CR shows where the warnings are. A few other minor tweaks are in the CR. Ignore those. AC: Investigate within the time-boxed story point time. |
| Comments |
| Comment by Billy Donahue [ 23/May/22 ] | ||||||||||||
|
XCode12 is the production tool now, so these issues were fixed while rolling it out. | ||||||||||||
| Comment by Billy Donahue [ 30/Sep/20 ] | ||||||||||||
|
The problem illustrated. https://gcc.godbolt.org/z/fv3369
| ||||||||||||
| Comment by Billy Donahue [ 30/Sep/20 ] | ||||||||||||
|
|