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

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Thu Jul 12 09:02:42 UTC 2018


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.

> 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.



More information about the hotspot-runtime-dev mailing list