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

[Vladimir Ivanov]

Hi Jorn,

The assert does looks too strong and should be adjusted.

But it looks surprising that profiling is peformend for static methods:

   assert(..., "should be non-static concrete method");

Please, file a bug.

Best regards,
Vladimir Ivanov

On 13.02.2020 14:46, Jorn Vernee wrote:
> 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
>>