From c80984021b1248d7cb233f0f6568437bca7155b0 Mon Sep 17 00:00:00 2001
From: Nav <nav@navnav.me>
Date: Thu, 14 Sep 2023 21:10:57 +0100
Subject: [PATCH] Added 0xFFFF opcode as some AVRs treat it as an SBRS
 instruction. Also some tidying

---
 .../AVR/AVR8/OpcodeDecoder/Decoder.cpp        |  6 +++--
 .../AVR/AVR8/OpcodeDecoder/Decoder.hpp        |  2 +-
 .../AVR/AVR8/OpcodeDecoder/Instruction.hpp    |  7 +++++
 .../AVR/AVR8/OpcodeDecoder/Opcodes.hpp        | 26 +++++++++++++++++++
 4 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/src/Targets/Microchip/AVR/AVR8/OpcodeDecoder/Decoder.cpp b/src/Targets/Microchip/AVR/AVR8/OpcodeDecoder/Decoder.cpp
index 98a4eaa1..c861e457 100644
--- a/src/Targets/Microchip/AVR/AVR8/OpcodeDecoder/Decoder.cpp
+++ b/src/Targets/Microchip/AVR/AVR8/OpcodeDecoder/Decoder.cpp
@@ -23,6 +23,7 @@ namespace Targets::Microchip::Avr::Avr8Bit::OpcodeDecoder
 
         while(dataIt != dataEndIt) {
             auto opcodeMatched = false;
+            const auto wordAvailable = std::distance(dataIt, dataEndIt) >= 2;
 
             for (const auto& decoder : decoders) {
                 auto instruction = decoder(dataIt, dataEndIt);
@@ -42,7 +43,7 @@ namespace Targets::Microchip::Avr::Avr8Bit::OpcodeDecoder
                 if (throwOnFailure) {
                     throw Exceptions::DecodeFailure(
                         instructionByteAddress,
-                        std::distance(dataIt, dataEndIt) >= 2
+                        wordAvailable
                             ? static_cast<std::uint32_t>(*(dataIt + 1) << 8) | *dataIt
                             : *dataIt
                     );
@@ -50,7 +51,7 @@ namespace Targets::Microchip::Avr::Avr8Bit::OpcodeDecoder
 
                 output.insert(std::pair(instructionByteAddress, std::nullopt));
 
-                if (std::distance(dataIt, dataEndIt) < 2) {
+                if (!wordAvailable) {
                     break;
                 }
 
@@ -69,6 +70,7 @@ namespace Targets::Microchip::Avr::Avr8Bit::OpcodeDecoder
          * I've used the same order that is used in the AVR implementation of GDB.
          */
         return Decoder::OpcodeDecoders({
+            std::bind(&Opcodes::UndefinedOrErased::decode, std::placeholders::_1, std::placeholders::_2),
             std::bind(&Opcodes::Clc::decode, std::placeholders::_1, std::placeholders::_2),
             std::bind(&Opcodes::Clh::decode, std::placeholders::_1, std::placeholders::_2),
             std::bind(&Opcodes::Cli::decode, std::placeholders::_1, std::placeholders::_2),
diff --git a/src/Targets/Microchip/AVR/AVR8/OpcodeDecoder/Decoder.hpp b/src/Targets/Microchip/AVR/AVR8/OpcodeDecoder/Decoder.hpp
index bd2c9a65..2f3d4fe0 100644
--- a/src/Targets/Microchip/AVR/AVR8/OpcodeDecoder/Decoder.hpp
+++ b/src/Targets/Microchip/AVR/AVR8/OpcodeDecoder/Decoder.hpp
@@ -49,7 +49,7 @@ namespace Targets::Microchip::Avr::Avr8Bit::OpcodeDecoder
                 const Targets::TargetMemoryBuffer::const_iterator&
            )
         >;
-        using OpcodeDecoders = std::array<Decoder::OpcodeDecoderFunction, 144>;
+        using OpcodeDecoders = std::array<Decoder::OpcodeDecoderFunction, 145>;
 
         static OpcodeDecoders opcodeDecoders();
     };
diff --git a/src/Targets/Microchip/AVR/AVR8/OpcodeDecoder/Instruction.hpp b/src/Targets/Microchip/AVR/AVR8/OpcodeDecoder/Instruction.hpp
index 7038d014..9f48d4da 100644
--- a/src/Targets/Microchip/AVR/AVR8/OpcodeDecoder/Instruction.hpp
+++ b/src/Targets/Microchip/AVR/AVR8/OpcodeDecoder/Instruction.hpp
@@ -19,6 +19,13 @@ namespace Targets::Microchip::Avr::Avr8Bit::OpcodeDecoder
             MUL, MULS, MULSU, NEG, NOP, OR, ORI, OUT, POP, PUSH, RCALL, RET, RETI, RJMP, ROL, ROR, SBC, SBCI, SBI,
             SBIC, SBIS, SBIW, SBR, SBRC, SBRS, SEC, SEH, SEI, SEN, SER, SES, SET, SEV, SEZ, SLEEP, SPM, ST, STD, STS,
             SUB, SUBI, SWAP, TST, WDR, XCH,
+
+            /*
+             * For undefined opcodes that we've had to define, because the hardware doesn't treat them as undefined.
+             *
+             * For example, see the 0xFFFF opcode.
+             */
+            UNDEFINED,
         };
 
         const std::string& name;
diff --git a/src/Targets/Microchip/AVR/AVR8/OpcodeDecoder/Opcodes.hpp b/src/Targets/Microchip/AVR/AVR8/OpcodeDecoder/Opcodes.hpp
index 38e1ea17..6d79c427 100644
--- a/src/Targets/Microchip/AVR/AVR8/OpcodeDecoder/Opcodes.hpp
+++ b/src/Targets/Microchip/AVR/AVR8/OpcodeDecoder/Opcodes.hpp
@@ -1689,6 +1689,32 @@ namespace Targets::Microchip::Avr::Avr8Bit::OpcodeDecoder::Opcodes
         RegisterParameter({9, 10})
     >;
 
+    /*
+     * So apparently, some AVRs treat the undefined 0xFFFF opcode as 0xFFF7 (SBRS R31, 7).
+     *
+     * See https://www.avrfreaks.net/s/topic/a5C3l000000UL4NEAW/t094785
+     *
+     * For this reason, we have to define it here.
+     */
+    using UndefinedOrErased = Opcode<
+        "????",
+        0b1111111111111111,
+        1,
+        Instruction::Mnemonic::UNDEFINED,
+        true,
+        std::nullopt,
+        std::nullopt,
+        std::nullopt,
+        std::nullopt,
+        std::nullopt,
+        std::nullopt,
+        std::nullopt,
+        std::nullopt,
+        std::nullopt,
+        std::nullopt,
+        true
+    >;
+
     using Wdr = Opcode<
         "WDR",
         0b1001010110101000,