RFR (S): 8218939: vm/mlvm/anonloader/stress/byteMutation crashed on windows

[David Holmes]

Thanks Igor. I'll push this now.

David

On 15/02/2019 12:59 pm, Igor Ignatyev wrote:
> 
> 
>> On Feb 14, 2019, at 5:26 PM, David Holmes <david.holmes at oracle.com 
>> <mailto:david.holmes at oracle.com>> wrote:
>>
>> On 15/02/2019 10:45 am, Igor Ignatyev wrote:
>>> I'm sorry I wasn't clear enough, I'm suggesting to harden 
>>> runtime/classFileParserBug/TestBadClassName.java test, not 
>>> vm/mlvm/anonloader/stress/byteMutation.
>>> in TestBadClassName.java, we should always get CFE, so we can assert 
>>> that it has the expected message.
>>
>> Ah! Sure. That would have caught this bug much earlier.
>>
>> http://cr.openjdk.java.net/~dholmes/8218939/webrev.v2/
> 
> LGTM.
> 
> -- Igor
>>
>> Thanks,
>> David
>>
>>> -- Igor
>>>> On Feb 14, 2019, at 4:36 PM, David Holmes <david.holmes at oracle.com 
>>>> <mailto:david.holmes at oracle.com>> wrote:
>>>>
>>>> Hi Igor,
>>>>
>>>> Thanks for looking at this.
>>>>
>>>> On 15/02/2019 10:20 am, Igor Ignatyev wrote:
>>>>> Hi David,
>>>>> the fix looks good to me. does it make sense to harden the test and 
>>>>> assert the message?
>>>>
>>>> In what way would you harden the test? It does random byte writes 
>>>> which could possibly still result in a legal classfile. You'd 
>>>> actually have to control the modifications to trigger specific 
>>>> errors that could be asserted. And I would expect such tests live 
>>>> somewhere in the JCK already.
>>>>
>>>> Thanks,
>>>> David
>>>> -----
>>>>
>>>>> -- Igor
>>>>>> On Feb 14, 2019, at 2:58 PM, David Holmes <david.holmes at oracle.com 
>>>>>> <mailto:david.holmes at oracle.com>> wrote:
>>>>>>
>>>>>> webrev: http://cr.openjdk.java.net/~dholmes/8218939/webrev
>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8218939
>>>>>>
>>>>>> The bytes() in a Symbol are UTF8 but do not form a NUL-terminated 
>>>>>> C-string. In a couple of places in the classfile parser we are 
>>>>>> treating it as NUL-terminated:
>>>>>>
>>>>>> - in name validation we were calling strchr to find the 
>>>>>> semi-colon, but this could run off through memory if there was no 
>>>>>> semi-colon (as per the testcase). This is replaced with memchr 
>>>>>> which takes the expected length.
>>>>>>
>>>>>> - in formatting the exception message we used %s but instead we 
>>>>>> need %.*s and pass the length
>>>>>>
>>>>>> There's a minor change to a test to print the exception 
>>>>>> information as that exposes the fact we were doing things 
>>>>>> incorrectly e.g:
>>>>>>
>>>>>> java.lang.ClassFormatError: Illegal class name 
>>>>>> "p1//BadInterface1\u00f1\u00ab\u00ab\u00ab\u00ab\u00ab\u00ab\u00ab\u00ab\u00ab\u00ab\u00ab\u00ab\u00ab\u00ab\u00ab\u00abU" 
>>>>>> in class file
>>>>>>
>>>>>> versus (after bug fix)
>>>>>>
>>>>>> java.lang.ClassFormatError: Illegal class name "p1//BadInterface1" 
>>>>>> in class file UseBadInterface1
>>>>>>
>>>>>> Testing:
>>>>>> - re-ran failing test on Windows with seeds known to have caused 
>>>>>> failures
>>>>>> - ran ran failing test on Windows 150 times with random seeds
>>>>>> - tiers 1-3
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>> -----
>