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.

Sign up for GitHub

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

Fix byte array element type in the bytecode transformation code #357

Merged
merged 4 commits into from
Aug 8, 2024
Merged

Fix byte array element type in the bytecode transformation code #357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
06b54db
use BYTE_ARRAY_TYPE for BALOAD/BASTORE instructions in SharedMemoryAc…
eupp Aug 7, 2024
1b16143
determine BYTE_TYPE or BOOLEAN_TYPE for BALOAD/BASTORE instructions u…
eupp Aug 8, 2024
bb87c40
fix `VerifyError`-s due to incorrect usage of `AnalyzerAdapter`
eupp Aug 8, 2024
55075b9
fix typo
eupp Aug 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions src/jvm/main/org/jetbrains/kotlinx/lincheck/transformation/LincheckClassVisitor.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,28 +108,34 @@ internal class LincheckClassVisitor(
if (access and ACC_SYNCHRONIZED != 0) {
mv = SynchronizedMethodTransformer(fileName, className, methodName, mv.newAdapter(), classVersion)
}
val skipVisitor: MethodVisitor = mv // Must not capture `MethodCallTransformer` (in order to filter static method calls inserted by coverage library)
// `skipVisitor` must not capture `MethodCallTransformer`
// (to filter static method calls inserted by coverage library)
val skipVisitor: MethodVisitor = mv
mv = MethodCallTransformer(fileName, className, methodName, mv.newAdapter())
mv = MonitorTransformer(fileName, className, methodName, mv.newAdapter())
mv = WaitNotifyTransformer(fileName, className, methodName, mv.newAdapter())
mv = ParkingTransformer(fileName, className, methodName, mv.newAdapter())
mv = ObjectCreationTransformer(fileName, className, methodName, mv.newAdapter())
mv = DeterministicHashCodeTransformer(fileName, className, methodName, mv.newAdapter())
mv = DeterministicTimeTransformer(mv.newAdapter())
mv = DeterministicRandomTransformer(fileName, className, methodName, mv.newAdapter())
mv = UnsafeMethodTransformer(fileName, className, methodName, mv.newAdapter())
mv = AtomicFieldUpdaterMethodTransformer(fileName, className, methodName, mv.newAdapter())
mv = VarHandleMethodTransformer(fileName, className, methodName, mv.newAdapter())
// `SharedMemoryAccessTransformer` goes first because it relies on `AnalyzerAdapter`,
// which should be put in front of the byte-code transformer chain,
// so that it can correctly analyze the byte-code and compute required type-information
mv = run {
val sv = SharedMemoryAccessTransformer(fileName, className, methodName, mv.newAdapter())
val aa = AnalyzerAdapter(className, access, methodName, desc, sv)
sv.analyzer = aa
aa
}
// Must appear in code after `SharedMemoryAccessTransformer` (to be able to skip this transformer)
mv = CoverageBytecodeFilter(
skipVisitor.newAdapter(),
mv.newAdapter()
) // Must appear in code after `SharedMemoryAccessTransformer` (in order to be able to skip this transformer
mv = DeterministicHashCodeTransformer(fileName, className, methodName, mv.newAdapter())
mv = DeterministicTimeTransformer(mv.newAdapter())
mv = DeterministicRandomTransformer(fileName, className, methodName, mv.newAdapter())
)
return mv
}

Expand Down
24 changes: 18 additions & 6 deletions ...g/jetbrains/kotlinx/lincheck/transformation/transformers/SharedMemoryAccessTransformer.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,25 +271,37 @@ internal class SharedMemoryAccessTransformer(
// STACK: value
}

