RFR(M): 8206977: Minor improvements of runtime code.

[Lois Foltan]

On 7/12/2018 5:02 AM, Lindenmaier, Goetz wrote:

> Hi Coleen,
>
> thanks for looking at my change.
>
> New webrev:
> http://cr.openjdk.java.net/~goetz/wr18/8206977-covRuntime/02/
>
>> + name ? name->as_C_string() : "<unnamed>");
>> Can you change to:
>> + name != NULL ? name->as_C_string() : "<unnamed>");
> Fixed.
>   
>> http://cr.openjdk.java.net/~goetz/wr18/8206977-
>> covRuntime/01/src/hotspot/share/oops/klassVtable.cpp.udiff.html
>> This looks a lot nicer!   Similar code is in linkResolver.cpp, can you
>> look at changing it too?
> I removed it and leave it to Lois.
Sounds good.  I will pick this change up for klassVtable.cpp and make 
similar changes to linkResolver.cpp.
Thanks!
Lois

>
>> http://cr.openjdk.java.net/~goetz/wr18/8206977-
>> covRuntime/01/src/hotspot/share/services/writeableFlags.cpp.udiff.html
>> If name is null here, what would this do?  Should there be an 'else' to
>> print something?
> I think the case with the null name is for "MISSING_NAME"
> and thus no message is needed here. (If range is null no
> message is printed either.)
>
> Best regards,
>    Goetz.
>
>
>> I think this looks fine.  It doesn't look major to me.  The asserts
>> turned to guarantees don't appear to be anywhere performance sensitive.
>>
>> Thanks,
>> Coleen
>>
>>
>>
>> On 7/10/18 6:53 AM, Lindenmaier, Goetz wrote:
>>> Hi,
>>>
>>> I ran coverity on the jdk11 hotspot sources and want to propose the
>>> following fixes to the runtime code. I scanned the linux x86_64 build.
>>> Some issues are similar to previous parfait fixes (check for NULL, add
>>> guarantees etc.) I also identified some issues I consider real problems.
>>> http://cr.openjdk.java.net/~goetz/wr18/8206977-covRuntime/01/
>>>
>>> In detail:
>>>
>>> Real issues:
>>> ------------
>>>
>>> jvmtiEnvBase.cpp
>>>     || should be &&.
>>>     Attention, this is the only change that really will change behaviour.
>>>     But if thr == NULL we will see a crash below.
>>>
>>> perfMemory_linux.cpp:
>>>     Wrong buffer length used.
>>>
>>> systemDictionary.cpp:
>>>     Move code dereferencing ik under if (ik != NULL).
>>>
>>> virtualspace.cpp
>>>     Initialization is missing. Moved constructor up to the other
>>>     constructors.
>>>
>>>
>>> Useful code improvements:
>>> -------------------------
>>>
>>> vm_version_ext_x86.cpp
>>>     Assure buffer is not accessed at offset -1.
>>>
>>> os_linux.cpp
>>>     Numa_max_node returns int, and a -1 in some cases.
>>>
>>> moduleEntry.cpp
>>>     name might be NULL. Just a fix for tracing.
>>>
>>> systemDictionaryShared.cpp
>>>     clearify code.
>>>     It would be wrong if only entry == NULL would hold, one
>>>     would hit the assertion below.
>>>
>>> verifier.cpp
>>>     Fix tracing.
>>>     Illegal opcode is -1 and should not be passed to name array.
>>>
>>> logOutput.cpp
>>>     If n_selections == 0, best_selection would be NULL.
>>>     Move up the assertion and turn into a guarantee.
>>>
>>> filemap.cpp
>>>     Either base can be NULL, or parts of the code before are dead.
>>>
>>> metaspace.cpp
>>>     We now an exception is pending.
>>>
>>> klassVtable.cpp
>>>     Coverity does not like the format in a variable.
>>>     Anyways this is quite rough coding, transformed to use stringStream
>>>     as with other similar exceptions.
>>>
>>> jvmFlag.cpp
>>>     match might be NULL.
>>>
>>> writableFlags.cpp
>>>     name might be NULL.
>>>
>>> ostream.cpp
>>>     If ftell returns error code -1, we need not continue.
>>>     Especially we should not fseek(-1).
>>>
>>> logTestUtils.inline.hpp
>>>     ftell returns -1.
>>>
>>> test_metachunk.cpp
>>>     wrong datatype.