Skip to content

Commit 9dd1c5f

Browse files
JornVerneeJornVernee
committedJul 20, 2023
8312473: Return value corrupted when using CCS + isTrivial
Reviewed-by: mcimadamore
1 parent 7983194 commit 9dd1c5f

File tree

5 files changed

+39
-27
lines changed

5 files changed

+39
-27
lines changed
 

‎src/hotspot/cpu/aarch64/downcallLinker_aarch64.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ void DowncallStubGenerator::generate() {
168168
assert(_abi._shadow_space_bytes == 0, "not expecting shadow space on AArch64");
169169
allocated_frame_size += arg_shuffle.out_arg_bytes();
170170

171-
bool should_save_return_value = !_needs_return_buffer && _needs_transition;
171+
bool should_save_return_value = !_needs_return_buffer;
172172
RegSpiller out_reg_spiller(_output_registers);
173173
int spill_offset = -1;
174174

‎src/hotspot/cpu/ppc/downcallLinker_ppc.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ void DowncallStubGenerator::generate() {
165165
int parameter_save_area_slots = MAX2(_input_registers.length(), 8);
166166
int allocated_frame_size = frame::native_abi_minframe_size + parameter_save_area_slots * BytesPerWord;
167167

168-
bool should_save_return_value = !_needs_return_buffer && _needs_transition;
168+
bool should_save_return_value = !_needs_return_buffer;
169169
RegSpiller out_reg_spiller(_output_registers);
170170
int spill_offset = -1;
171171

‎src/hotspot/cpu/riscv/downcallLinker_riscv.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ void DowncallStubGenerator::generate() {
165165
assert(_abi._shadow_space_bytes == 0, "not expecting shadow space on RISCV64");
166166
allocated_frame_size += arg_shuffle.out_arg_bytes();
167167

168-
bool should_save_return_value = !_needs_return_buffer && _needs_transition;
168+
bool should_save_return_value = !_needs_return_buffer;
169169
RegSpiller out_reg_spiller(_output_registers);
170170
int spill_offset = -1;
171171

‎src/hotspot/cpu/x86/downcallLinker_x86_64.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ void DowncallStubGenerator::generate() {
166166
allocated_frame_size += arg_shuffle.out_arg_bytes();
167167

168168
// when we don't use a return buffer we need to spill the return value around our slow path calls
169-
bool should_save_return_value = !_needs_return_buffer && _needs_transition;
169+
bool should_save_return_value = !_needs_return_buffer;
170170
RegSpiller out_reg_spiller(_output_registers);
171171
int spill_rsp_offset = -1;
172172

‎test/jdk/java/foreign/capturecallstate/TestCaptureCallState.java

+35-23
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,16 @@ public class TestCaptureCallState extends NativeTestHelper {
6363
}
6464
}
6565

66-
private record SaveValuesCase(String nativeTarget, FunctionDescriptor nativeDesc, String threadLocalName, Consumer<Object> resultCheck) {}
66+
private record SaveValuesCase(String nativeTarget, FunctionDescriptor nativeDesc, boolean trivial, String threadLocalName, Consumer<Object> resultCheck) {}
6767

6868
@Test(dataProvider = "cases")
6969
public void testSavedThreadLocal(SaveValuesCase testCase) throws Throwable {
70-
Linker.Option stl = Linker.Option.captureCallState(testCase.threadLocalName());
71-
MethodHandle handle = downcallHandle(testCase.nativeTarget(), testCase.nativeDesc(), stl);
70+
List<Linker.Option> options = new ArrayList<>();
71+
options.add(Linker.Option.captureCallState(testCase.threadLocalName()));
72+
if (testCase.trivial()) {
73+
options.add(Linker.Option.isTrivial());
74+
}
75+
MethodHandle handle = downcallHandle(testCase.nativeTarget(), testCase.nativeDesc(), options.toArray(Linker.Option[]::new));
7276

7377
StructLayout capturedStateLayout = Linker.Option.captureStateLayout();
7478
VarHandle errnoHandle = capturedStateLayout.varHandle(groupElement(testCase.threadLocalName()));
@@ -101,36 +105,44 @@ public void testInvalidCaptureSegment(MemorySegment captureSegment,
101105
}
102106
}
103107

108+
interface CaseAdder {
109+
void addCase(String nativeTarget, FunctionDescriptor nativeDesc, String threadLocalName, Consumer<Object> resultCheck);
110+
}
111+
104112
@DataProvider
105113
public static Object[][] cases() {
106114
List<SaveValuesCase> cases = new ArrayList<>();
115+
CaseAdder adder = (nativeTarget, nativeDesc, threadLocalName, resultCheck) -> {
116+
cases.add(new SaveValuesCase(nativeTarget, nativeDesc, false, threadLocalName, resultCheck));
117+
cases.add(new SaveValuesCase(nativeTarget, nativeDesc, true, threadLocalName, resultCheck));
118+
};
107119

108-
cases.add(new SaveValuesCase("set_errno_V", FunctionDescriptor.ofVoid(JAVA_INT), "errno", o -> {}));
109-
cases.add(new SaveValuesCase("set_errno_I", FunctionDescriptor.of(JAVA_INT, JAVA_INT), "errno", o -> assertEquals((int) o, 42)));
110-
cases.add(new SaveValuesCase("set_errno_D", FunctionDescriptor.of(JAVA_DOUBLE, JAVA_INT), "errno", o -> assertEquals((double) o, 42.0)));
111-
112-
cases.add(structCase("SL", Map.of(JAVA_LONG.withName("x"), 42L)));
113-
cases.add(structCase("SLL", Map.of(JAVA_LONG.withName("x"), 42L,
114-
JAVA_LONG.withName("y"), 42L)));
115-
cases.add(structCase("SLLL", Map.of(JAVA_LONG.withName("x"), 42L,
116-
JAVA_LONG.withName("y"), 42L,
117-
JAVA_LONG.withName("z"), 42L)));
118-
cases.add(structCase("SD", Map.of(JAVA_DOUBLE.withName("x"), 42D)));
119-
cases.add(structCase("SDD", Map.of(JAVA_DOUBLE.withName("x"), 42D,
120-
JAVA_DOUBLE.withName("y"), 42D)));
121-
cases.add(structCase("SDDD", Map.of(JAVA_DOUBLE.withName("x"), 42D,
122-
JAVA_DOUBLE.withName("y"), 42D,
123-
JAVA_DOUBLE.withName("z"), 42D)));
120+
adder.addCase("set_errno_V", FunctionDescriptor.ofVoid(JAVA_INT), "errno", o -> {});
121+
adder.addCase("set_errno_I", FunctionDescriptor.of(JAVA_INT, JAVA_INT), "errno", o -> assertEquals((int) o, 42));
122+
adder.addCase("set_errno_D", FunctionDescriptor.of(JAVA_DOUBLE, JAVA_INT), "errno", o -> assertEquals((double) o, 42.0));
123+
124+
structCase(adder, "SL", Map.of(JAVA_LONG.withName("x"), 42L));
125+
structCase(adder, "SLL", Map.of(JAVA_LONG.withName("x"), 42L,
126+
JAVA_LONG.withName("y"), 42L));
127+
structCase(adder, "SLLL", Map.of(JAVA_LONG.withName("x"), 42L,
128+
JAVA_LONG.withName("y"), 42L,
129+
JAVA_LONG.withName("z"), 42L));
130+
structCase(adder, "SD", Map.of(JAVA_DOUBLE.withName("x"), 42D));
131+
structCase(adder, "SDD", Map.of(JAVA_DOUBLE.withName("x"), 42D,
132+
JAVA_DOUBLE.withName("y"), 42D));
133+
structCase(adder, "SDDD", Map.of(JAVA_DOUBLE.withName("x"), 42D,
134+
JAVA_DOUBLE.withName("y"), 42D,
135+
JAVA_DOUBLE.withName("z"), 42D));
124136

125137
if (IS_WINDOWS) {
126-
cases.add(new SaveValuesCase("SetLastError", FunctionDescriptor.ofVoid(JAVA_INT), "GetLastError", o -> {}));
127-
cases.add(new SaveValuesCase("WSASetLastError", FunctionDescriptor.ofVoid(JAVA_INT), "WSAGetLastError", o -> {}));
138+
adder.addCase("SetLastError", FunctionDescriptor.ofVoid(JAVA_INT), "GetLastError", o -> {});
139+
adder.addCase("WSASetLastError", FunctionDescriptor.ofVoid(JAVA_INT), "WSAGetLastError", o -> {});
128140
}
129141

130142
return cases.stream().map(tc -> new Object[] {tc}).toArray(Object[][]::new);
131143
}
132144

133-
static SaveValuesCase structCase(String name, Map<MemoryLayout, Object> fields) {
145+
static void structCase(CaseAdder adder, String name, Map<MemoryLayout, Object> fields) {
134146
StructLayout layout = MemoryLayout.structLayout(fields.keySet().toArray(MemoryLayout[]::new));
135147

136148
Consumer<Object> check = o -> {};
@@ -141,7 +153,7 @@ static SaveValuesCase structCase(String name, Map<MemoryLayout, Object> fields)
141153
check = check.andThen(o -> assertEquals(fieldHandle.get(o, 0L), value));
142154
}
143155

144-
return new SaveValuesCase("set_errno_" + name, FunctionDescriptor.of(layout, JAVA_INT), "errno", check);
156+
adder.addCase("set_errno_" + name, FunctionDescriptor.of(layout, JAVA_INT), "errno", check);
145157
}
146158

147159
@DataProvider

0 commit comments

Comments
 (0)
Please sign in to comment.