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

[David Holmes]

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/

Thanks,
David

> -- Igor
> 
>> On Feb 14, 2019, at 4:36 PM, David Holmes <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> 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
>>>> -----
>