RFR: 8365994: ZGC: Incorrect type signature in ZMappedCache comparator
Axel Boldt-Christmas
aboldtch at openjdk.org
Mon Aug 25 15:24:53 UTC 2025
On Mon, 25 Aug 2025 14:14:35 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:
>> Hello,
>>
>> The comparator with two IntrusiveRBNode* in ZMappedCache has the incorrent type signature. This prevents IntrusiveRBTree from using the comparator as intended during validation, resulting in it using the fallback verify function, which always returns true. This could mask potential issues in the tree structure.
>>
>> Testing:
>> * Manually placing an assert in the fallback verify function to see if it is used. It is no longer used with this patch.
>> * Oracle's tier 1-2
>
> src/hotspot/share/gc/z/zMappedCache.hpp line 44:
>
>> 42: struct EntryCompare {
>> 43: static int cmp(zoffset a, const IntrusiveRBNode* b);
>> 44: static bool cmp(const IntrusiveRBNode* a, const IntrusiveRBNode* b);
>
> I'd expect `cmp` to return `int`; otherwise, the `cmp` name is rather misleading. I wonder if rbtree can be changed so that all `cmp` methods return `int` instead.
I tend to agree, or rename it to `less`.
And I think `cmp` should even return a more strongly typed class, rather than `int` to discourage code like `return b - a;` as you have to be very careful to not introduce underflow arithmetic bugs. Maybe even take inspiration from `operator<=>`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26904#discussion_r2298416095
More information about the hotspot-gc-dev
mailing list