From 95aae275018fc40cf72e9263be302f88b08775f3 Mon Sep 17 00:00:00 2001 From: emteere <47253321+emteere@users.noreply.github.com> Date: Thu, 11 Jul 2024 15:34:53 -0400 Subject: [PATCH] GP-4596 refactored attach table to be more explicit, code review changes --- .../Processors/PIC/data/languages/pic16.sinc | 1 - .../data/languages/pic16_instructions.sinc | 55 +++++++++++-------- .../plugin/core/analysis/Pic16Analyzer.java | 4 +- 3 files changed, 32 insertions(+), 28 deletions(-) diff --git a/Ghidra/Processors/PIC/data/languages/pic16.sinc b/Ghidra/Processors/PIC/data/languages/pic16.sinc index d42492ab9f..68207af119 100644 --- a/Ghidra/Processors/PIC/data/languages/pic16.sinc +++ b/Ghidra/Processors/PIC/data/languages/pic16.sinc @@ -77,7 +77,6 @@ define register offset=0x0007 size=1 [ IRP RP ]; # @if PROCESSOR == "PIC_16" define DATA offset=0x0000 size=1 [ -#PIC16 FRM INDF TMR0 PCL STATUS FSR PORTA PORTB PORTC PORTD PORTE PCLATH INTCON PIR1 PIR2 TMR1L TMR1H INDF _ PCL STATUS FSR _ _ _ _ _ PCLATH INTCON _ _ _ _ ]; @elif PROCESSOR == "PIC_16F" diff --git a/Ghidra/Processors/PIC/data/languages/pic16_instructions.sinc b/Ghidra/Processors/PIC/data/languages/pic16_instructions.sinc index 8e7581edd5..4d31346ed3 100644 --- a/Ghidra/Processors/PIC/data/languages/pic16_instructions.sinc +++ b/Ghidra/Processors/PIC/data/languages/pic16_instructions.sinc @@ -25,7 +25,6 @@ define token instr16(16) uf7 = (4,6) fsr = (2,2) fsrk = (6,6) - fregCore = (0,3) k5 = (0,4) k6 = (0,5) k7 = (0,6) @@ -44,15 +43,7 @@ define context contextreg ; -@if PROCESSOR == "PIC_16" -attach variables [ fregCore ] [ - INDF _ PCL STATUS FSR _ _ _ _ _ PCLATH INTCON _ _ _ _ -]; - -@elif PROCESSOR == "PIC_16F" -attach variables [ fregCore ] [ - INDF0 INDF1 PCL STATUS FSR0L FSR0H FSR1L FSR1H BSR W PCLATH INTCON _ _ _ _ -]; +@if PROCESSOR == "PIC_16F" attach names [IntConBits] [ IOCIF INTF TMR0IF IOCIE INTE TMR0IE PEIE GIE ]; @@ -178,6 +169,18 @@ srcREG: fv is uf7=0x7 & lf7 [fv = 0x70 + lf7; ] { export *[DATA]:1 addr; } +# The registers listed here are explicitly defined as registers in sleigh. +# There are other registers but they are named in the .pspec file. +# The reason this is done is to have cross references created to certain registers, and to have +# only the registers that must be accessed directly in sleigh (e.g. PCL, FSR) defined in sleigh. +# Register explicitly defined in sleigh will not have xref's created to them. +# Registers named only in the .pspec file will have xref's to them in most cases. +# +# Also, these registers ignore RP, or BSR which allow more registers to be in a different register bank. +# +# PIC16 : INDF _ PCL STATUS FSR _ _ _ _ _ PCLATH INTCON _ _ _ _ +# PIC16F: INDF0 INDF1 PCL STATUS FSR0L FSR0H FSR1L FSR1H BSR W PCLATH INTCON _ _ _ _ + # File register index (f7=0): INDF use implies indirect data access using FSR value and IRP bit in STATUS reg @if PROCESSOR == "PIC_16" srcREG: INDF is f7=0 & INDF { @@ -189,20 +192,22 @@ srcREG: lf7 is f7=1 & lf7 { addr:2 = (zext(rpval) << 7) + 1; export *[DATA]:1 addr; } + @elif PROCESSOR == "PIC_16F" -srcREG: fregCore is f7=0 & fregCore { + +srcREG: INDF0 is f7=0 & INDF0 { addr:2 = FSR0; export *[DATA]:1 addr; } -srcREG: fregCore is f7=1 & fregCore { +srcREG: INDF1 is f7=1 & INDF1 { addr:2 = FSR1; export *[DATA]:1 addr; } @endif # Special File Registers always mapped to Bank-0 -srcREG: fregCore is f7=0x02 & fregCore { +srcREG: PCL is f7=0x02 & PCL { # PCL and PCLATH must be latched addr:2 = inst_start >> 1; # Compensate for CODE wordsize PCL = addr:1; @@ -210,17 +215,19 @@ srcREG: fregCore is f7=0x02 & fregCore { export PCL; } -srcREG: fregCore is f7=0x03 & fregCore { export fregCore; } -srcREG: fregCore is f7=0x04 & fregCore { export fregCore; } -@if PROCESSOR == "PIC_16F" -srcREG: fregCore is f7=0x05 & fregCore { export fregCore; } -srcREG: fregCore is f7=0x06 & fregCore { export fregCore; } -srcREG: fregCore is f7=0x07 & fregCore { export fregCore; } -srcREG: fregCore is f7=0x08 & fregCore { export fregCore; } -srcREG: fregCore is f7=0x09 & fregCore { export fregCore; } +srcREG: STATUS is f7=0x03 & STATUS { export STATUS; } +@if PROCESSOR == "PIC_16" +srcREG: FSR is f7=0x04 & FSR { export FSR; } +@elif PROCESSOR == "PIC_16F" +srcREG: FSR0L is f7=0x04 & FSR0L { export FSR0L; } +srcREG: FSR0H is f7=0x05 & FSR0H { export FSR0H; } +srcREG: FSR1L is f7=0x06 & FSR1L { export FSR1L; } +srcREG: FSR1H is f7=0x07 & FSR1H { export FSR1H; } +srcREG: BSR is f7=0x08 & BSR { export BSR; } +srcREG: W is f7=0x09 & W { export W; } @endif -srcREG: fregCore is f7=0x0a & fregCore { export fregCore; } -srcREG: fregCore is f7=0x0b & fregCore { export fregCore; } +srcREG: PCLATH is f7=0x0a & PCLATH { export PCLATH; } +srcREG: INTCON is f7=0x0b & INTCON { export INTCON; } # Destination register (either srcREG or W) @@ -230,7 +237,7 @@ destREG: "0" is d=0 { export W; } destREG: "1" is d=1 & f7 & srcREG { export srcREG; } # Destination register: Special File Registers always mapped to Bank-0 -destREG: "1" is d=1 & f7=0x02 & fregCore { export fregCore; } # PCL (special behavior reqd) +destREG: "1" is d=1 & f7=0x02 { export PCL; } # PCL (special behavior reqd) # Destination operand representation (w: W register is destination; f: specified srcREG is destination) D: "w" is d=0 { } diff --git a/Ghidra/Processors/PIC/src/main/java/ghidra/app/plugin/core/analysis/Pic16Analyzer.java b/Ghidra/Processors/PIC/src/main/java/ghidra/app/plugin/core/analysis/Pic16Analyzer.java index 44a3e519b2..2a42b481b3 100644 --- a/Ghidra/Processors/PIC/src/main/java/ghidra/app/plugin/core/analysis/Pic16Analyzer.java +++ b/Ghidra/Processors/PIC/src/main/java/ghidra/app/plugin/core/analysis/Pic16Analyzer.java @@ -53,9 +53,7 @@ public class Pic16Analyzer extends ConstantPropagationAnalyzer { @Override public boolean canAnalyze(Program p) { - boolean cananalyze = super.canAnalyze(p); - - if (!cananalyze) { + if (!super.canAnalyze(p)) { return false; } Language lang = p.getLanguage();