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