Request for reviews (L): 7012914: JSR 292 MethodHandlesTest C1: frame::verify_return_pc(return_address) failed: must be a return pc

[Tom Rodriguez]

On Feb 21, 2011, at 9:15 AM, Christian Thalinger wrote:

> http://cr.openjdk.java.net/~twisti/7012914/webrev.01/
> 
> 7012914: JSR 292 MethodHandlesTest C1: frame::verify_return_pc(return_address) failed: must be a return pc
> Reviewed-by:
> 
> The changes of 7007377 changed the way
> MethodHandlesImpl.raiseException is called from the raise_exception
> method handle adapter as it calls the C2I adapter.  Since
> MethodHandlesImpl.raiseException has three arguments, on 32-bit x86 we
> need to pass one argument on the stack.  This additional word on the
> stack needs to be removed when we return to the actual callee during
> unwinding.
> 
> Since the call site that triggers the exception is a method handle
> call site we just need to restore the saved SP from BP.
> 
> Right now this is only a problem on 32-bit x86 because of the way we
> implement the raise_exception method handle adapter.  But I assume
> it's not impossible that in the future there are stack manipulations
> before we throw an exception.  So a fix for all architectures seems
> reasonable.
> 
> This change removes some unused code in
> StubGenerator::generate_forward_exception as this stub is only used
> for runtime calls with a pending exception and runtime calls can't be
> method handle call sites.  It also removes the unused
> ExceptionCache::_unwind_handler.
> 
> Additionally there is a small cleanup of
> SharedRuntime::raw_exception_handler_for_return_address to not call
> CodeCache::find_blob twice.

Isn't the second call unreachable?  If nm != null then we will already have returned.  So can't the whole block be replaced with:

guarantee(blob == NULL || !blob->is_runtime_stub(), "caller should have skipped stub");

Otherwise it looks fine.

tom