From 921449cbb8bf8150d49f889c65b0e550d9a2a02a Mon Sep 17 00:00:00 2001 From: chrisly42 Date: Wed, 4 Aug 2021 17:39:54 +0200 Subject: [PATCH] Refactored stuff. Added control flow information to ISA. In ISA exg is no longer treated as setting a definitive value. Added inspection find dead writes to registers. --- README.md | 2 + build.gradle | 14 +- .../plugins/m68k/asm/ConditionCode.kt | 4 +- .../intellij/plugins/m68k/asm/M68kIsa.kt | 61 ++++--- .../M68kRegisterFlowDocumentationProvider.kt | 34 +--- .../inspections/M68kDeadWriteInspection.kt | 102 ++++++++++++ .../plugins/m68k/psi/M68kAddressModeUtil.kt | 4 +- .../plugins/m68k/utils/M68kIsaUtil.kt | 25 +++ src/main/resources/META-INF/plugin.xml | 3 + .../inspectionDescriptions/M68kDeadWrite.html | 11 ++ .../inspections/AbstractInspectionTest.kt | 1 + .../M68kDeadWriteInspectionTest.kt | 157 ++++++++++++++++++ 12 files changed, 351 insertions(+), 67 deletions(-) create mode 100644 src/main/java/de/platon42/intellij/plugins/m68k/inspections/M68kDeadWriteInspection.kt create mode 100644 src/main/resources/inspectionDescriptions/M68kDeadWrite.html create mode 100644 src/test/java/de/platon42/intellij/plugins/m68k/inspections/M68kDeadWriteInspectionTest.kt diff --git a/README.md b/README.md index 3b9d92c..61fb362 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,8 @@ make it work with JUnit 5. Feel free to use the code (in package ```de.platon42. - Bugfix: Added alternate condition code tests `HS (=CC)` and `LO (=CS)`. - Performance: Optimized mnemonic lookup. - Enhancement: Reworked Instruction Documentation provider, now shows condition codes. +- Bugfix: In ISA `exg` is no longer treated as setting a definitive value. +- New: Added inspection find dead writes to registers! ### V0.4 (03-Aug-21) diff --git a/build.gradle b/build.gradle index 6ef5c2f..fb37bc7 100644 --- a/build.gradle +++ b/build.gradle @@ -65,6 +65,8 @@ patchPluginXml {
  • Bugfix: Added alternate condition code tests HS (=CC) and LO (=CS).
  • Performance: Optimized mnemonic lookup.
  • Enhancement: Reworked Instruction Documentation provider, now shows condition codes. +
  • Bugfix: In ISA exg is no longer treated as setting a definitive value. +
  • New: Added inspection find dead writes to registers!

    V0.4 (03-Aug-21)

    -

    V0.3 (28-Jul-21)

    -

    Full changelog available at Github project site.

    """) } diff --git a/src/main/java/de/platon42/intellij/plugins/m68k/asm/ConditionCode.kt b/src/main/java/de/platon42/intellij/plugins/m68k/asm/ConditionCode.kt index 3696a87..7fde8f0 100644 --- a/src/main/java/de/platon42/intellij/plugins/m68k/asm/ConditionCode.kt +++ b/src/main/java/de/platon42/intellij/plugins/m68k/asm/ConditionCode.kt @@ -187,12 +187,12 @@ enum class ConditionCode(val cc: String, val testedCc: Int) { fun getCcFromName(cc: String) = NAME_TO_CC_MAP[cc.lowercase()]!! - fun getCcFromMnemonic(mnemonic: String) = + fun getCcFromMnemonic(originalMnemonic: String, mnemonic: String) = // handle special case for dbra if (mnemonic.equals("dbra", ignoreCase = true)) { FALSE } else { - NAME_TO_CC_MAP[mnemonic.substring(mnemonic.length - 2).lowercase()]!! + NAME_TO_CC_MAP[mnemonic.removePrefix(originalMnemonic.removeSuffix("CC")).lowercase()]!! } } } diff --git a/src/main/java/de/platon42/intellij/plugins/m68k/asm/M68kIsa.kt b/src/main/java/de/platon42/intellij/plugins/m68k/asm/M68kIsa.kt index 9b7df89..ced94df 100644 --- a/src/main/java/de/platon42/intellij/plugins/m68k/asm/M68kIsa.kt +++ b/src/main/java/de/platon42/intellij/plugins/m68k/asm/M68kIsa.kt @@ -72,11 +72,13 @@ const val RWM_READ_OPSIZE = 0x800 const val RWM_READ_B = 0x900 const val RWM_READ_W = 0xb00 const val RWM_READ_L = 0xf00 +const val RWM_READ_SHIFT = 8 -const val RWM_MODIFY_OPSIZE = 0x880 -const val RWM_MODIFY_B = 0x990 -const val RWM_MODIFY_W = 0xbb0 -const val RWM_MODIFY_L = 0xff0 +const val RWM_MODIFY_OPSIZE = 0x080 +const val RWM_MODIFY_B = 0x090 +const val RWM_MODIFY_W = 0x0b0 +const val RWM_MODIFY_L = 0x0f0 +const val RWM_MODIFY_SHIFT = 4 const val RWM_OP1_SHIFT = 0 const val RWM_OP2_SHIFT = 12 @@ -133,6 +135,7 @@ data class IsaData( val isPrivileged: Boolean = false, val hasOps: Boolean = true, val modes: List = listOf(AllowedAdrMode()), + val changesControlFlow: Boolean = false ) object M68kIsa { @@ -389,7 +392,7 @@ object M68kIsa { setOf(AddressMode.DATA_REGISTER_DIRECT, AddressMode.ADDRESS_REGISTER_DIRECT), setOf(AddressMode.DATA_REGISTER_DIRECT, AddressMode.ADDRESS_REGISTER_DIRECT), OP_SIZE_L, - modInfo = RWM_SET_OP1_L or RWM_SET_OP2_L + modInfo = RWM_MODIFY_OP1_L or RWM_MODIFY_OP2_L // exchanging registers does not set value to a defined state ) ) ), @@ -687,13 +690,19 @@ object M68kIsa { // Program Control Instructions IsaData( "bCC", "Branch Conditionally", conditionCodes = conditionCodesBcc, - modes = listOf(AllowedAdrMode(setOf(AddressMode.ABSOLUTE_ADDRESS), null, OP_SIZE_SBW, testedCc = cc("-????"))) + modes = listOf(AllowedAdrMode(setOf(AddressMode.ABSOLUTE_ADDRESS), null, OP_SIZE_SBW, testedCc = cc("-????"))), + changesControlFlow = true + ), + IsaData( + "bra", "Branch", + modes = listOf(AllowedAdrMode(setOf(AddressMode.ABSOLUTE_ADDRESS), null, OP_SIZE_SBW)), + changesControlFlow = true ), - IsaData("bra", "Branch", modes = listOf(AllowedAdrMode(setOf(AddressMode.ABSOLUTE_ADDRESS), null, OP_SIZE_SBW))), IsaData( "bsr", "Branch to Subroutine", - modes = listOf(AllowedAdrMode(setOf(AddressMode.ABSOLUTE_ADDRESS), null, OP_SIZE_SBW, modInfo = RWM_MODIFY_STACK)) + modes = listOf(AllowedAdrMode(setOf(AddressMode.ABSOLUTE_ADDRESS), null, OP_SIZE_SBW, modInfo = RWM_MODIFY_STACK)), + changesControlFlow = true ), IsaData( @@ -701,12 +710,8 @@ object M68kIsa { "Test Condition, Decrement, and Branch", altMnemonics = listOf("dbra"), conditionCodes = conditionCodes, - modes = listOf( - AllowedAdrMode( - DREG_ONLY, setOf(AddressMode.ABSOLUTE_ADDRESS), OP_SIZE_W, modInfo = RWM_MODIFY_OP1_W, - testedCc = cc("-????") - ) - ) + modes = listOf(AllowedAdrMode(DREG_ONLY, setOf(AddressMode.ABSOLUTE_ADDRESS), OP_SIZE_W, modInfo = RWM_MODIFY_OP1_W, testedCc = cc("-????"))), + changesControlFlow = true ), IsaData( "sCC", "Set Conditionally", conditionCodes = conditionCodes, @@ -726,7 +731,8 @@ object M68kIsa { AddressMode.PROGRAM_COUNTER_INDIRECT_WITH_INDEX ), null, OP_UNSIZED ) - ) + ), + changesControlFlow = true ), IsaData( "jsr", "Jump to Subroutine", @@ -741,7 +747,8 @@ object M68kIsa { AddressMode.PROGRAM_COUNTER_INDIRECT_WITH_INDEX ), null, OP_UNSIZED, modInfo = RWM_MODIFY_STACK ) - ) + ), + changesControlFlow = true ), IsaData("nop", "No Operation", hasOps = false, modes = NO_OPS_UNSIZED), @@ -749,9 +756,14 @@ object M68kIsa { "rtr", "Return and Restore", hasOps = false, - modes = listOf(AllowedAdrMode(size = OP_UNSIZED, modInfo = RWM_MODIFY_STACK, affectedCc = cc("*****"))) + modes = listOf(AllowedAdrMode(size = OP_UNSIZED, modInfo = RWM_MODIFY_STACK, affectedCc = cc("*****"))), + changesControlFlow = true + ), + IsaData( + "rts", "Return from Subroutine", hasOps = false, + modes = listOf(AllowedAdrMode(size = OP_UNSIZED, modInfo = RWM_MODIFY_STACK)), + changesControlFlow = true ), - IsaData("rts", "Return from Subroutine", hasOps = false, modes = listOf(AllowedAdrMode(size = OP_UNSIZED, modInfo = RWM_MODIFY_STACK))), IsaData( "tst", "Test Operand", modes = listOf( @@ -823,7 +835,8 @@ object M68kIsa { IsaData("reset", "Reset External Devices", isPrivileged = true, hasOps = false, modes = NO_OPS_UNSIZED), IsaData( "rte", "Return from Exception", isPrivileged = true, hasOps = false, - modes = listOf(AllowedAdrMode(size = OP_UNSIZED, modInfo = RWM_MODIFY_STACK)) + modes = listOf(AllowedAdrMode(size = OP_UNSIZED, modInfo = RWM_MODIFY_STACK)), + changesControlFlow = true ), IsaData( "stop", "Stop", isPrivileged = true, @@ -834,9 +847,13 @@ object M68kIsa { "chk", "Check Register Against Bound", modes = listOf(AllowedAdrMode(ALL_EXCEPT_AREG, DREG_ONLY, OP_SIZE_W, modInfo = RWM_READ_OP1_W or RWM_READ_OP2_W, affectedCc = cc("-*UUU"))) ), - IsaData("illegal", "Take Illegal Instruction Trap", hasOps = false, modes = NO_OPS_UNSIZED), - IsaData("trap", "Trap", modes = listOf(AllowedAdrMode(setOf(AddressMode.IMMEDIATE_DATA), null, OP_UNSIZED))), - IsaData("trapv", "Trap on Overflow", hasOps = false, modes = NO_OPS_UNSIZED), + IsaData("illegal", "Take Illegal Instruction Trap", hasOps = false, modes = NO_OPS_UNSIZED, changesControlFlow = true), + IsaData("trap", "Trap", modes = listOf(AllowedAdrMode(setOf(AddressMode.IMMEDIATE_DATA), null, OP_UNSIZED)), changesControlFlow = true), + IsaData( + "trapv", "Trap on Overflow", hasOps = false, + modes = listOf(AllowedAdrMode(size = OP_UNSIZED, testedCc = cc("---?-"))), + changesControlFlow = true + ), IsaData( "andi", "AND Immediate to Condition Code Register", id = "andi to CCR", altMnemonics = listOf("and"), diff --git a/src/main/java/de/platon42/intellij/plugins/m68k/documentation/M68kRegisterFlowDocumentationProvider.kt b/src/main/java/de/platon42/intellij/plugins/m68k/documentation/M68kRegisterFlowDocumentationProvider.kt index ad51270..87dd973 100644 --- a/src/main/java/de/platon42/intellij/plugins/m68k/documentation/M68kRegisterFlowDocumentationProvider.kt +++ b/src/main/java/de/platon42/intellij/plugins/m68k/documentation/M68kRegisterFlowDocumentationProvider.kt @@ -13,8 +13,8 @@ import com.intellij.ui.JBColor import de.platon42.intellij.plugins.m68k.asm.* import de.platon42.intellij.plugins.m68k.asm.Register.Companion.getRegFromName import de.platon42.intellij.plugins.m68k.psi.* -import de.platon42.intellij.plugins.m68k.psi.M68kAddressModeUtil.getOtherReadWriteModifyRegisters -import de.platon42.intellij.plugins.m68k.psi.M68kAddressModeUtil.getReadWriteModifyRegisters +import de.platon42.intellij.plugins.m68k.utils.M68kIsaUtil.checkIfInstructionUsesRegister +import de.platon42.intellij.plugins.m68k.utils.M68kIsaUtil.evaluateRegisterUse import de.platon42.intellij.plugins.m68k.utils.M68kIsaUtil.findExactIsaDataAndAllowedAdrModeForInstruction import de.platon42.intellij.plugins.m68k.utils.M68kIsaUtil.getOpSizeOrDefault import de.platon42.intellij.plugins.m68k.utils.M68kIsaUtil.modifyRwmWithOpsize @@ -64,10 +64,10 @@ class M68kRegisterFlowDocumentationProvider : AbstractDocumentationProvider() { ) 0 } else { - (RWM_SET_L and RWM_SIZE_MASK) and totalRwm.inv() + RWM_SIZE_MASK and totalRwm.inv() } } else { - (RWM_SET_L and RWM_SIZE_MASK) and ((cursorRwm and RWM_MODIFY_L) ushr 8) + RWM_SIZE_MASK and (((cursorRwm and RWM_MODIFY_L) ushr RWM_MODIFY_SHIFT) or ((cursorRwm and RWM_READ_L) ushr RWM_READ_SHIFT)) } val initialStatement: M68kStatement = asmInstruction.parent as M68kStatement val localLabelName = PsiTreeUtil.findChildOfType(initialStatement, M68kLocalLabel::class.java)?.name ?: "-->" @@ -85,7 +85,7 @@ class M68kRegisterFlowDocumentationProvider : AbstractDocumentationProvider() { ) }) backtrace.reverse() - val traceBits = (cursorRwm or (cursorRwm ushr 8)) and RWM_SIZE_MASK + val traceBits = (cursorRwm or (cursorRwm ushr RWM_MODIFY_SHIFT) or (cursorRwm ushr RWM_READ_SHIFT)) and RWM_SIZE_MASK backtrace.addAll(analyseFlow(register, traceBits, false, initialStatement, linesLimit) { PsiTreeUtil.getNextSiblingOfType( it, @@ -210,30 +210,6 @@ class M68kRegisterFlowDocumentationProvider : AbstractDocumentationProvider() { ) .children(DocumentationMarkup.SECTION_CONTENT_CELL.child(HtmlChunk.nbsp())) - private fun evaluateRegisterUse( - asmInstruction: M68kAsmInstruction, - adrMode: AllowedAdrMode, - register: Register - ): List { - val opSize = getOpSizeOrDefault(asmInstruction.asmOp.opSize, adrMode) - - val rwm1 = modifyRwmWithOpsize((adrMode.modInfo ushr RWM_OP1_SHIFT) and RWM_OP_MASK, opSize) - val rwm2 = if (asmInstruction.addressingModeList.size > 1) modifyRwmWithOpsize((adrMode.modInfo ushr RWM_OP2_SHIFT) and RWM_OP_MASK, opSize) else 0 - return getReadWriteModifyRegisters(asmInstruction.addressingModeList[0], rwm1).asSequence() - .plus(getReadWriteModifyRegisters(asmInstruction.addressingModeList.getOrNull(1), rwm2)) - .plus(getOtherReadWriteModifyRegisters(adrMode.modInfo)) - .filter { it.first == register } - .map { it.second } - .toList() - } - - private fun checkIfInstructionUsesRegister(instruction: M68kAsmInstruction, register: Register): Boolean { - if (instruction.addressingModeList.isEmpty()) { - return false - } - return instruction.addressingModeList.any { aml -> getReadWriteModifyRegisters(aml, 0).any { it.first == register } } - } - override fun getCustomDocumentationElement(editor: Editor, file: PsiFile, contextElement: PsiElement?, targetOffset: Int): PsiElement? { if (contextElement == null) return null if (contextElement is M68kDataRegister || contextElement is M68kAddressRegister) return contextElement diff --git a/src/main/java/de/platon42/intellij/plugins/m68k/inspections/M68kDeadWriteInspection.kt b/src/main/java/de/platon42/intellij/plugins/m68k/inspections/M68kDeadWriteInspection.kt new file mode 100644 index 0000000..678c2de --- /dev/null +++ b/src/main/java/de/platon42/intellij/plugins/m68k/inspections/M68kDeadWriteInspection.kt @@ -0,0 +1,102 @@ +package de.platon42.intellij.plugins.m68k.inspections + +import com.intellij.codeInspection.InspectionManager +import com.intellij.codeInspection.ProblemDescriptor +import com.intellij.codeInspection.ProblemHighlightType +import com.intellij.psi.util.PsiTreeUtil +import com.intellij.util.SmartList +import de.platon42.intellij.plugins.m68k.asm.* +import de.platon42.intellij.plugins.m68k.asm.M68kIsa.findMatchingInstructions +import de.platon42.intellij.plugins.m68k.psi.M68kAddressModeUtil +import de.platon42.intellij.plugins.m68k.psi.M68kAsmInstruction +import de.platon42.intellij.plugins.m68k.psi.M68kGlobalLabel +import de.platon42.intellij.plugins.m68k.psi.M68kStatement +import de.platon42.intellij.plugins.m68k.utils.M68kIsaUtil +import de.platon42.intellij.plugins.m68k.utils.M68kIsaUtil.checkIfInstructionUsesRegister +import de.platon42.intellij.plugins.m68k.utils.M68kIsaUtil.evaluateRegisterUse +import de.platon42.intellij.plugins.m68k.utils.M68kIsaUtil.findExactIsaDataAndAllowedAdrModeForInstruction +import de.platon42.intellij.plugins.m68k.utils.M68kIsaUtil.getConcreteTestedCcFromMnemonic + +class M68kDeadWriteInspection : AbstractBaseM68kLocalInspectionTool() { + + companion object { + private const val DISPLAY_NAME = "Dead writes to registers" + + private const val DEAD_WRITE_MSG = "Register %s is overwritten later without being used" + private const val POSSIBLY_DEAD_WRITE_MSG = "Register %s is overwritten later (only CC evaluated?)" + } + + override fun getDisplayName() = DISPLAY_NAME + + override fun checkAsmInstruction(asmInstruction: M68kAsmInstruction, manager: InspectionManager, isOnTheFly: Boolean): Array? { + val asmOp = asmInstruction.asmOp + if (asmInstruction.addressingModeList.isEmpty()) return emptyArray() + + val isaDataCandidates = findMatchingInstructions(asmOp.mnemonic) + if (isaDataCandidates.isEmpty()) return emptyArray() + val (_, adrMode) = findExactIsaDataAndAllowedAdrModeForInstruction(asmInstruction) ?: return emptyArray() + + val opSize = M68kIsaUtil.getOpSizeOrDefault(asmInstruction.asmOp.opSize, adrMode) + val rwm1 = M68kIsaUtil.modifyRwmWithOpsize((adrMode.modInfo ushr RWM_OP1_SHIFT) and RWM_OP_MASK, opSize) + val rwm2 = if (asmInstruction.addressingModeList.size > 1) M68kIsaUtil.modifyRwmWithOpsize( + (adrMode.modInfo ushr RWM_OP2_SHIFT) and RWM_OP_MASK, opSize + ) else 0 + val regsWritten = M68kAddressModeUtil.getReadWriteModifyRegisters(asmInstruction.addressingModeList[0], rwm1).asSequence() + .plus(M68kAddressModeUtil.getReadWriteModifyRegisters(asmInstruction.addressingModeList.getOrNull(1), rwm2)) + .plus(M68kAddressModeUtil.getOtherReadWriteModifyRegisters(adrMode.modInfo)) + .filter { (it.second and RWM_SET_L) > 0 } + .distinct() + .toList() + + val hints = SmartList() + for (regPair in regsWritten) { + val register = regPair.first + val rwm = regPair.second + var currStatement = asmInstruction.parent as M68kStatement + + var ccModification = adrMode.affectedCc + var ccOverwritten = false + var ccTested = false + var hasModification = false + + while (true) { + currStatement = PsiTreeUtil.getNextSiblingOfType(currStatement, M68kStatement::class.java) ?: break + val globalLabel = PsiTreeUtil.findChildOfType(currStatement, M68kGlobalLabel::class.java) + if (globalLabel != null) break + + val currAsmInstruction = PsiTreeUtil.getChildOfType(currStatement, M68kAsmInstruction::class.java) ?: continue + val (isaData, currAdrMode) = findExactIsaDataAndAllowedAdrModeForInstruction(currAsmInstruction) ?: continue + if (isaData.changesControlFlow) break + val testedCc = getConcreteTestedCcFromMnemonic(currAsmInstruction.asmOp.mnemonic, isaData, adrMode) + if (((testedCc and ccModification) > 0) && !ccOverwritten) ccTested = true + if (currAdrMode.affectedCc != 0) ccOverwritten = true + if (checkIfInstructionUsesRegister(currAsmInstruction, register)) { + val totalRwms = evaluateRegisterUse(currAsmInstruction, currAdrMode, register).reduce(Int::or) + if (totalRwms and RWM_READ_L > 0) break + if (totalRwms and RWM_MODIFY_L > 0) { + hasModification = true + ccOverwritten = false + ccModification = ccModification or currAdrMode.affectedCc + } + if (totalRwms and RWM_SET_L >= rwm) { + if (ccTested && hasModification) { + break + } + hints.add( + manager.createProblemDescriptor( + asmInstruction, + asmInstruction, + (if (ccTested) POSSIBLY_DEAD_WRITE_MSG else DEAD_WRITE_MSG).format(register.regname), + if (ccTested) ProblemHighlightType.WEAK_WARNING else ProblemHighlightType.WARNING, + isOnTheFly + ) + ) + break + } + } + } + + } + return hints.toTypedArray() + } +} \ No newline at end of file diff --git a/src/main/java/de/platon42/intellij/plugins/m68k/psi/M68kAddressModeUtil.kt b/src/main/java/de/platon42/intellij/plugins/m68k/psi/M68kAddressModeUtil.kt index 92e6d8e..d0f51f7 100644 --- a/src/main/java/de/platon42/intellij/plugins/m68k/psi/M68kAddressModeUtil.kt +++ b/src/main/java/de/platon42/intellij/plugins/m68k/psi/M68kAddressModeUtil.kt @@ -43,8 +43,8 @@ object M68kAddressModeUtil { is M68kProgramCounterIndirectWithDisplacementOldAddressingMode, is M68kAbsoluteAddressAddressingMode -> emptyList() - is M68kAddressRegisterIndirectPostIncAddressingMode -> listOf(Register.getRegFromName(addressingMode.addressRegister.text) to RWM_MODIFY_L) - is M68kAddressRegisterIndirectPreDecAddressingMode -> listOf(Register.getRegFromName(addressingMode.addressRegister.text) to RWM_MODIFY_L) + is M68kAddressRegisterIndirectPostIncAddressingMode -> listOf(Register.getRegFromName(addressingMode.addressRegister.text) to (RWM_READ_L or RWM_MODIFY_L)) + is M68kAddressRegisterIndirectPreDecAddressingMode -> listOf(Register.getRegFromName(addressingMode.addressRegister.text) to (RWM_READ_L or RWM_MODIFY_L)) is M68kWithAddressRegisterIndirect -> { if (addressingMode is M68kWithIndexRegister) { listOf( diff --git a/src/main/java/de/platon42/intellij/plugins/m68k/utils/M68kIsaUtil.kt b/src/main/java/de/platon42/intellij/plugins/m68k/utils/M68kIsaUtil.kt index 747e2a9..2872da9 100644 --- a/src/main/java/de/platon42/intellij/plugins/m68k/utils/M68kIsaUtil.kt +++ b/src/main/java/de/platon42/intellij/plugins/m68k/utils/M68kIsaUtil.kt @@ -1,6 +1,7 @@ package de.platon42.intellij.plugins.m68k.utils import de.platon42.intellij.plugins.m68k.asm.* +import de.platon42.intellij.plugins.m68k.asm.ConditionCode.Companion.getCcFromMnemonic import de.platon42.intellij.plugins.m68k.psi.M68kAddressModeUtil import de.platon42.intellij.plugins.m68k.psi.M68kAsmInstruction import de.platon42.intellij.plugins.m68k.psi.M68kSpecialRegisterDirectAddressingMode @@ -34,6 +35,30 @@ object M68kIsaUtil { return matchedIsaData.map { it to M68kIsa.findMatchingAddressMode(it.modes, op1, op2, opSize, specialReg) } } + fun checkIfInstructionUsesRegister(instruction: M68kAsmInstruction, register: Register): Boolean { + if (instruction.addressingModeList.isEmpty()) { + return false + } + return instruction.addressingModeList.any { aml -> M68kAddressModeUtil.getReadWriteModifyRegisters(aml, 0).any { it.first == register } } + } + + fun evaluateRegisterUse(asmInstruction: M68kAsmInstruction, adrMode: AllowedAdrMode, register: Register): List { + val opSize = getOpSizeOrDefault(asmInstruction.asmOp.opSize, adrMode) + + val rwm1 = modifyRwmWithOpsize((adrMode.modInfo ushr RWM_OP1_SHIFT) and RWM_OP_MASK, opSize) + val rwm2 = if (asmInstruction.addressingModeList.size > 1) modifyRwmWithOpsize((adrMode.modInfo ushr RWM_OP2_SHIFT) and RWM_OP_MASK, opSize) else 0 + return M68kAddressModeUtil.getReadWriteModifyRegisters(asmInstruction.addressingModeList[0], rwm1).asSequence() + .plus(M68kAddressModeUtil.getReadWriteModifyRegisters(asmInstruction.addressingModeList.getOrNull(1), rwm2)) + .plus(M68kAddressModeUtil.getOtherReadWriteModifyRegisters(adrMode.modInfo)) + .filter { it.first == register } + .map { it.second } + .distinct() + .toList() + } + + fun getConcreteTestedCcFromMnemonic(mnemonic: String, isaData: IsaData, adrMode: AllowedAdrMode) = + if (isaData.conditionCodes.isNotEmpty()) getCcFromMnemonic(isaData.mnemonic, mnemonic).testedCc else adrMode.testedCc + fun getOpSizeOrDefault(opSize: Int, adrMode: AllowedAdrMode): Int { if (opSize == OP_UNSIZED && (adrMode.size != OP_UNSIZED)) { return if ((adrMode.size and OP_SIZE_W) == OP_SIZE_W) { diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml index 71c2579..327a41c 100644 --- a/src/main/resources/META-INF/plugin.xml +++ b/src/main/resources/META-INF/plugin.xml @@ -50,6 +50,9 @@ + diff --git a/src/main/resources/inspectionDescriptions/M68kDeadWrite.html b/src/main/resources/inspectionDescriptions/M68kDeadWrite.html new file mode 100644 index 0000000..d6b95b1 --- /dev/null +++ b/src/main/resources/inspectionDescriptions/M68kDeadWrite.html @@ -0,0 +1,11 @@ + + +Finds dead writes to registers, i.e. writes that will not have any effect. + +Issues a weak warning if the instruction affects only condition codes that are later tested. + +Analysis is terminated at the next global label or instruction that reads the register or changes control flow. + +

    Note: As there is no evaluation of macros right now, the inspection might report some false positives.

    + + \ No newline at end of file diff --git a/src/test/java/de/platon42/intellij/plugins/m68k/inspections/AbstractInspectionTest.kt b/src/test/java/de/platon42/intellij/plugins/m68k/inspections/AbstractInspectionTest.kt index 74fdda8..e4375bb 100644 --- a/src/test/java/de/platon42/intellij/plugins/m68k/inspections/AbstractInspectionTest.kt +++ b/src/test/java/de/platon42/intellij/plugins/m68k/inspections/AbstractInspectionTest.kt @@ -11,6 +11,7 @@ import org.junit.jupiter.api.extension.ExtendWith @ExtendWith(LightCodeInsightExtension::class) @TestDataPath("src/test/resources/inspections") abstract class AbstractInspectionTest : AbstractM68kTest() { + protected fun assertHighlightings(myFixture: CodeInsightTestFixture, count: Int, snippet: String) { assertThat(myFixture.doHighlighting()) .areExactly(count, Condition({ it.description?.contains(snippet) ?: false }, "containing")) diff --git a/src/test/java/de/platon42/intellij/plugins/m68k/inspections/M68kDeadWriteInspectionTest.kt b/src/test/java/de/platon42/intellij/plugins/m68k/inspections/M68kDeadWriteInspectionTest.kt new file mode 100644 index 0000000..e51952d --- /dev/null +++ b/src/test/java/de/platon42/intellij/plugins/m68k/inspections/M68kDeadWriteInspectionTest.kt @@ -0,0 +1,157 @@ +package de.platon42.intellij.plugins.m68k.inspections + +import com.intellij.testFramework.fixtures.CodeInsightTestFixture +import de.platon42.intellij.jupiter.MyFixture +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test + +internal class M68kDeadWriteInspectionTest : AbstractInspectionTest() { + + @Test + internal fun find_direct_dead_write_to_register(@MyFixture myFixture: CodeInsightTestFixture) { + myFixture.enableInspections(M68kDeadWriteInspection::class.java) + myFixture.configureByText( + "deadwrite.asm", """ + move.l #123,d1 + add.l d0,d2 + moveq.l #123,d1 + """ + ) + assertHighlightings(myFixture, 1, "Register d1 is overwritten later without being used") + } + + @Test + internal fun find_direct_dead_write_to_register_with_modification(@MyFixture myFixture: CodeInsightTestFixture) { + myFixture.enableInspections(M68kDeadWriteInspection::class.java) + myFixture.configureByText( + "deadwrite.asm", """ + move.l #123,d1 + add.l d0,d1 + moveq.l #123,d1 + """ + ) + assertHighlightings(myFixture, 1, "Register d1 is overwritten later without being used") + } + + @Test + internal fun no_dead_write_to_register_due_to_part_modification(@MyFixture myFixture: CodeInsightTestFixture) { + myFixture.enableInspections(M68kDeadWriteInspection::class.java) + myFixture.configureByText( + "deadwrite.asm", """ + move.l #123,d1 + add.b d1,d0 + moveq.l #123,d1 + """ + ) + assertThat(myFixture.doHighlighting()).isEmpty() + } + + @Test + internal fun use_of_condition_code_causes_weak_warning(@MyFixture myFixture: CodeInsightTestFixture) { + myFixture.enableInspections(M68kDeadWriteInspection::class.java) + myFixture.configureByText( + "deadwrite.asm", """ + move.l #123,d1 + scc d0 + moveq.l #123,d1 + """ + ) + assertHighlightings(myFixture, 1, "Register d1 is overwritten later (only CC evaluated?)") + } + + @Test + internal fun use_of_conditional_instruction_for_non_conditional_causes_full_warning(@MyFixture myFixture: CodeInsightTestFixture) { + myFixture.enableInspections(M68kDeadWriteInspection::class.java) + myFixture.configureByText( + "deadwrite.asm", """ + move.l #123,d1 + st d0 + moveq.l #123,d1 + """ + ) + assertHighlightings(myFixture, 1, "Register d1 is overwritten later without being used") + } + + @Test + internal fun use_of_condition_code_after_modification_causes_no_warning(@MyFixture myFixture: CodeInsightTestFixture) { + myFixture.enableInspections(M68kDeadWriteInspection::class.java) + myFixture.configureByText( + "deadwrite.asm", """ + move.l #123,d1 + add.b d0,d1 + seq .foo + moveq.l #123,d1 +.foo + """ + ) + assertThat(myFixture.doHighlighting()).isEmpty() + } + + @Test + internal fun use_of_control_flow_causes_no_warning(@MyFixture myFixture: CodeInsightTestFixture) { + myFixture.enableInspections(M68kDeadWriteInspection::class.java) + myFixture.configureByText( + "deadwrite.asm", """ + move.l #123,d1 + add.b d0,d1 + bra.s .foo + moveq.l #123,d1 +.foo + """ + ) + assertThat(myFixture.doHighlighting()).isEmpty() + } + + @Test + internal fun movem_can_cause_multiple_warnings(@MyFixture myFixture: CodeInsightTestFixture) { + myFixture.enableInspections(M68kDeadWriteInspection::class.java) + myFixture.configureByText( + "deadwrite.asm", """ + movem.l (a0),d0-d7 + add.l d0,d1 + moveq.l #123,d3 + clr.l #123,d6 + movem.l (a0),d5-d7 + """ + ) + assertHighlightings(myFixture, 1, "Register d5 is overwritten later without being used") + assertHighlightings(myFixture, 1, "Register d6 is overwritten later without being used") + assertHighlightings(myFixture, 1, "Register d7 is overwritten later without being used") + } + + @Test + internal fun overwrite_in_same_instruction_with_read_causes_no_warning(@MyFixture myFixture: CodeInsightTestFixture) { + myFixture.enableInspections(M68kDeadWriteInspection::class.java) + myFixture.configureByText( + "deadwrite.asm", """ + move.w #123,d3 + move.w (a0,d3.w),d3 + """ + ) + assertThat(myFixture.doHighlighting()).isEmpty() + } + + @Test + internal fun partial_overwrite_in_causes_no_warning(@MyFixture myFixture: CodeInsightTestFixture) { + myFixture.enableInspections(M68kDeadWriteInspection::class.java) + myFixture.configureByText( + "deadwrite.asm", """ + move.w #123,d3 + move.b d2,d3 + """ + ) + assertThat(myFixture.doHighlighting()).isEmpty() + } + + @Test + internal fun oversized_overwrite_in_causes_warning(@MyFixture myFixture: CodeInsightTestFixture) { + myFixture.enableInspections(M68kDeadWriteInspection::class.java) + myFixture.configureByText( + "deadwrite.asm", """ + move.w #123,d3 + move.l d2,d3 + """ + ) + assertHighlightings(myFixture, 1, "Register d3 is overwritten later without being used") + } +} \ No newline at end of file