Skip to content

Commit

Permalink
Fix byte array element type in the bytecode transformation code (#357)
Browse files Browse the repository at this point in the history 
---------

Signed-off-by: Evgeniy Moiseenko <[email protected]>
  • Loading branch information
@eupp
eupp authored Aug 8, 2024
1 parent 0ee6850 commit 49454e7
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 15 deletions.
16 changes: 11 additions & 5 deletions src/jvm/main/org/jetbrains/kotlinx/lincheck/transformation/LincheckClassVisitor.kt
View file
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
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
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

0 comments on commit 49454e7

Please sign in to comment.