RFR(S): 8235762: JVM crash in SWPointer during C2 compilation

[Yangfei (Felix)]

Hi, 

  I have created a webrev for the patch: http://cr.openjdk.java.net/~fyang/8235762/webrev.00/  
  Tested tier1-3 with both aarch64 and x86_64 linux release build.  
  Newly added test case fail without the patch and pass with the patch.  

Thanks,
Felix

> -----Original Message-----
> From: Yangfei (Felix)
> Sent: Wednesday, December 11, 2019 11:12 PM
> To: 'Christian Hagedorn' <christian.hagedorn at oracle.com>; Tobias Hartmann
> <tobias.hartmann at oracle.com>; hotspot-compiler-dev at openjdk.java.net
> Subject: RE: RFR(S): 8235762: JVM crash in SWPointer during C2 compilation
> 
> Hi Christian,
> 
> Thanks for the suggestions. Comments inlined.
> 
> > -----Original Message-----
> > From: Christian Hagedorn [mailto:christian.hagedorn at oracle.com]
> > Sent: Wednesday, December 11, 2019 10:54 PM
> > To: Yangfei (Felix) <felix.yang at huawei.com>; Tobias Hartmann
> > <tobias.hartmann at oracle.com>; hotspot-compiler-dev at openjdk.java.net
> > Subject: Re: RFR(S): 8235762: JVM crash in SWPointer during C2
> > compilation
> >
> > Hi Felix
> >
> > Thanks for working on that. Your fix also seems to work for JDK-8235700.
> > I closed that one as a duplicate of yours.
> >
> > >>> +              for (int i = 0; i < orig_msize; i++) {
> >
> > Should be uint since orig_msize is a uint
> 
> -- Yes, will modify accordingly when I am preparing webrev.
> 
> >
> > >>> +              best_align_to_mem_ref = find_align_to_ref(memops,
> > >> max_idx);
> > >>> +              assert(best_align_to_mem_ref == NULL, "sanity");
> >
> > You can merge these two lines together into
> > assert(find_align_to_ref(memops,
> > max_idx) == NULL, "sanity"); since the call belongs to the sanity
> > check. Or just surround it by a #ifdef ASSERT.
> 
> -- The purpose of line 721 here is to calculate the max_idx.
>   So I don't think it's suitable to treat this line as assertion logic.
> 
> > >>> +  idx = max_idx;
> >
> > Is max_idx always guaranteed to be valid and not -1 when accessing it later?
> 
> -- Yes, I think so.
>   When memops is not empty and the memory ops in memops are not
> comparable, find_align_to_ref will always sets its max_idx.
> 
> Thanks,
> Felix