[11] RFR (XXS): 8206476: Wrong assert in phase_enum_2_phase_string() in referenceProcessorPhaseTimes.cpp

[Erik Helin]

On 07/09/2018 12:13 PM, Thomas Schatzl wrote:
> Hi all,
> 
>    can I have reviews to this issue found with our C++ source code
> analysis tool that complains about one assert not being strict enough.
> 
> In particular:
> 
> static const char*
> phase_enum_2_phase_string(ReferenceProcessor::RefProcPhases phase) {
>    assert(phase >= ReferenceProcessor::RefPhase1 && phase <=
> ReferenceProcessor::RefPhaseMax,
>           "Invalid reference processing phase (%d)", phase);
>    return PhaseNames[phase];
> }
> 
> The second "<=" should be a "<".
> 
> Actually there is an existing (correct) macro for the whole assert.
> Replaced that line with the macro as follows:
> 
> @@ -80,8 +80,7 @@
>   STATIC_ASSERT((REF_PHANTOM + 1) == ARRAY_SIZE(ReferenceTypeNames));
>   
>   static const char*
> phase_enum_2_phase_string(ReferenceProcessor::RefProcPhases phase) {
> -  assert(phase >= ReferenceProcessor::RefPhase1 && phase <=
> ReferenceProcessor::RefPhaseMax,
> -         "Invalid reference processing phase (%d)", phase);
> +  ASSERT_PHASE(phase);
>     return PhaseNames[phase];
>   }
> 
> There is no actual failure, and there are no known failures with the
> change either; the reason for putting this into 11 is to get rid of
> unnecessary noise in source code analysis tool results.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8206476
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8206476/webrev/index.html

Looks good, Reviewed.

Thanks,
Erik

> Testing:
> hs-tier1-3,jdk-tier1
> 
> Thanks,
>    Thomas
>