C1 GraphBuilder::profile_call assert failing when inlining static interface method through MethodHandle

[Jorn Vernee]

Looks like the reproducer code got crumpled up for some reason. Let me 
try again:

public class Test {

     static final MethodHandle MH_m;

     static {
         try {
             MH_m = lookup().findStatic(MyInterface.class, "m", 
MethodType.methodType(void.class));
         } catch (ReflectiveOperationException e) {
             throw new BootstrapMethodError(e);
         }
     }

     public static void main(String[] args) throws Throwable {
         for (int i = 0; i < 20_000; i++) {
             payload();
         }
     }

     static void payload() throws Throwable {
         MH_m.invokeExact();
     }

}

interface MyInterface {
     static void m() {}
}

Jorn

On 13/02/2020 12:39, Jorn Vernee wrote:
> Hi,
>
> We were seeing an assert being hit in C1 when running the project 
> Panama test suite. After some debugging I have narrowed the failure 
> down to the following reproducer:
>
> public class Test { static final MethodHandle MH_m; static { try { 
> MH_m = lookup().findStatic(MyInterface.class, "m", 
> MethodType.methodType(void.class)); } catch 
> (ReflectiveOperationException e) { throw new BootstrapMethodError(e); 
> } } public static void main(String[] args) throws Throwable { for (int 
> i = 0; i < 20_000; i++) { payload(); } } static void payload() throws 
> Throwable { MH_m.invokeExact(); } } interface MyInterface { static 
> void m() {} }
>
> Run with `-Xbatch -XX:TieredStopAtLevel=3`.
>
> The problem is the following assert in GraphBuild::profile_call:
>
>   assert(known_holder == NULL || (known_holder->is_instance_klass() &&
> (!known_holder->is_interface() ||
> ((ciInstanceKlass*)known_holder)->has_nonstatic_concrete_methods())), 
> "should be non-static concrete method");
>
> In this particular case, we are inlining a static method in a known 
> holder which is also an interface, but doesn't have any other 
> non-static concrete methods, so the assert fails.
>
> The MethodHandle seems to be significant since known_holder is NULL 
> when simply calling MyInterface.m(). Looking at the a assert and it's 
> history, I'm not really sure what it's supposed to guard against. It 
> seems to want to check that the callee is non-abstract when we have a 
> known holder, but in that case I'm not sure why it doesn't just use 
> `!callee->is_abstract()`.
>
> Not sure what should be done here? I tried changing the assert to 
> `assert(known_holder == NULL || !callee->is_abstract())` but that 
> seems to fail in other cases. Maybe we can just remove it?
>
> Thanks,
> Jorn
>