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

[Nils Eliasson]

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.

// 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
>>
>>
>>