JDK-8314258 : checked_cast doesn't properly check some cases
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 22
  • Priority: P4
  • Status: Open
  • Resolution: Unresolved
  • Submitted: 2023-08-15
  • Updated: 2024-05-13
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
tbdUnresolved
Related Reports
Relates :  
JDK-8331799 - ubsan: g1Policy.cpp:1114:33: runtime error: 2.71198e+167 is outside the range of representable values of type 'const unsigned long'
Relates :  
JDK-8314694 - Separate checked_cast from globalDefinitions.hpp
Description
The round-trip test currently used by checked_cast doesn't work in some cases.  For example:

checked_cast<uint>(SIZE_MAX) => error as expected
checked_cast<int>(SIZE_MAX) => no error? 

The problem is that SIZE_MAX successfully round-trips because
static_cast<size_t>(-1) => SIZE_MAX
due to sign extension when promoting the type.
Comments
A pull request was submitted for review. URL: https://git.openjdk.org/jdk/pull/16005 Date: 2023-10-02 03:10:29 +0000
16-10-2023
For one of the -Wconversion changes, I ended up having to do this: // sign-extended tolerant cast needed by callers of emit_int8 and emit_int16 // Some callers pass signed types that need to fit into the unsigned type so check // that the range is correct. template <typename T> constexpr T narrow_cast(int x) const { if (x < 0) { using stype = std::make_signed_t<T>; assert(x >= std::numeric_limits<stype>::min(), "too negative"); // >= -128 for 8 bits return static_cast<T>(x); // cut off sign bits } else { return checked_cast<T>(x); } } I don't know if a page of template code is going to help this situation or not, to be honest. The checked_cast additions for the places where the compiler complains about -Wconversion are useful as they verify that the type fits (via round trip conversion). I complained to [~kbarrett] about this privately because I really hate the assert message and was hoping there was something that could be done about that with more template magic.
24-08-2023
While being unchanged by a round-trip is what the description currently says, that isn't how it's being used, and I claim isn't particularly useful. The way it's being used is to static cast if the value is in the new type's range, else complain. That is, it's addressing overflow errors. Many uses are to silence -Wconversion warnings that we think aren't a problem, with a (debug-time) check for that.
23-08-2023
That is to say, for clarity: checked_cast is checking for loss of information. If the round trip has succeeded, information has not been lost.
23-08-2023
checked_cast is defined as checking that the round trip succeeds. It can't be said that it doesn't work. checked_cast<int>(1.5) is supposed to fail, and rightly so. I'd be leery about making checked_cast more elaborate in order to deal with corner cases. From what I've seen, each attempt brings up more corner cases and we may end up with something difficult to verify and to understand. With regard to UB, that is (as you say) inherited from C++ floating-integral conversion. It might well be worth defining a saturating_cast and using that instead of static_cast in order to avoid the UB.
23-08-2023
Another problem with the current round-trip test is that we get undefined behavior if converting a floating point value and the value is outside the range of values for the destination type (whether the destination type is integral or floating point). So `static_cast<int>(std::numeric_limits<double>::max())` is UB.
21-08-2023
checked_cast also doesn't really support float => integral conversions, as it will fail if the float has a non-zero fractional part. For that category I think we want to quietly discard any fractional part, and only check for magnitude issues.
15-08-2023