/*
* For an array access instruction (either load or store),
* tries to obtain the type of the read/written array element.
*
* If the type can be determined from the opcode of the instruction itself
* (e.g., IALOAD/IASTORE) returns it immediately.
*
* Otherwise, queries the analyzer to determine the type of the array in the respective stack slot.
* This is used in two cases:
* - for `BALOAD` and `BASTORE` instructions, since they are used to access both boolean and byte arrays;
* - for `AALOAD` and `AASTORE` instructions, to get the class name of the array elements.
*/
private fun getArrayElementType(opcode: Int): Type = when (opcode) {
// Load
AALOAD -> getArrayAccessTypeFromStack(2) // OBJECT_TYPE
IALOAD -> INT_TYPE
FALOAD -> FLOAT_TYPE
BALOAD -> BOOLEAN_TYPE
CALOAD -> CHAR_TYPE
SALOAD -> SHORT_TYPE
LALOAD -> LONG_TYPE
DALOAD -> DOUBLE_TYPE
BALOAD -> getArrayAccessTypeFromStack(2) ?: BYTE_TYPE
AALOAD -> getArrayAccessTypeFromStack(2) ?: OBJECT_TYPE
// Store
AASTORE -> getArrayAccessTypeFromStack(3) // OBJECT_TYPE
IASTORE -> INT_TYPE
FASTORE -> FLOAT_TYPE
BASTORE -> BOOLEAN_TYPE
CASTORE -> CHAR_TYPE
SASTORE -> SHORT_TYPE
LASTORE -> LONG_TYPE
DASTORE -> DOUBLE_TYPE
BASTORE -> getArrayAccessTypeFromStack(3) ?: BYTE_TYPE
AASTORE -> getArrayAccessTypeFromStack(3) ?: OBJECT_TYPE
else -> throw IllegalStateException("Unexpected opcode: $opcode")
}

