[13] RFR: 8202414: Unsafe write after primitive array creation may result in array length change

[Vladimir Ivanov]

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