[13] RFR: 8202414: Unsafe write after primitive array creation may result in array length change
dean.long at oracle.com
dean.long at oracle.com
Thu Mar 28 00:27:12 UTC 2019
I don't think we can use is_mismatched_access(), because we seem to have
the same problem even if int[] is used. I took another look at this,
and I believe we can fix this in InitializeNode::complete_stores(),
while still allowing the captured store optimization.
dl
On 3/27/19 10:12 AM, Vladimir Ivanov wrote:
> First, I'd like to note that it's a good practice to include problem &
> root cause descriptions in the request. Otherwise, reviewers have to
> find that information themselves which complicates review process.
>
> (In this particular case, I found some analysis from the submitter [1]
> in the bug only after carefully reading through it.)
>
> On 27/03/2019 06:44, Rahul Raghavan wrote:
>> Hi,
>>
>> Thank you Vladimir.
>>
>> Yes, tried following fix.
>> (needed to add checks to avoid SIGFPE crash).
>>
>> + int size_in_bytes = st->memory_size();
>> + if ((size_in_bytes != 0) && (get_store_offset(st, phase) %
>> size_in_bytes) != 0) {
>> + return FAIL;
>> + }
>>
>>
>> <webrev.02> - http://cr.openjdk.java.net/~rraghavan/8202414/webrev.02/
>
> It seems the problem is due to mismatched unsafe store being captured
> as a initializing one. Why not check for it explicitly?
>
> if (st->is_unaligned_access() || st->is_mismatched_access()) {
> return FAIL;
> }
>
> Best regards,
> Vladimir Ivanov
>
>
> [1]
>
> For your convenience, our analysis shows the problem may relate to
> array InitializeNode logic.
> It `capture_store` the the memory write of Unsafe.putInt.
> Since the putInt occupied offset range [17, 21] from the array pointer,
> then it decided to `clear_memory` of offset range [16, 17] of the
> array pointer.
> This range actually cannot pass the assert "assert((end_offset %
> BytesPerInt) == 0, "odd end offset")".
> While in jvm product mode, without the assert, the compiler falsely
> calculated to clear range [13, 17],
> which will clear the three most significant bytes of the `length` of
> this array.
>
>
>>
>> Confirmed no issues with testing for this revised fix.
>>
>> Thanks,
>> Rahul
>>
>> On 26/03/19 1:03 AM, Vladimir Kozlov wrote:
>>>
>>> Suggestion:
>>>
>>> if ((get_store_offset(st, phase) % st->memory_size()) != 0) {
>>>
>>> Vladimir
>>>
>>>
More information about the hotspot-compiler-dev
mailing list