From 2abb5af8b0baf90afa8ab751d2a3e658cd17c4d8 Mon Sep 17 00:00:00 2001 From: chrisly42 Date: Thu, 5 Aug 2021 16:21:32 +0200 Subject: [PATCH] Macros with register name now abort Dead Write analysis. Fix for modifying statements extending the with of the written data. --- .../inspections/M68kDeadWriteInspection.kt | 8 +++- .../inspectionDescriptions/M68kDeadWrite.html | 5 ++- .../M68kDeadWriteInspectionTest.kt | 39 +++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) 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 index 47fc815..971cd51 100644 --- a/src/main/java/de/platon42/intellij/plugins/m68k/inspections/M68kDeadWriteInspection.kt +++ b/src/main/java/de/platon42/intellij/plugins/m68k/inspections/M68kDeadWriteInspection.kt @@ -48,7 +48,7 @@ class M68kDeadWriteInspection : AbstractBaseM68kLocalInspectionTool() { val hints = SmartList() for (regPair in regsWritten) { val register = regPair.first - val rwm = regPair.second + var rwm = regPair.second var currStatement = asmInstruction.parent as M68kStatement var ccModification = adrMode.affectedCc @@ -61,6 +61,11 @@ class M68kDeadWriteInspection : AbstractBaseM68kLocalInspectionTool() { val globalLabel = PsiTreeUtil.findChildOfType(currStatement, M68kGlobalLabel::class.java) if (globalLabel != null) break if (PsiTreeUtil.getChildOfType(currStatement, M68kPreprocessorDirective::class.java) != null) break + + // as we cannot evaluate macros right now, abort at macros containing the register name (only lower case for simplicity) + val macroCall = PsiTreeUtil.getChildOfType(currStatement, M68kMacroCall::class.java) + if (macroCall?.exprList?.any { it.textMatches(register.regname) } == true) break + val currAsmInstruction = PsiTreeUtil.getChildOfType(currStatement, M68kAsmInstruction::class.java) ?: continue val (isaData, currAdrMode) = findExactIsaDataAndAllowedAdrModeForInstruction(currAsmInstruction) ?: continue if (isaData.changesControlFlow) break @@ -74,6 +79,7 @@ class M68kDeadWriteInspection : AbstractBaseM68kLocalInspectionTool() { hasModification = true ccOverwritten = false ccModification = ccModification or currAdrMode.affectedCc + rwm = (totalRwms ushr RWM_MODIFY_SHIFT) and RWM_SET_L } if (totalRwms and RWM_SET_L >= rwm) { if (ccTested && hasModification) { diff --git a/src/main/resources/inspectionDescriptions/M68kDeadWrite.html b/src/main/resources/inspectionDescriptions/M68kDeadWrite.html index 5d36d20..efb8481 100644 --- a/src/main/resources/inspectionDescriptions/M68kDeadWrite.html +++ b/src/main/resources/inspectionDescriptions/M68kDeadWrite.html @@ -7,6 +7,9 @@ Issues a weak warning if the instruction affects only condition codes that are l Analysis is terminated at the next global label or instruction that reads the register or changes control flow (or preprocessor statements, like conditional IF statements). -

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

+

Note: As there is no evaluation of macros right now, the inspection might report some false positives. + + As an attempt to reduce these false positives, macros containing the register name will abort the analysis. +

\ No newline at end of file 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 index fb5815a..961a53b 100644 --- a/src/test/java/de/platon42/intellij/plugins/m68k/inspections/M68kDeadWriteInspectionTest.kt +++ b/src/test/java/de/platon42/intellij/plugins/m68k/inspections/M68kDeadWriteInspectionTest.kt @@ -169,4 +169,43 @@ internal class M68kDeadWriteInspectionTest : AbstractInspectionTest() { ) assertThat(myFixture.doHighlighting()).isEmpty() } + + @Test + internal fun modification_extending_width_does_not_cause_a_warning_on_smaller_write_later(@MyFixture myFixture: CodeInsightTestFixture) { + myFixture.enableInspections(M68kDeadWriteInspection::class.java) + myFixture.configureByText( + "deadwrite.asm", """ + move.b (a0)+,d0 + lsl #8,d0 + move.b (a0)+,d0 + """ + ) + assertThat(myFixture.doHighlighting()).isEmpty() + } + + @Test + internal fun macro_call_with_register_name_will_not_cause_a_warning(@MyFixture myFixture: CodeInsightTestFixture) { + myFixture.enableInspections(M68kDeadWriteInspection::class.java) + myFixture.configureByText( + "deadwrite.asm", """ + moveq.l #-1,d0 + COPRMOVE d0,bltafwm + move.l pd_CurrPlanesPtr(a4),d0 + """ + ) + assertThat(myFixture.doHighlighting()).isEmpty() + } + + @Test + internal fun macro_call_with_different_register_name_will_cause_a_warning(@MyFixture myFixture: CodeInsightTestFixture) { + myFixture.enableInspections(M68kDeadWriteInspection::class.java) + myFixture.configureByText( + "deadwrite.asm", """ + moveq.l #-1,d0 + COPRMOVE d1,bltafwm + move.l pd_CurrPlanesPtr(a4),d0 + """ + ) + assertHighlightings(myFixture, 1, "Register d0 is overwritten later without being used") + } } \ No newline at end of file