[foreign] RFR 8218144: Direct invoker does not handle varargs correctly

Maurizio Cimadamore maurizio.cimadamore at oracle.com
Thu Jan 31 14:10:40 UTC 2019


Hi,
our CI systems detected a spurious failures on StdLibTest, only on Linux 
x64. After a lot of digging, I figured out the problem was that SystemV 
ABI requires the RAX register to be set to the number of vector 
arguments in a variadic call:

%rax: temporary register;with variable arguments passes information 
about the number of vector registers used

So, if we are doing a variadic call, we need to set this register to the 
number of floating point args we're passing. The universal invoker seems 
to do that:

http://hg.openjdk.java.net/panama/dev/file/tip/src/hotspot/cpu/x86/universalNativeInvoker_x86.cpp#l515

For both variadic and non-variadic calls.

So, a natural fix for the direct approach is to tweak the cast to the 
function pointers in our native stubs - e.g. from this:

void DNI_invokeNative_V_JD(JNIEnv *env, jobject _unused, jlong addr, 
jlong arg0, jdouble arg1) {
      ((void (*)(jlong, jdouble))addr)(arg0, arg1);
}

To this:

void DNI_invokeNative_V_JD(JNIEnv *env, jobject _unused, jlong addr, 
jlong arg0, jdouble arg1) {
      ((void (*)(jlong, jdouble,...))addr)(arg0, arg1);
}

Which will generate the right set for RAX (even though it will do so 
also for non-variadic call - but  that's the same as what universal 
invoker does).

I also strengthened the test - I tried first to use sprintf instead of 
printf, but that didn't work, as that doesn't seem to reproduce the 
issue (my suspicion here is that 'printf' is optimized to look into 
RAX). So, instead, I tweaked the test to run for a lot longer, by 
repeatedly shuffling the permutation array, and I also increased the 
decimal length of the double formatting string - that's because, 
unfortunately, we were using 1.23, which has 4 chars, and sometimes the 
result of printf ended up with "-NaN" which also has 4 chars, despite 
being very wrong. With both measures, tests are much more likely, so 
much so that in the affected machine I've never been able to get a pass 
w/o the proposed patch.

I verified that the patch works on all platforms, and I also verified 
that the patch does not introduce performance regressions, which is not 
the case (kind of expected, given that the only difference for adding a 
"..." is the extra MOV to RAX, as shown here [1]).

Webrev:

http://cr.openjdk.java.net/~mcimadamore/panama/8218144/

Cheers
Maurizio

[1] - https://gcc.godbolt.org/z/VAgfZc







More information about the panama-dev mailing list