RFR: 8264358: Don't create invalid oop in method handle tracing

[Tobias Hartmann]

On Mon, 29 Mar 2021 11:49:56 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

> The `mh` field in:
> 
> struct MethodHandleStubArguments {
>   const char* adaptername;
>   oopDesc* mh;
>   intptr_t* saved_regs;
>   intptr_t* entry_sp;
> };
> 
> doesn't always point to a valid object. The `oopDesc*` is then implicitly converted to an `oop` here:
> 
> void trace_method_handle_stub_wrapper(MethodHandleStubArguments* args) {
>   trace_method_handle_stub(args->adaptername,
>                            args->mh,
>                            args->saved_regs,
>                            args->entry_sp);
> }
> 
> This gets caught by my ad-hoc verification code that verifies oops when they are created/used.
> 
> I propose that we don't create an oop until it `mh` is actually used, and it has been checked that the argument should contain a valid oop.  I started with a more elaborate fix that changed the type of `mh` to be `void*`, but then reverted to a more targetted fix to remove the early oopDesc* > oop conversion.
> 
> One thing that I am curious about is this code inside trace_method_handle_stub:
> if (has_mh && oopDesc::is_oop(mh)) {
>   mh->print_on(&ls);
> 
> Delaying the oopDesc* > oop conversion to after the `has_mh` check solves my verification failure, but I wonder if the `oopDesc::is_oop(mh)` call is really needed when we have the `has_mh` check?

Looks good to me too.

-------------

Marked as reviewed by thartmann (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3242