RFR(M): 8207049: Minor improvements of compiler code.

[Vladimir Kozlov]

Nice changes, Goetz

I agree with changes. Let me test them.

Thanks,
Vladimir

On 7/11/18 7:23 AM, Lindenmaier, Goetz wrote:
> Hi,
> 
> I ran coverity on the jdk11 hotspot sources and want to propose the following fixes. 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.  If you consider some too conservative, I'm happy to remove them.
> 
> http://cr.openjdk.java.net/~goetz/wr18/8207049-covCompiler/01/
> 
> Real issues
> -----------
> 
> x86.ad
>    For Op_CMoveVF the code always returned 'false'.
> 
> compileLog.cpp
>    read returns -1 on unsuccesful completion. Use a
>    singned variable and check for error before
>    subtracting -1 from to_read etc.
> 
> disassembler.cpp
>    check for _library.
> 
> jvmciCompilerToVM.cpp, getResolvedJavaType
>    base_object is checked for null above, so
>    check before printing the name, too.
> 
> simpleThresholdPolicy.cpp
>    If I understand correctly, if the compile queue is empty,
>    no max_task will be found. This is quite unlikely, though.
> 
> 
> 
> Useful code improvements:
> -------------------------
> 
> codeBuffer.cpp
>    Above, next_cs is checked for NULL. I would assume this
>    check is boguous, but checking below, too, is on the
>    safe side.
> 
>    The codebuffer should be aligned properly, so the
>    first section does not need alignment.
>    Make this more clear in the code. Else it looks
>    as if prev_dest_cs is dereferenced when NULL.
> 
> c1_IR.cpp
>    It's not obvious that n->subst() != n if false
>    on the first iteration.
> 
> c1_LinearScan.cpp
>    spill_candidate is used as index to an array in
>    _mapping_from.at() and thus should not be negative.
> 
>    prev_cmp is dereferenced further down. change to guarantee.
> 
> codeCache.cpp
>    Use format specifyer to print strings.
> 
> methodLiveness.cpp
>    Turn assert into guarantee().
> 
> oopMap.cpp
>    Change loop so that last is always initilaized.
>    last gets dereferenced below.
> 
> 
> jvmciCodeInstaller.cpp
>    DebugInformationRecorder::describe_scope might pass NULL.
> 
> arraycopy.cpp
>    If c == NULL the condition of the next if will crash.
>    Change to guarantee.
> 
> bytecodeInfo.cpp
>    If callee_method ever had been NULL, we would have
>    seen a crach or assertion in post_inlining_event.
> 
> callnode.cpp
>    Would fail if IR is broken.
> 
> compile.cpp
>    Don't access arrays at negative indexes.
> 
> gcm.cpp
>    There must be an LCA, as there should be at least one
>    ancestor.
> 
> ifnode.cpp
>    It's unclear that a proj is found in if (vop == Op_Phi)
> 
> indexSet.cpp
>    Wrong data type used.
> 
> lcm.cpp
>    Turn assertion into guarantee, don't access worklist at -1.
> 
> loopPredicate.cpp
>    limit is dereferenced by _igvn.type(limit)
> 
> loopTransform.cpp
>    if limit_n == NULL, we will skip the following if and do _igvn.type(limit_n)
>    either return false, a guarantee or assert would do, too, I think.
> 
> macro.cpp
>    igvn.replace_node(fallthroughproj, ctrl) would fail if ctrl == NULL.
> 
> memnode.cpp
>    igvn is checked for NULL above, too. Protect all uses similarly.
> 
>    In the default case load == NULL would be dereferenced below.
> 
> node.hpp
>    arr is checked for NULL above. Maybe remove (arr == NULL) check above?
> 
> output.cpp
>    turn assertion into guarantee
> 
> parse1.cpp
>    turn assertion into guarantee
> 
> reg_split.cpp
>    turn assertion into guarantee
> 
> runtime.cpp
>    print "no method" if m == NULL (assigned NULL above).
> 
> type.cpp
>    One of the if blocks should be reached in any case.
>    Change else if into a guarantee.
> 
> 
> It's obviously pointless to scan adlc, but fixing the
> issues does not hurt either I think.
> 
> archDesc.cpp
>    Above, node is checked for NULL.
> 
> arena.cpp
>    memset writes bytes.
> 
> dfa.cpp, valid_loc():
>    breaks are missing.
> dfa.cpp, compute_external():
>    Use strncat (should also use snprintf, but we still have
>    VS 2013 which does not support that)
> 
> filebuff.cpp
>    breaks are missing.
> 
> formssel.cpp
>    name_left was just checked for NULL.
>    form2 is checked for NULL above, so check needed here, too.
> 
> main.cpp
>    Wrong delete operator used.
> 
> output_c.cpp
>    We cannot continue after this error, as below
>    inst_rep_var is dereferenced.
>    opc is assigned NULL if local == NULL. (in both cases).
>