Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FFI Upcall: Recognizing certain unions as J9_FFI_UPCALL_SIG_TYPE_STRUCT_AGGREGATE_ALL_SP is inconsistent with z/OS ABI #19952

Closed
dchopra001 opened this issue Jul 31, 2024 · 2 comments · Fixed by #20018
Labels
comp:vm os:zos project:panama Used to track Project Panama related work

Comments

@dchopra001
Copy link
Contributor

dchopra001 commented Jul 31, 2024

As part of the FFI Upcall implementation work, I have encountered a scenario where certain unions get mapped to what z/OS calls "complex" types.

Background
As per z/OS ABI, all struct arguments except for the following cases below are passed in GPRs.

Floating point types, including complex types, are returned FPR0, FPR2, FPR4 and FPR6, using as
many registers as required.
Restriction: Not every language supports complex types. For the purposes of argument passing and
function return values, in every language every aggregate that is (a) not a union, and (b) contains
exactly two floating-point types of the same size (4, 8, or 16 bytes) is treated as a complex type.

To put it another way, all struct arguments are passed in GPRs except for the following two:

  • struct{float, float} // size is always 8 bytes
  • struct{double, double} // size is always 16 bytes

FFI recognizes type of structure's datatype as: J9_FFI_UPCALL_SIG_TYPE_STRUCT_AGGREGATE_ALL_SP/J9_FFI_UPCALL_SIG_TYPE_STRUCT_AGGREGATE_ALL_DP. We can then check the size of the argument to see if one of the above two cases is encountered. If that case is encountered, then we must access arguments from FPRs instead of GPRs.

There is however another scenario where FFI will recognize a struct with datatype J9_FFI_UPCALL_SIG_TYPE_STRUCT_AGGREGATE_ALL_SP/J9_FFI_UPCALL_SIG_TYPE_STRUCT_AGGREGATE_ALL_DP. This struct will also have a size of 8 for float elements, and 16 for double elements. That struct looks like below:

union { float, struct { float, float }}

This union also has size 8. Unions are passed in GPRs as per the z/OS ABI. So if the above union is recognized as J9_FFI_UPCALL_SIG_TYPE_STRUCT_AGGREGATE_ALL_SP, then the upcall thunk routine can't differentiate between a struct argument with 2 elements (passed in FPRs) and a union with size = 8 (passed in GPRs).

As a solution, we propose to recognize the above union as J9_FFI_UPCALL_SIG_TYPE_STRUCT_AGGREGATE_MISC. In fact, all unions will be passed in GPRs, so they can all be recognized as that type from the perspective of the FFI Upcall routine on z/OS.

Searching around, I think this would be the place to make the change:

/* @brief This wrapper function invokes parseStruct() to determine
* the AGGREGATE subtype of the specified struct.
*
* @param structSig[in] A pointer to the specified struct signature string
* @param sizeInByte[in] the struct size in bytes
* @return An encoded type of the struct signature
*/
static U_8
encodeOuterStruct(char *structSig, U_32 sizeInByte)
{
bool isAllSP = true;
bool isAllDP = true;
U_8 structSigType = 0;
U_8 first16ByteComposTypes[J9_FFI_UPCALL_COMPOSITION_TYPE_ARRAY_LENGTH] = {0};
UDATA curIndex = 0;
/* Analyze the specified native signature to fill up a 16-byte composition type
* array so as to determine the aggregate subtype.
*/
parseStruct(&structSig, &isAllSP, &isAllDP, first16ByteComposTypes, &curIndex);
if (isAllSP) {
structSigType = J9_FFI_UPCALL_SIG_TYPE_STRUCT_AGGREGATE_ALL_SP;
} else if (isAllDP) {
structSigType = J9_FFI_UPCALL_SIG_TYPE_STRUCT_AGGREGATE_ALL_DP;
} else if (sizeInByte > J9_FFI_UPCALL_COMPOSITION_TYPE_ARRAY_LENGTH) {
/* AGGREGATE_OTHER (mix of different types without pure float/double) is
* intended for the native signature greater than 16 bytes in size
*/
structSigType = J9_FFI_UPCALL_SIG_TYPE_STRUCT_AGGREGATE_OTHER;
} else {
/* Analyze the 16-byte composition type array to determine the aggregate subtype
* of the native signature which is equal to or less than 16 bytes in size).
*/
structSigType = getStructSigTypeFrom16ByteComposTypes(first16ByteComposTypes);
}
return structSigType;
}

@dchopra001
Copy link
Contributor Author

@ChengJin01 @r30shah @zl-wang FYI

@ChengJin01
Copy link

ChengJin01 commented Jul 31, 2024

This is something we detected on z/OS which might need to handle only on z/OS in terms of the mapping code for arguments in upcall.

FYI: @tajila, @pshipton, @joransiu

@ChengJin01 ChengJin01 added comp:vm project:panama Used to track Project Panama related work os:zos labels Jul 31, 2024
ChengJin01 pushed a commit to ChengJin01/openj9 that referenced this issue Aug 19, 2024
The changes resolve the issue with the register mapping for the
struct FF/DD or its variants in upcall on z/OS by categorizing
them into ALL_SP/ALL_DP placed on FPRs while other non-complex
type are categorized into AGGREGATE_OTHER placed on GPRs.

Fixes: eclipse-openj9#19952

Signed-off-by: ChengJin01 <[email protected]>
ChengJin01 pushed a commit to ChengJin01/openj9 that referenced this issue Aug 23, 2024
The changes resolve the issue with the register mapping for the
struct FF/DD or its variants in upcall on z/OS by categorizing
them into ALL_SP/ALL_DP placed on FPRs while other non-complex
type are categorized into AGGREGATE_OTHER placed on GPRs.

Fixes: eclipse-openj9#19952

Signed-off-by: ChengJin01 <[email protected]>
ChengJin01 pushed a commit to ChengJin01/openj9 that referenced this issue Aug 23, 2024
The changes resolve the issue with the register mapping for the
struct FF/DD or its variants in upcall on z/OS by categorizing
them into ALL_SP/ALL_DP placed on FPRs while other non-complex
type are categorized into AGGREGATE_OTHER placed on GPRs.

Fixes: eclipse-openj9#19952

Signed-off-by: ChengJin01 <[email protected]>
ChengJin01 pushed a commit to ChengJin01/openj9 that referenced this issue Aug 24, 2024
The changes resolve the issue with the register mapping for the
struct FF/DD or its variants in upcall on z/OS by categorizing
them into ALL_SP/ALL_DP placed on FPRs while other non-complex
type are categorized into AGGREGATE_OTHER placed on GPRs.

Fixes: eclipse-openj9#19952

Signed-off-by: ChengJin01 <[email protected]>
ChengJin01 pushed a commit to ChengJin01/openj9 that referenced this issue Aug 24, 2024
The changes resolve the issue with the register mapping for the
struct FF/DD or its variants in upcall on z/OS by categorizing
them into ALL_SP/ALL_DP placed on FPRs while other non-complex
type are categorized into AGGREGATE_OTHER placed on GPRs.

Fixes: eclipse-openj9#19952

Signed-off-by: ChengJin01 <[email protected]>
ChengJin01 pushed a commit to ChengJin01/openj9 that referenced this issue Aug 24, 2024
The changes resolve the issue with the register mapping for the
struct FF/DD or its variants in upcall on z/OS by categorizing
them into ALL_SP/ALL_DP placed on FPRs while other non-complex
type are categorized into AGGREGATE_OTHER placed on GPRs.

Fixes: eclipse-openj9#19952

Signed-off-by: ChengJin01 <[email protected]>
ChengJin01 pushed a commit to ChengJin01/openj9 that referenced this issue Aug 26, 2024
The changes resolve the issue with the register mapping for the
struct FF/DD or its variants in upcall on z/OS by categorizing
them into ALL_SP/ALL_DP placed on FPRs while other non-complex
type are categorized into AGGREGATE_OTHER placed on GPRs.

Fixes: eclipse-openj9#19952

Signed-off-by: ChengJin01 <[email protected]>
ChengJin01 pushed a commit to ChengJin01/openj9 that referenced this issue Aug 26, 2024
The changes resolve the issue with the register mapping for the
struct FF/DD or its variants in upcall on z/OS by categorizing
them into ALL_SP/ALL_DP placed on FPRs while other non-complex
type are categorized into AGGREGATE_OTHER placed on GPRs.

Fixes: eclipse-openj9#19952

Signed-off-by: ChengJin01 <[email protected]>
ChengJin01 pushed a commit to ChengJin01/openj9 that referenced this issue Aug 26, 2024
The changes resolve the issue with the register mapping for the
struct FF/DD or its variants in upcall on z/OS by categorizing
them into ALL_SP/ALL_DP placed on FPRs while other non-complex
type are categorized into AGGREGATE_OTHER placed on GPRs.

Fixes: eclipse-openj9#19952

Signed-off-by: ChengJin01 <[email protected]>
@tajila tajila closed this as completed in babde24 Aug 30, 2024
rmnattas pushed a commit to rmnattas/openj9 that referenced this issue Sep 5, 2024
The changes resolve the issue with the register mapping for the
struct FF/DD or its variants in upcall on z/OS by categorizing
them into ALL_SP/ALL_DP placed on FPRs while other non-complex
type are categorized into AGGREGATE_OTHER placed on GPRs.

Fixes: eclipse-openj9#19952

Signed-off-by: ChengJin01 <[email protected]>
luke-li-2003 pushed a commit to luke-li-2003/openj9 that referenced this issue Sep 23, 2024
The changes resolve the issue with the register mapping for the
struct FF/DD or its variants in upcall on z/OS by categorizing
them into ALL_SP/ALL_DP placed on FPRs while other non-complex
type are categorized into AGGREGATE_OTHER placed on GPRs.

Fixes: eclipse-openj9#19952

Signed-off-by: ChengJin01 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:vm os:zos project:panama Used to track Project Panama related work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants