RFR: 8313564: Fix -Wconversion warnings in classfile code [v4]

[Coleen Phillimore]

On Thu, 3 Aug 2023 10:27:57 GMT, Dean Long <dlong at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix Atomic items_added code.
>
> src/hotspot/cpu/x86/vm_version_x86.cpp line 1808:
> 
>> 1806: 
>> 1807:   // Allocation prefetch settings
>> 1808:   int cache_line_size = checked_cast<int>(prefetch_data_size());
> 
> I don't see why the old code would produce a warning, except with -Wsign-conversion on 32-bit platforms.

I put the cast in because prefetch_data_size() returns uint and I'm narrowing it to int which has a lower positive range.  I assume checked_cast would find that.

> src/hotspot/share/classfile/classFileParser.cpp line 973:
> 
>> 971:   void remove_annotation(ID id) {
>> 972:     assert((int)id >= 0 && (int)id < (int)_annotation_LIMIT, "oob");
>> 973:     _annotations_present &= (int)~nth_bit((int)id);
> 
> This silences the warning but without a runtime check.  It would be nice to have a constexpr template version of nth_bit that return the desire type and also do a range check on the input, unlike this macro version always returns intx and returns 0 if the shift is out of range.

Let's fine a separate RFE for nth_bit and fix all of these at once.  This appears a lot.

> src/hotspot/share/classfile/stringTable.cpp line 245:
> 
>> 243: 
>> 244: void StringTable::item_added() {
>> 245:   Atomic::inc(&_items_count);
> 
> Can this overflow?

Yes, I'm going to revert the _items_count change.

> src/hotspot/share/classfile/symbolTable.cpp line 90:
> 
>> 88: static size_t _current_size = 0;
>> 89: 
>> 90: static volatile int    _items_count = 0;
> 
> Shouldn't there be a check somewhere that this value doesn't overflow?

Yes, there should be here too.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15111#discussion_r1283108957
PR Review Comment: https://git.openjdk.org/jdk/pull/15111#discussion_r1283109704
PR Review Comment: https://git.openjdk.org/jdk/pull/15111#discussion_r1283110162
PR Review Comment: https://git.openjdk.org/jdk/pull/15111#discussion_r1283113986