On Thu, 12 Mar 2026 17:31:58 GMT, Shaojin Wen <[email protected]> wrote:
>> FloatEntryImpl.equals() and DoubleEntryImpl.equals() used == for comparison, >> which returns false for NaN == NaN per IEEE 754. This caused two different >> FloatEntry/DoubleEntry objects holding NaN to not be considered equal, >> violating the equals/hashCode contract (same hashCode but equals returns >> false). >> >> Similarly, SplitConstantPool.findFloatEntry() and findDoubleEntry() used == >> to match values, so NaN entries could never be found in the pool, causing >> every floatEntry(Float.NaN) or doubleEntry(Double.NaN) call to create a >> duplicate entry and bloat the constant pool. >> >> Fix by using Float.floatToRawIntBits() and Double.doubleToRawLongBits() for >> bitwise comparison, which correctly handles NaN and also properly >> distinguishes +0.0 from -0.0. > > Shaojin Wen has updated the pull request incrementally with two additional > commits since the last revision: > > - Fix SplitConstantPool lookup hash to match updated entry hashCode > > The previous commit updated FloatEntryImpl/DoubleEntryImpl to use > raw bits for hashCode, but findFloatEntry/findDoubleEntry still > used Float.hashCode/Double.hashCode for lookup. This mismatch would > cause non-canonical NaN entries to never be found in the map. > > Co-Authored-By: Claude Opus 4.6 <[email protected]> > - Use raw bits for Float/Double constant pool entry hashCode > > Update hashCode to use floatToRawIntBits/doubleToRawLongBits, > consistent with the equals methods, so that different NaN bit > patterns hash to different buckets instead of always colliding. > > Co-Authored-By: Claude Opus 4.6 <[email protected]> @wenshao Since the code makes use of the _raw_ variants when converting floating-point values to integral bits, maybe the tests should cover the cases of NaNs with different bit patterns. WDYT? test/jdk/jdk/classfile/ConstantPoolNaNTest.java line 210: > 208: private static <T extends PoolEntry> T findEntry(ClassModel cm, > Class<T> type) { > 209: var cp = cm.constantPool(); > 210: for (int i = 1; i < cp.size(); i++) { Suggestion: for (int i = 1; i < cp.size(); i += e.width()) { test/jdk/jdk/classfile/ConstantPoolNaNTest.java line 215: > 213: return (T) e; > 214: } > 215: i += e.width() - 1; Suggestion: ------------- PR Review: https://git.openjdk.org/jdk/pull/30196#pullrequestreview-3962786031 PR Review Comment: https://git.openjdk.org/jdk/pull/30196#discussion_r2948657861 PR Review Comment: https://git.openjdk.org/jdk/pull/30196#discussion_r2948658390