Expand All @@ -300,8 +312,8 @@ internal class SharedMemoryAccessTransformer(
* If the analyzer does not know the type, then return null
* (according to the ASM docs, this can happen, for example, when the visited instruction is unreachable).
*/
private fun getArrayAccessTypeFromStack(position: Int): Type {
if (analyzer.stack == null) return OBJECT_TYPE // better than throwing an exception
private fun getArrayAccessTypeFromStack(position: Int): Type? {
if (analyzer.stack == null) return null
val arrayDesc = analyzer.stack[analyzer.stack.size - position]
check(arrayDesc is String)
val arrayType = getType(arrayDesc)
Expand Down
8 changes: 4 additions & 4 deletions src/jvm/test/resources/expected_logs/array_name_in_trace.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Detailed trace:
| charArray.READ: CharArray#1 at ArrayNameInTraceRepresentationTest.readActions(OwnerNameInTraceRepresentationTests.kt:76) | |
| CharArray#1[0].READ: 1 at ArrayNameInTraceRepresentationTest.readActions(OwnerNameInTraceRepresentationTests.kt:76) | |
| byteArray.READ: ByteArray#1 at ArrayNameInTraceRepresentationTest.readActions(OwnerNameInTraceRepresentationTests.kt:77) | |
| ByteArray#1[0].READ: true at ArrayNameInTraceRepresentationTest.readActions(OwnerNameInTraceRepresentationTests.kt:77) | |
| ByteArray#1[0].READ: 1 at ArrayNameInTraceRepresentationTest.readActions(OwnerNameInTraceRepresentationTests.kt:77) | |
| booleanArray.READ: BooleanArray#1 at ArrayNameInTraceRepresentationTest.readActions(OwnerNameInTraceRepresentationTests.kt:78) | |
| BooleanArray#1[0].READ: true at ArrayNameInTraceRepresentationTest.readActions(OwnerNameInTraceRepresentationTests.kt:78) | |
| FloatArray#1[0].READ: 1.0 at ArrayNameInTraceRepresentationTest.readActions(OwnerNameInTraceRepresentationTests.kt:80) | |
Expand All @@ -51,7 +51,7 @@ Detailed trace:
| charArray.READ: CharArray#1 at ArrayNameInTraceRepresentationTest.writeActions(OwnerNameInTraceRepresentationTests.kt:88) | |
| CharArray#1[0].WRITE(0) at ArrayNameInTraceRepresentationTest.writeActions(OwnerNameInTraceRepresentationTests.kt:88) | |
| byteArray.READ: ByteArray#1 at ArrayNameInTraceRepresentationTest.writeActions(OwnerNameInTraceRepresentationTests.kt:89) | |
| ByteArray#1[0].WRITE(false) at ArrayNameInTraceRepresentationTest.writeActions(OwnerNameInTraceRepresentationTests.kt:89) | |
| ByteArray#1[0].WRITE(0) at ArrayNameInTraceRepresentationTest.writeActions(OwnerNameInTraceRepresentationTests.kt:89) | |
| booleanArray.READ: BooleanArray#1 at ArrayNameInTraceRepresentationTest.writeActions(OwnerNameInTraceRepresentationTests.kt:90) | |
| BooleanArray#1[0].WRITE(false) at ArrayNameInTraceRepresentationTest.writeActions(OwnerNameInTraceRepresentationTests.kt:90) | |
| FloatArray#1[0].WRITE(0.0) at ArrayNameInTraceRepresentationTest.writeActions(OwnerNameInTraceRepresentationTests.kt:92) | |
Expand All @@ -68,7 +68,7 @@ Detailed trace:
| | charArray.READ: CharArray#1 at ArrayNameInTraceRepresentationTest.readActions(OwnerNameInTraceRepresentationTests.kt:76) |
| | CharArray#1[0].READ: 0 at ArrayNameInTraceRepresentationTest.readActions(OwnerNameInTraceRepresentationTests.kt:76) |
| | byteArray.READ: ByteArray#1 at ArrayNameInTraceRepresentationTest.readActions(OwnerNameInTraceRepresentationTests.kt:77) |
| | ByteArray#1[0].READ: false at ArrayNameInTraceRepresentationTest.readActions(OwnerNameInTraceRepresentationTests.kt:77) |
| | ByteArray#1[0].READ: 0 at ArrayNameInTraceRepresentationTest.readActions(OwnerNameInTraceRepresentationTests.kt:77) |
| | booleanArray.READ: BooleanArray#1 at ArrayNameInTraceRepresentationTest.readActions(OwnerNameInTraceRepresentationTests.kt:78) |
| | BooleanArray#1[0].READ: false at ArrayNameInTraceRepresentationTest.readActions(OwnerNameInTraceRepresentationTests.kt:78) |
| | FloatArray#1[0].READ: 0.0 at ArrayNameInTraceRepresentationTest.readActions(OwnerNameInTraceRepresentationTests.kt:80) |
Expand All @@ -82,7 +82,7 @@ Detailed trace:
| | charArray.READ: CharArray#1 at ArrayNameInTraceRepresentationTest.writeActions(OwnerNameInTraceRepresentationTests.kt:88) |
| | CharArray#1[0].WRITE(0) at ArrayNameInTraceRepresentationTest.writeActions(OwnerNameInTraceRepresentationTests.kt:88) |
| | byteArray.READ: ByteArray#1 at ArrayNameInTraceRepresentationTest.writeActions(OwnerNameInTraceRepresentationTests.kt:89) |
| | ByteArray#1[0].WRITE(false) at ArrayNameInTraceRepresentationTest.writeActions(OwnerNameInTraceRepresentationTests.kt:89) |
| | ByteArray#1[0].WRITE(0) at ArrayNameInTraceRepresentationTest.writeActions(OwnerNameInTraceRepresentationTests.kt:89) |
| | booleanArray.READ: BooleanArray#1 at ArrayNameInTraceRepresentationTest.writeActions(OwnerNameInTraceRepresentationTests.kt:90) |
| | BooleanArray#1[0].WRITE(false) at ArrayNameInTraceRepresentationTest.writeActions(OwnerNameInTraceRepresentationTests.kt:90) |
| | FloatArray#1[0].WRITE(0.0) at ArrayNameInTraceRepresentationTest.writeActions(OwnerNameInTraceRepresentationTests.kt:92) |
Expand Down