Skip to content

Commit

Permalink
Increment stackSlotCount for structs correctly in FFI Upcall on z/OS
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@dchopra001
dchopra001 committed Oct 10, 2024
1 parent 1153568 commit fc65fff
Showing 1 changed file with 24 additions and 34 deletions.
58 changes: 24 additions & 34 deletions runtime/vm/mz64/UpcallThunkGen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,11 +269,11 @@ createUpcallThunk(J9UpcallMetaData *metaData)
}
metaData->thunkAddress = (void *)thunkMem;

I_32 gprIndex = 1;
const I_32 maxGPRIndex = 3;
I_32 fprIndex = 0;
const I_32 maxFPRIndex = 6;
I_32 currStackSlotIndex = 0;
U_32 gprIndex = 1;
const U_32 maxGPRIndex = 3;
U_32 fprIndex = 0;
const U_32 maxFPRIndex = 6;
U_32 currStackSlotIndex = 0;

// Now we loop through the arguments and generate the actual instructions to store any register arguments in the caller frame.
const I_32 numRegsToPreserve = 10;
Expand All @@ -287,6 +287,7 @@ createUpcallThunk(J9UpcallMetaData *metaData)
}
for (I_32 i = 0; i < lastSigIdx; i++) {
U_32 argSizeInBytes = sigArray[i].sizeInByte;
const U_32 totalStackSlotsReq = (argSizeInBytes + 7) / 8;
switch(sigArray[i].type) {
case J9_FFI_UPCALL_SIG_TYPE_CHAR: /* Fall through */
case J9_FFI_UPCALL_SIG_TYPE_SHORT: /* Fall through */
Expand Down Expand Up @@ -328,33 +329,28 @@ 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;
}
}
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;
U_32 numGPRsAvailable = maxGPRIndex - gprIndex + 1;
U_32 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
currStackSlotIndex += totalStackSlotsReq;
}
}
currStackSlotIndex += totalStackSlotsReq;
break;
case J9_FFI_UPCALL_SIG_TYPE_STRUCT_AGGREGATE_ALL_DP:
if (argSizeInBytes == 2 * sizeof(double)) {
Expand All @@ -364,34 +360,30 @@ 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;
}
}
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) {
U_32 numGPRsAvailable = maxGPRIndex - gprIndex + 1;
U_32 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:
Expand All @@ -404,17 +396,15 @@ 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;
U_32 numGPRsAvailable = maxGPRIndex - gprIndex + 1;
U_32 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
currStackSlotIndex += totalStackSlotsReq;
}
currStackSlotIndex += totalStackSlotsReq;
break;
default:
Assert_VM_unreachable();
Expand Down

0 comments on commit fc65fff

Please sign in to comment.