[Lldb-commits] [PATCH] D146965: [lldb] Add support for MSP430 in LLDB.

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> Looked into core dumping in LLDB a bit, I think it pretty much requires that there is an OS, and there needs to be an explicit support for the architecture in question. Plus, mspdebug doesn't actually support core files, there is no use for it now even if LLDB can generate it.

That makes sense. I was more thinking about carving useful memory out of a raw dump of the RAM in any case, which is exactly what you've done in the test you added.

If you wanted to have a corefile like experience I guess you could do that RAM dump, load it back into a simulator and connect to that. But regardless, out of scope here.



================
Comment at: lldb/test/API/functionalities/gdb_remote_client/TestMSP430MSPDebug.py:6
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+class MyResponder(MockGDBServerResponder):
----------------
Please add a comment along the lines of: This test ensures that lldb correctly handles packets sent by <the msp430 debug stub>.

Ideally with a link to that stub or its source or the vendor's website, whatever's available.

If you can say how you captured the packets that would be good too. I assume just connecting and turning on packet logging but worth mentioning in case you had to do something unique.


================
Comment at: lldb/test/API/functionalities/gdb_remote_client/TestMSP430MSPDebug.py:23
+            case 3:
+                return "T0500:16050000;01:baff0000;02:05000000;03:00000000;04:00000000;05:00000000;06:00000000;07:00000000;08:00000000;09:00000000;0a:00000000;0b:00000000;0c:00000000;0d:00000000;0e:00000000;0f:00000000;"
+
----------------
Can you generate these with a helper function? Seems like the first bit changes and the rest are blanks you could add each time.

Also could you yield each time? Been a long time since I did serious Python but I think that could remove the counter.


================
Comment at: lldb/test/API/functionalities/gdb_remote_client/TestMSP430MSPDebug.py:29
+    def readMemory(self, addr, length):
+        match addr:
+            case 0x0400:
----------------
I don't know that we have recent enough Pythons to use match, when was it added?

I don't have the minimum versions for lldb to hand, I would guess it is 3.7.


================
Comment at: lldb/test/API/functionalities/gdb_remote_client/TestMSP430MSPDebug.py:33
+            case 0xfe00:
+                return "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff280500000a05ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0005"
+
----------------
For these can you write a helper to generate the bulk of it? And ideally describe which bit is the important part. I guess some of this is the stack.


================
Comment at: lldb/test/API/functionalities/gdb_remote_client/TestMSP430MSPDebug.py:96
+        self.assertEqual(crt0_addr, 0x50a)
+
+
----------------
Could you also check that the disassembler works?

I mistakenly thought that msp430 didn't have an llvm backend but it seems that it does so it should "just work".


================
Comment at: lldb/test/API/functionalities/gdb_remote_client/msp430.yaml:1
+--- !ELF
+FileHeader:
----------------
I forget if/how you add comments to a YAML file but however it's done, please add the C source that was used to generate the program file as a comment at the top.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146965/new/

https://reviews.llvm.org/D146965