[Lldb-commits] [PATCH] D30457: [LLDB][MIPS] Core Dump Support

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Have you tried running the `test/testcases/functionalities/postmortem/elf-core/make-core.sh` script? Does it generate a reasonably-sized core file (potentially you may need to increase the stack limit slightly). It would be great to have a mips core file test for this.



================
Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.h:10
 
-#if defined(__mips__)
 
----------------
These need to stay, otherwise you'll get multiply-defined symbols on non-mips platforms.
These register contexts were meant to be used in lldb-server only and I am pretty sure you don't need them for mips core file support.


================
Comment at: source/Plugins/Process/Utility/RegisterInfoInterface.h:32
 
+  virtual const lldb_private::RegisterSet *
+  GetRegisterSet(size_t set) const {return nullptr;}
----------------
While I don't see anything obviously wrong about adding this interface, I am wondering why the other subclasses have not needed this.

I'd defer to @clayborg judgement on the appropriateness of the interface. What I don't like however, is that the default implementation will blatantly lie about the number of register sets for the non-mips case.

What I can suggest is to avoid putting these functions in the generic class -- you seem to be calling them from mips code only, so I don't see any immediate need to have them here. (e.g. have GetRegisterInfoInterface() cast to the appropriate type).


https://reviews.llvm.org/D30457