[foreign] RFR 8211722: Add comprehensive ABI test for downcalls

Maurizio Cimadamore maurizio.cimadamore at oracle.com
Thu Oct 4 16:39:17 UTC 2018


Hi,
since we are now supporting multiple invocation schemes (for both 
upcalls and downcalls) I thought it would be a good idea to start 
testing the ABI code more in depth. I have created a generator which 
creates an header/C file with aroun 2K different functions in it, each 
of which is a specific permutation of a fixes set of parameter types. 
Example:

void f_PI_V_(void* p0, int p1) ;
void f_PF_V_(void* p0, float p1) ;
void f_PP_V_(void* p0, void* p1) ;

The implementation of these functions (in the C file) is derived in a 
simple way:

* if the function is void, body is empty
* otherwise, it is assumed that the function will return its first parameter

For struct parameters the generator also generates several combinations 
of struct fields, to make sure that everything works correctly - here an 
example of the structs defined:

struct S_II { int p0; int p1; };
struct S_IF { int p0; float p1; };
struct S_IP { int p0; void* p1; };

To keep the number of combination under control, I had to limit the 
function arity to 3, and the struct field arity to 2. Otherwise the 
extracted @NativeHeader had a declaration string which was too big to 
fit in the constant pool (!!).

The test extracts the header, loads the extracted classes and binds 
them. Then it uses reflection to call every single function in the 
header. This means that for every function to be called, the test will 
have to synthesize some parameter values to be passed to the function. 
In addition to all this, the test also verifies that, if a non-void 
function is called, the function call returns a value that is equal to 
the first argument passed.

Needless to say, this test uncovered several issues which have been 
fixed as part of this patch:

* The universal invoker doesn't like empty structs; there were several 
crashes, as no 'binding' was generated from them. Looking at the System 
V ABI it's not even 100% clear what should happen in case of an empty 
struct passed by value, but after inspecting output of clang/gcc it 
seems evident that empty structs are simply skipped (e.g. they do not 
take any register, which makes sense). So, to support this, I had to add 
support for empty structs in the classing sequence builder, and then 
special case UniversalInvoker::boxValue and 
UniversalInvoker::unboxValue, accordingly (to avoid touching pointers 
whose layout is void).

* There was another issue in UniversalInvoker: when a function returned 
a struct by value using multi-register return,  the register values were 
not copied in the correct order into the resulting struct. To fix this I 
had to tweak CallingSequence to add 'offsets' for return types and fix 
the code in UniversalInvoker::invoke to compute the offset into the 
return register array correctly (the code was erroneously computing the 
offset using ArgumentBinding::getOffset).

* The direct invocation scheme was also broken when mixed long and 
floating point parameters were passed; more specifically, the 
nativeMethodType signature was not normalized (e.g. put into a form 
where longs came first then followed by doubles) and as a result we 
failed to lookup the right method handle trampoline. I have fixed the 
direct shuffler in a more general way - now a nativeMethodType is always 
'normalized' and additionally I tweaked the way in which method handle 
permutations were computed, as I realized that the permutation step has 
to be applied at different steps depending on whether we are inside an 
upcall or a downcall (and with different method type targets too).

The resulting code passes the new ABI test (and all other tests too of 
course); note that the new ABI test is ran two times: once w/o any 
invocation scheme parameter (meaning NativeInvoker is free to chose fast 
path if it wants) and another which force the universal scheme. This 
should provide maximum coverage for both schemes.

I will work separately on a similar approach to test upcalls (but I 
think it's better to split given that this one is already biggie).

Webrev:

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

Cheers
Maurizio





More information about the panama-dev mailing list