RFR: 8374445: Fix -Wzero-as-null-pointer-constant warnings in JfrSet
Kim Barrett
kbarrett at openjdk.org
Thu Jan 15 16:24:45 UTC 2026
On Wed, 14 Jan 2026 15:11:59 GMT, Markus Grönlund <mgronlun at openjdk.org> wrote:
>> Please review this change to fix JfrSet to avoid triggering
>> -Wzero-as-null-pointer-constant warnings when that warning is enabled.
>>
>> The old code uses an entry value with representation 0 to indicate the entry
>> doesn't have a value. It compares an entry value against literal 0 to check
>> for that. If the key type is a pointer type, this involves an implicit 0 =>
>> null pointer constant conversion, so we get a warning when that warning is
>> enabled.
>>
>> Instead we initialize entry values to a value-initialized key, and compare
>> against a value-initialized key. This changes the (currently undocumented)
>> requirements on the key type. The key type is no longer required to be
>> trivially constructible (to permit memset-based initialization), but is now
>> required to be value-initializable. That's currently a wash, since all of the
>> in-use key types are fundamental types (traceid (u8) and Klass*).
>>
>> Testing: mach5 tier1-3 (tier3 is where most jfr tests are run)
>
> src/hotspot/share/jfr/utilities/jfrSet.hpp line 72:
>
>> 70: }
>> 71: for (unsigned i = 0; i < table_size; ++i) {
>> 72: ::new (&table[i]) K{};
>
> Is this the new (placement pun intended) way to do a placement new, using the outer scope operator ::? Or is it because we don't know what Hotspot type this is?
It's the same old way one should always do it. If one wants global placement
new, one should say so. An unqualified `new` expression does a class-based
lookup of `operator new`, so if the class has one (and lots of ours do), that
will be used. We don't want that here, regardless of the type of `K`. As it
happens, for all current uses `K` is a fundamental type, so it doesn't matter.
But it's clearer and future proof to be explicit.
> src/hotspot/share/jfr/utilities/jfrSet.hpp line 142:
>
>> 140: for (unsigned i = 0; i < old_table_size; ++i) {
>> 141: const K k = old_table[i];
>> 142: if (k != K{}) {
>
> Are these K{}'s compile constant expressions?
For the types currently used, yes. This is a "value-initialized" (C++17
11.6/8) temporary. For fundamental types, that's a zero-initialized
temporary.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29022#discussion_r2695057339
PR Review Comment: https://git.openjdk.org/jdk/pull/29022#discussion_r2695088728
More information about the hotspot-jfr-dev
mailing list