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

[John Rose]

Here's a late comment:  Is there any reason to
put the deletion and insertion in different
places?  If not, it would be easier to follow
the history, and to do merges, if they were
placed at the same point in the code.
That is, insert the new code where the old
code is deleted.

On Apr 30, 2019, at 12:04 AM, Rahul Raghavan <rahul.v.raghavan at oracle.com> wrote:
> 
> Thank you Vladimir Ivanov for suggestions.
> 
> Please note following latest changes tried.
> <webrev.04> - http://cr.openjdk.java.net/~rraghavan/8202414/webrev.04/
> 
> Hope did not miss any points.
> Confirmed no failures with the reported test cases.
> Also hs-tier1 to tier4, hs-precheckin-comp testing in progress.
> 
> Thanks,
> Rahul
> 
> On 27/04/19 11:48 AM, Vladimir Ivanov wrote:
>> On 26/04/2019 19:30, Vladimir Ivanov wrote:
>> After thinking more about it, I believe new offset alignment check supersedes is_unaligned_access(). And is_mismatched_access() is too conservative here: what is_mismatched_access() adds here (in addition to existing alignment & size checks) is whether type match between location and stored value, but what matters for IN are sizes and offsets only.
>> Type mismatches (e.g., byte vs boolean, char vs short) may cause problems when consequent loads are replaced with values from initializing stores, but it should be already handled in MemNode::can_see_stored_value() and Load?Node::Ideal().
>> So, it seems both checks (is_unaligned_access() & is_mismatched_access()) can be safely omitted.
>> You are right, I missed that IN::captured_store_insertion_point() inspects already other stores which are already captured. Sorry for the confusion.
>> I agree that IN::can_capture_store() is the right place to put the fix in and I like (iii). (Just add a comment, "// mismatched access" is enough)