RFR(S): 8219642: ciReplay loads wrong data when MethodData size changes

[Vladimir Kozlov]

On 2/26/19 1:14 AM, Nils Eliasson wrote:
> Hi Vladimir,
> 
> There is a mutex in a MethodData, it changed size with the removal of sneaky looking:
> 
> http://hg.openjdk.java.net/jdk/jdk/file/554c3c813ad6/src/hotspot/share/oops/methodData.hpp#l1971
> 
> When a replay files is written - we take the size of the MethodData here:
> 
> http://hg.openjdk.java.net/jdk/jdk/file/554c3c813ad6/src/hotspot/share/ci/ciMethodData.cpp#l685
> 
> Then we serializes it as a char* array.
> 
> Without my change, we trust the data without verifying it, and end up reading wrong trap data.

I understand what you are talking about. But as I stated before, ciReplay was not designed to support multiply JVM 
versions. It is not only MethodData but also other structures/data used by JIT could be different.

On other hand it is our tool and I am fine to make it more flexible.

My concern about you changes changes is part where you remove existing data. I think we should bailout in this case.
I am fine with padding - is it enough to fix the issue?

Thanks,
Vladimir

> 
> // Nils
> 
> 
> On 2019-02-25 19:10, Vladimir Kozlov wrote:
>> Hi Nils,
>>
>> Is it happening because different version of VM is used for replay? Or different flags?
>> ciReplay doesn't support mixing different versions because of differences in data.
>>
>> What information is removed in MD? we should not remove any profiling information and traps information.
>>
>> Thanks,
>> Vladimir
>>
>> On 2/25/19 5:16 AM, Nils Eliasson wrote:
>>> Hi,
>>>
>>> I stumbled upon this problem when trying to reproducehttps://bugs.openjdk.java.net/browse/JDK-8219448on JDK 13. The 
>>> crash was recorded on a late 12 build, but the issue doesn't reproduce on 13. A bisection revealed that JDK-8210832 
>>> <https://bugs.openjdk.java.net/browse/JDK-8210832>"Remove sneaky locking in class Monitor" caused the problem, at it 
>>> doesn't even touch the compilers.
>>>
>>> The problem is that when ciReplay serializes a ciMethodData it will serialize a MethodData as an array preceded by 
>>> the size.
>>> But a MethodData contains an inlined Mutex, and its size changed with the removal of sneaky locking.
>>>
>>> This fix adds code for detecting a size change of MethodData, and tries to recover by adding padding or dropping 
>>> data. Since all non significant serialization data are in the beginning, the padding or dropping of data is done from 
>>> the start.
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8219642
>>>
>>> http://cr.openjdk.java.net/~neliasso/8219642/webrev.01/
>>>
>>> Please review,
>>>
>>> Nils Eliasson
>>>
>>>
>>>