Hi Andrew, Please take a look at this updated patch:
http://cr.openjdk.java.net/~ngasson/8209413/webrev.03/ It contains your suggested change below, as well as an extra check in the AARCH64Frame two-argument constructor that the PC from SP[-1] is valid. Also changed SharedRuntime::generate_native_wrapper and TemplateInterpreterGenerator::generate_native_entry to save an accurate return PC in the frame anchor. There's a FIXME in set_last_Java_frame like this: } else { // FIXME: This is almost never correct. We should delete all // cases of set_last_Java_frame with last_java_pc=NULL and use the // correct return address instead. adr(scratch, pc()); } Should we do this now? There's only two places I can see: StubGenerator::generate_throw_exception and the set_last_Java_frame overload that takes a Label. Thanks, Nick On 12/02/2019 23:32, Andrew Haley wrote: > What's the status of this? Are you still looking at it? > > On 2/7/19 3:39 PM, Andrew Haley wrote: >> On 2/7/19 2:36 PM, Nick Gasson (Arm Technology China) wrote: >>> >>> Yeah I tried this method too (see the end of my first email). It >>> works fine except the PC is off by a few instructions. That can be >>> fixed quite easily though if required, and it always points into the >>> right method. >> >> We should fix those few cases. >> >>> I ended up doing the stack scanning thing because as far as I can >>> tell that constructor is only used when we need to find the return >>> PC from a native frame >> >> I don't think that ever happens. In the code I modified we know that >> we have a good PC in the frame anchor. In the cases where we have a >> native frame on the stack and nothing in the frame anchor we give >> up. At least, I can see no counter-cases. >> >>> (I might have been wrong about this), so I couldn't see how it would >>> ever get the right value. If we check vm.isJavaPCDbg() before we use >>> the SP[-1] value that will make the remaining callers a bit safer >>> (and set this.pc to null otherwise). >> >> That would be safe, for sure. >> > >
