> 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 refreshed the contents of this pull request, and previous
commits have been removed. The incremental views will show differences compared
to the previous content of the PR. The pull request contains seven new commits
since the last revision:
- Simplify findEntry loop: use for with direct width increment
Change from while loop to for loop, moving the width increment
directly into the loop body for better readability.
Co-Authored-By: rgiulietti
- Refactor findEntry loop to use while with direct width increment
Change the loop from for-with-post-increment to a while loop that
increments by e.width() directly, making the code clearer.
- Add tests for Float/Double NaN with different bit patterns
Since the code uses raw bit variants (floatToRawIntBits/doubleToRawLongBits)
when converting floating-point values to integral bits for hashCode and
equals, the tests should cover the cases of NaNs with different bit patterns.
- testFloatNaNWithDifferentBitPatternsAreDistinct: Verify that NaNs with
different bit patterns create distinct constant pool entries
- testDoubleNaNWithDifferentBitPatternsAreDistinct: Same for Double
- testFloatNaNWithDifferentBitPatternsAreNotEqual: Verify that NaNs with
different bit patterns are not equal
- testDoubleNaNWithDifferentBitPatternsAreNotEqual: Same for Double
- testFloatNaNWithSameBitPatternIsDeduplicated: Verify that NaNs with the
same bit pattern are deduplicated
- testDoubleNaNWithSameBitPatternIsDeduplicated: Same for Double
- 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.
- 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.
- Add tests for Float/Double NaN constant pool equality and deduplication
Test coverage for NaN and +/-0.0 handling in constant pool entries:
- NaN entries are deduplicated (same index from same builder)
- NaN entries from different pools are equals()
- +0.0 and -0.0 are kept as distinct entries
- Normal float/double deduplication still works
- NaN values survive class file round-trip and rebuild
- Fix Float/Double NaN constant pool entry equality and deduplication
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.
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/30196/files
- new: https://git.openjdk.org/jdk/pull/30196/files/34f4d54e..fd2dd7a0
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=30196&range=04
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=30196&range=03-04
Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod
Patch: https://git.openjdk.org/jdk/pull/30196.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/30196/head:pull/30196
PR: https://git.openjdk.org/jdk/pull/30196
