From a3f979b48b6c5048712dc19559da28932258816f Mon Sep 17 00:00:00 2001 From: chrisly42 Date: Tue, 13 May 2025 21:12:52 +0200 Subject: [PATCH] Added M68kAddressRegisterWordAccessInspection --- README.md | 15 ++++ ...M68kAddressRegisterWordAccessInspection.kt | 69 +++++++++++++++++++ ...kGlobalLabelSymbolCompletionContributor.kt | 22 +++--- src/main/resources/META-INF/plugin.xml | 3 + ...AddressRegisterWordAccessInspectionTest.kt | 29 ++++++++ 5 files changed, 129 insertions(+), 9 deletions(-) create mode 100644 src/main/java/de/platon42/intellij/plugins/m68k/inspections/M68kAddressRegisterWordAccessInspection.kt create mode 100644 src/test/java/de/platon42/intellij/plugins/m68k/inspections/M68kAddressRegisterWordAccessInspectionTest.kt diff --git a/README.md b/README.md index 5fa9f36..b567836 100644 --- a/README.md +++ b/README.md @@ -84,6 +84,17 @@ optimize the order of execution. Points out unresolved references such for global and local labels, macros or symbols. Right now, missing symbol and global label references are shown only as weak warnings as missing macro evaluation will not resolve symbols defined via `STRUCT` macros. +#### M68kAddressRegisterWordAccessInspection - Suspicious word access to address register + +Warns on `movea.w` to address registers which is often a typo and takes ages to find and fix. +Of course there are legal intentional uses of this instruction that sign extends the word +to a long word, or using it as a temporary register or for index purposes. + +Also warns on `cmpa.w` with address register as second operand (there are some clever ways +using this, such as using `cmpa.w ax,ax` to see if a value is between $ffff8000 and $7fff). + +Third instruction checked is `tst.w` on address registers for 68020+. + ### Documentation provider #### M68kSymbolDefinition @@ -166,6 +177,10 @@ are appreciated. It really is keeping me motivated to continue development. ## Changelog +### V0.11 (unreleased) + +- New: Added M68kAddressRegisterWordAccessInspection to find suspicious uses of address registers. + ### V0.10 (20-Feb-24) - Decided to release some features that have been sitting on my harddrive for almost two years, but never got released, diff --git a/src/main/java/de/platon42/intellij/plugins/m68k/inspections/M68kAddressRegisterWordAccessInspection.kt b/src/main/java/de/platon42/intellij/plugins/m68k/inspections/M68kAddressRegisterWordAccessInspection.kt new file mode 100644 index 0000000..626e249 --- /dev/null +++ b/src/main/java/de/platon42/intellij/plugins/m68k/inspections/M68kAddressRegisterWordAccessInspection.kt @@ -0,0 +1,69 @@ +package de.platon42.intellij.plugins.m68k.inspections + +import com.intellij.codeInspection.InspectionManager +import com.intellij.codeInspection.ProblemDescriptor +import com.intellij.codeInspection.ProblemHighlightType +import de.platon42.intellij.plugins.m68k.asm.AddressMode +import de.platon42.intellij.plugins.m68k.asm.M68kIsa.findMatchingInstructions +import de.platon42.intellij.plugins.m68k.asm.OP_SIZE_W +import de.platon42.intellij.plugins.m68k.psi.M68kAddressModeUtil +import de.platon42.intellij.plugins.m68k.psi.M68kAsmInstruction +import de.platon42.intellij.plugins.m68k.utils.M68kIsaUtil.findExactIsaDataAndAllowedAdrModeForInstruction + +class M68kAddressRegisterWordAccessInspection : AbstractBaseM68kLocalInspectionTool() { + + companion object { + private const val DISPLAY_NAME = "Word access to address register" + + private const val ADDR_REG_WORD_ACCESS_MSG_TEMPLATE = "Suspicious %s %s address register" + } + + override fun getDisplayName() = DISPLAY_NAME + + override fun checkAsmInstruction(asmInstruction: M68kAsmInstruction, manager: InspectionManager, isOnTheFly: Boolean): Array? { + val asmOp = asmInstruction.asmOp + if (asmOp.opSize != OP_SIZE_W) return emptyArray() + + val isaDataCandidates = findMatchingInstructions(asmOp.mnemonic) + if (isaDataCandidates.isEmpty()) return emptyArray() + val (isaData, adrMode) = findExactIsaDataAndAllowedAdrModeForInstruction(asmInstruction) ?: return emptyArray() + + return when (isaData.mnemonic) { + "movea" -> arrayOf( + manager.createProblemDescriptor( + asmInstruction, + asmInstruction, + ADDR_REG_WORD_ACCESS_MSG_TEMPLATE.format(isaData.mnemonic, "word write to"), + ProblemHighlightType.WARNING, + isOnTheFly + ) + ) + + "cmpa" -> arrayOf( + manager.createProblemDescriptor( + asmInstruction, + asmInstruction, + ADDR_REG_WORD_ACCESS_MSG_TEMPLATE.format(isaData.mnemonic, "comparing with word-sized"), + ProblemHighlightType.WARNING, + isOnTheFly + ) + ) + + "tst" -> if (M68kAddressModeUtil.getAddressModeForType(asmInstruction.addressingModeList.getOrNull(0)) == AddressMode.ADDRESS_REGISTER_DIRECT) { + arrayOf( + manager.createProblemDescriptor( + asmInstruction, + asmInstruction, + ADDR_REG_WORD_ACCESS_MSG_TEMPLATE.format(isaData.mnemonic, "with word-sized"), + ProblemHighlightType.WARNING, + isOnTheFly + ) + ) + } else { + emptyArray() + } + + else -> emptyArray() + } + } +} diff --git a/src/main/java/de/platon42/intellij/plugins/m68k/refs/M68kGlobalLabelSymbolCompletionContributor.kt b/src/main/java/de/platon42/intellij/plugins/m68k/refs/M68kGlobalLabelSymbolCompletionContributor.kt index 9254e49..9804d62 100644 --- a/src/main/java/de/platon42/intellij/plugins/m68k/refs/M68kGlobalLabelSymbolCompletionContributor.kt +++ b/src/main/java/de/platon42/intellij/plugins/m68k/refs/M68kGlobalLabelSymbolCompletionContributor.kt @@ -12,22 +12,26 @@ import de.platon42.intellij.plugins.m68k.psi.utils.M68kLookupUtil class M68kGlobalLabelSymbolCompletionContributor : CompletionContributor() { companion object { + val REGS = listOf( + "d0", "d1", "d2", "d3", "d4", "d5", "d6", "d7", + "a0", "a1", "a2", "a3", "a4", "a5", "a6", "sp", + "pc" + ) + val REGSET = REGS.toSet() val REGISTER_SUGGESTIONS: List = - listOf( - "d0", "d1", "d2", "d3", "d4", "d5", "d6", "d7", - "a0", "a1", "a2", "a3", "a4", "a5", "a6", "sp", - "pc" - ) - .map { PrioritizedLookupElement.withPriority(LookupElementBuilder.create(it).withIcon(M68kIcons.REGISTER).withBoldness(true), 2.0) } + REGS.map { PrioritizedLookupElement.withPriority(LookupElementBuilder.create(it).withIcon(M68kIcons.REGISTER).withBoldness(true), 2.0) } } init { extend(CompletionType.BASIC, PlatformPatterns.psiElement(M68kTypes.SYMBOL), object : CompletionProvider() { override fun addCompletions(parameters: CompletionParameters, context: ProcessingContext, resultSet: CompletionResultSet) { resultSet.addAllElements(REGISTER_SUGGESTIONS) - resultSet.addAllElements(M68kLookupUtil.findAllGlobalLabels(parameters.originalFile.project).map(LookupElementBuilder::createWithIcon)) - resultSet.addAllElements(M68kLookupUtil.findAllSymbolDefinitions(parameters.originalFile.project).map(LookupElementBuilder::createWithIcon)) + val prefix = resultSet.prefixMatcher.prefix.lowercase() + if (!(prefix == "a" || prefix == "d" || REGSET.contains(prefix))) { + resultSet.addAllElements(M68kLookupUtil.findAllGlobalLabels(parameters.originalFile.project).map(LookupElementBuilder::createWithIcon)) + resultSet.addAllElements(M68kLookupUtil.findAllSymbolDefinitions(parameters.originalFile.project).map(LookupElementBuilder::createWithIcon)) + } } }) } -} \ No newline at end of file +} diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml index 6f3140d..5d776c5 100644 --- a/src/main/resources/META-INF/plugin.xml +++ b/src/main/resources/META-INF/plugin.xml @@ -77,6 +77,9 @@ + diff --git a/src/test/java/de/platon42/intellij/plugins/m68k/inspections/M68kAddressRegisterWordAccessInspectionTest.kt b/src/test/java/de/platon42/intellij/plugins/m68k/inspections/M68kAddressRegisterWordAccessInspectionTest.kt new file mode 100644 index 0000000..b5faa0d --- /dev/null +++ b/src/test/java/de/platon42/intellij/plugins/m68k/inspections/M68kAddressRegisterWordAccessInspectionTest.kt @@ -0,0 +1,29 @@ +package de.platon42.intellij.plugins.m68k.inspections + +import com.intellij.testFramework.fixtures.CodeInsightTestFixture +import de.platon42.intellij.jupiter.MyFixture +import org.junit.jupiter.api.Test + +internal class M68kAddressRegisterWordAccessInspectionTest : AbstractInspectionTest() { + + @Test + internal fun shows_warning_on_movea_word_sized(@MyFixture myFixture: CodeInsightTestFixture) { + myFixture.enableInspections(M68kAddressRegisterWordAccessInspection::class.java) + myFixture.configureByText("wordaccess.asm", " move.w d0,a0") + assertHighlightings(myFixture, 1, "Suspicious movea word write to address register") + } + + @Test + internal fun shows_warning_on_cmpa_word_sized(@MyFixture myFixture: CodeInsightTestFixture) { + myFixture.enableInspections(M68kAddressRegisterWordAccessInspection::class.java) + myFixture.configureByText("wordaccess.asm", " cmp.w d0,a0") + assertHighlightings(myFixture, 1, "Suspicious cmpa comparing with word-sized address register") + } + + @Test + internal fun shows_warning_on_tst_with_word_sized_address_register(@MyFixture myFixture: CodeInsightTestFixture) { + myFixture.enableInspections(M68kAddressRegisterWordAccessInspection::class.java) + myFixture.configureByText("wordaccess.asm", " tst.w a0") + assertHighlightings(myFixture, 1, "Suspicious tst with word-sized address register") + } +}