Uploaded image for project: 'Core Server'
  1. Core Server
  2. SERVER-51231

suspicious long long to double conversions in bounds checks

    • Type: Icon: Bug Bug
    • Resolution: Gone away
    • Priority: Icon: Major - P3 Major - P3
    • None
    • Affects Version/s: None
    • Component/s: None
    • ALL
    • Hide

      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.
      Reviewing these shows what look like incorrect range checks.

      Show
      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. Reviewing these shows what look like incorrect range checks.
    • 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
    • 2

      (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.
      So a double having the value LONG_LONG_MAX+1, or perhaps even LONG_LONG_MAX+1000 will pass a range check because the 1001 of error is still beyond a double's 53 bit precision.

      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.
      https://mongodbcr.appspot.com/663460026/

      AC: Investigate within the time-boxed story point time.

            Assignee:
            billy.donahue@mongodb.com Billy Donahue
            Reporter:
            billy.donahue@mongodb.com Billy Donahue
            Votes:
            0 Vote for this issue
            Watchers:
            10 Start watching this issue

              Created:
              Updated:
              Resolved: