From 4632a1437a32b27e8d036445b96bf05668838f6f Mon Sep 17 00:00:00 2001 From: Dhruv Chopra Date: Tue, 8 Oct 2024 00:07:30 -0400 Subject: [PATCH] Increment stackSlotCount for structs correctly in FFI Upcall on z/OS This commit fixes a bug in FFI Upcall on z/OS where the counter for stack slots in memory is not always increased when iterating through arguments. Specifically, the counter was not being incremented when structs were present in the argument list. It will now be incremented correctly for every argument. Signed-off-by: Dhruv Chopra --- runtime/vm/mz64/UpcallThunkGen.cpp | 40 +++++++++++------------------- 1 file changed, 15 insertions(+), 25 deletions(-) diff --git a/runtime/vm/mz64/UpcallThunkGen.cpp b/runtime/vm/mz64/UpcallThunkGen.cpp index c08ca0f5a2d..4f7f4ab8262 100644 --- a/runtime/vm/mz64/UpcallThunkGen.cpp +++ b/runtime/vm/mz64/UpcallThunkGen.cpp @@ -287,6 +287,7 @@ createUpcallThunk(J9UpcallMetaData *metaData) } for (I_32 i = 0; i < lastSigIdx; i++) { U_32 argSizeInBytes = sigArray[i].sizeInByte; + const int totalStackSlotsReq = ((argSizeInBytes - 1) / 8) + 1; switch(sigArray[i].type) { case J9_FFI_UPCALL_SIG_TYPE_CHAR: /* Fall through */ case J9_FFI_UPCALL_SIG_TYPE_SHORT: /* Fall through */ @@ -328,7 +329,6 @@ createUpcallThunk(J9UpcallMetaData *metaData) if (fprIndex <= maxFPRIndex - 2) { thunkMem += STEY(thunkMem, fprIndex, 0, R4, argAreaOffsetFromSP + (currStackSlotIndex * 8)); thunkMem += STEY(thunkMem, fprIndex + 2, 0, R4, argAreaOffsetFromSP + (currStackSlotIndex * 8) + 4); - currStackSlotIndex += 1; fprIndex += 4; if (i < 3) { gprIndex += 2; @@ -336,13 +336,11 @@ createUpcallThunk(J9UpcallMetaData *metaData) } else if (fprIndex <= maxFPRIndex) { thunkMem += STEY(thunkMem, fprIndex, 0, R4, argAreaOffsetFromSP + (currStackSlotIndex * 8)); - currStackSlotIndex += 1; fprIndex += 2; } } else { // All other pure float structs are passed in GPR (if there's space) if (gprIndex <= maxGPRIndex) { - int totalStackSlotsReq = ((argSizeInBytes - 1) / 8) + 1; int numGPRsAvailable = maxGPRIndex - gprIndex + 1; int tempArgIndex = currStackSlotIndex; for (int i = 1; i <= std::min(numGPRsAvailable, totalStackSlotsReq); i++) { @@ -350,11 +348,9 @@ createUpcallThunk(J9UpcallMetaData *metaData) tempArgIndex += 1; gprIndex++; } - - // increase currStackSlotIndex by the number of slots this struct takes in memory - currStackSlotIndex += totalStackSlotsReq; } } + currStackSlotIndex += totalStackSlotsReq; break; case J9_FFI_UPCALL_SIG_TYPE_STRUCT_AGGREGATE_ALL_DP: if (argSizeInBytes == 2 * sizeof(double)) { @@ -364,9 +360,7 @@ createUpcallThunk(J9UpcallMetaData *metaData) // argument area. if (fprIndex <= maxFPRIndex - 2) { thunkMem += STDY(thunkMem, fprIndex, 0, R4, argAreaOffsetFromSP + (currStackSlotIndex * 8)); - currStackSlotIndex += 1; - thunkMem += STDY(thunkMem, fprIndex + 2, 0, R4, argAreaOffsetFromSP + (currStackSlotIndex * 8)); - currStackSlotIndex += 1; + thunkMem += STDY(thunkMem, fprIndex + 2, 0, R4, argAreaOffsetFromSP + ((currStackSlotIndex + 1) * 8)); fprIndex += 4; if (i < 3) { gprIndex += 2; @@ -374,24 +368,22 @@ createUpcallThunk(J9UpcallMetaData *metaData) } else if (fprIndex <= maxFPRIndex) { thunkMem += STDY(thunkMem, fprIndex, 0, R4, argAreaOffsetFromSP + (currStackSlotIndex * 8)); - currStackSlotIndex += 1; fprIndex += 2; } } - else { // This is for all other pure double structs (they will be passed in GPRs if there's space) - if (gprIndex <= maxGPRIndex) { - int totalStackSlotsReq = argSizeInBytes / 8; // the size in bytes for a struct with only doubles will always be a multiple of 8. - int numGPRsAvailable = maxGPRIndex - gprIndex + 1; - int tempArgIndex = currStackSlotIndex; - for (int i = 1; i <= std::min(numGPRsAvailable, totalStackSlotsReq); i++) { - thunkMem += STG(thunkMem, gprIndex, 0, R4, argAreaOffsetFromSP + (tempArgIndex * 8)); - tempArgIndex += 1; - gprIndex++; + else { // This is for all other pure double structs (they will be passed in GPRs if there's space) + if (gprIndex <= maxGPRIndex) { + int numGPRsAvailable = maxGPRIndex - gprIndex + 1; + int tempArgIndex = currStackSlotIndex; + for (int i = 1; i <= std::min(numGPRsAvailable, totalStackSlotsReq); i++) { + thunkMem += STG(thunkMem, gprIndex, 0, R4, argAreaOffsetFromSP + (tempArgIndex * 8)); + tempArgIndex += 1; + gprIndex++; + } + // increase currStackSlotIndex by the number of slots this struct takes in memory } - // increase currStackSlotIndex by the number of slots this struct takes in memory - currStackSlotIndex += totalStackSlotsReq; } - } + currStackSlotIndex += totalStackSlotsReq; break; case J9_FFI_UPCALL_SIG_TYPE_STRUCT_AGGREGATE_SP_DP: case J9_FFI_UPCALL_SIG_TYPE_STRUCT_AGGREGATE_SP_SP_DP: @@ -404,7 +396,6 @@ createUpcallThunk(J9UpcallMetaData *metaData) case J9_FFI_UPCALL_SIG_TYPE_STRUCT_AGGREGATE_OTHER: case J9_FFI_UPCALL_SIG_TYPE_STRUCT_AGGREGATE_MISC: if (gprIndex <= maxGPRIndex) { - int totalStackSlotsReq = ((argSizeInBytes - 1) / 8) + 1; int numGPRsAvailable = maxGPRIndex - gprIndex + 1; int tempArgIndex = currStackSlotIndex; for (int i = 1; i <= std::min(numGPRsAvailable, totalStackSlotsReq); i++) { @@ -412,9 +403,8 @@ createUpcallThunk(J9UpcallMetaData *metaData) tempArgIndex += 1; gprIndex++; } - // increase currStackSlotIndex by the number of slots this struct takes in memory - currStackSlotIndex += totalStackSlotsReq; } + currStackSlotIndex += totalStackSlotsReq; break; default: Assert_VM_unreachable();