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

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Wed Jul 11 06:55:44 UTC 2018


Hi Lois, 

> > + name ? name->as_C_string() : "<unnamed>");
>  Instead of "<unnamed>" please use the UNNAMED_MODULE macro from
> moduleEntry.hpp.
Oh yes, sure!

> > This looks a lot nicer!   Similar code is in linkResolver.cpp, can you
> > look at changing it too?
> 
> I have an RFR out currently for JDK-8205611, (see
> http://mail.openjdk.java.net/pipermail/hotspot-dev/2018-
> June/033325.html),
> which needs one more reviewer's okay.  It contains changes to reword the
> error messages for loader constraint violations in order to follow the
> new proposed format for module and class loader information.  So our two
> changes will conflict in this area.
Your change is targeted at 12, mine to 11.  Changing this to use
stringStream will also simplify your change. Do you mind me
fixing that in 11, and you waiting with pushing until it's merged to 12?
I need to make a coverity change for the compiler sources, 
but after that I'll write a review for your change.

Best regards,
  Goetz.


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