[LLDB] Add support for AVR breakpoints

[aykevl]

Summary

I believe the actual opcode does not matter because the AVR architecture is a Harvard architecture that does not support writing to program memory. Therefore, debuggers and emulators provide hardware breakpoints. But for some reason, this opcode must be defined or else LLDB will crash with an assertion error.

---

I would like to add a test case for this, but I'm not quite sure how to do that. It appears that to trigger the crash (fixed by this patch), there needs to be a gdb-remote.

[labath]

We have infrastructure to "mock" a gdb-remote server (see packages/Python/lldbsuite/test/functionalities/gdb_remote_client/) to test lldb's interaction with it.

For a simple patch like this, I don't think a test is really required (especially as they are not the easiest tests to write), but OTOH, you are going to need this sooner or later, so now may be a good time to try it out.

[jingham]

Might also be good to fix the crash. If you don't support software breakpoints, you shouldn't get asked what your breakpoint opcode is.

[labath]

The whole flow here is pretty nonsensical -- the only reason we ask for the opcode is to get its size so we can put it into the Z0 packet -- however this is only needed for targets like arm, which have mutiple ISAs/opcodes, and we've already gotten complaints about sending that unconditionally.

However, if we look at this locally, if the AVR architecture has a trap opcode (maybe to implement __builtin_debugbreak() -- I am assuming that's what 0x98 0x95 is), then I don't see a problem with this function returning it.

[dylanmckay]

However, if we look at this locally, if the AVR architecture has a trap opcode (maybe to implement __builtin_debugbreak() -- I am assuming that's what 0x98 0x95 is), then I don't see a problem with this function returning it.

Chiming in from an AVR perspective, the AVR does have a trap opcode (BREAK) and this is encoded by 0x9895

The BREAK instruction is used by the On-chip Debug system, and is normally not used in the application
software. When the BREAK instruction is executed, the AVR CPU is set in the Stopped Mode. This gives the
On-chip Debugger access to internal resources.

If any Lock bits are set, or either the JTAGEN or OCDEN Fuses are unprogrammed, the CPU will treat the
BREAK instruction as a NOP and will not enter the Stopped mode.

This instruction is not available in all devices. Refer to the device specific instruction set summary.

[aykevl]

In D74255#1871958, @labath wrote:

However, if we look at this locally, if the AVR architecture has a trap opcode (maybe to implement __builtin_debugbreak() -- I am assuming that's what 0x98 0x95 is), then I don't see a problem with this function returning it.

As Dylan already mentioned, the opcode is indeed for the BREAK instruction. Here it is:
https://github.com/llvm/llvm-project/blob/release/9.x/llvm/test/MC/AVR/inst-break.s
https://www.microchip.com/webdoc/avrassembler/avrassembler.wb_BREAK.html

The documentation even states:

The BREAK instruction is used by the On-Chip Debug system, and is normally not used in the application software.

I would argue that even though the direct need for this patch may be to work around a limitation elsewhere, it is a useful change on its own.

[labath]

As Dylan already mentioned, the opcode is indeed for the BREAK instruction.

Ok, let's go with that.

lldb/source/Target/Platform.cpp

       trap_opcode = g_thumb_breakpoint_opcode;
       trap_opcode_size = sizeof(g_thumb_breakpoint_opcode);
     } else {
       trap_opcode = g_arm_breakpoint_opcode;
       trap_opcode_size = sizeof(g_arm_breakpoint_opcode);
     }
   } break;
 
+  case llvm::Triple::avr: {
+    static const uint8_t g_hex_opcode[] = {0x98, 0x95};
+    trap_opcode = g_hex_opcode;
+    trap_opcode_size = sizeof(g_hex_opcode);
+  } break;
+
   case llvm::Triple::mips:
   case llvm::Triple::mips64: {
     static const uint8_t g_hex_opcode[] = {0x00, 0x00, 0x00, 0x0d};
     trap_opcode = g_hex_opcode;
     trap_opcode_size = sizeof(g_hex_opcode);
   } break;
 
   case llvm::Triple::mipsel: