From 1cd7257ccc3781891d5a8bed631adb014371c9a1 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..33e7f4f8178 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; + 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();