[LLDB][MIPS] Getting correct signals for MIPS Host

[clayborg]

looks good.

[ovyalov]

I'm wondering whether it's possible to consolidate linux signal logic in a single place - now we have 3 sites where we instantiate LinuxSignals and need to check for mips as well:

1. source/Host/linux/Host.cpp
2. https://github.com/llvm-mirror/lldb/blob/7ca60062e09b2ddbc68845b4b6c000fedf308351/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp#L834 - btw, it checks only for mips64 or mips64el
3. https://github.com/llvm-mirror/lldb/blob/f7adf4b988da7bd5e13c99af60b6f030eb1beefe/source/Plugins/Process/elf-core/ProcessElfCore.cpp#L248 - we might want to put mips signals here as well.

As an idea - we can have factory method LinuxSignals::Create(const ArchSpec&) which will encompass decision making about creating LinuxSignals or MipsLinuxSignals. So, then LinuxSignals::Create can be reused on all call sites (to be safe - with protected LinuxSignals constructor to prevent explicit creation).

source/Host/linux/Host.cpp

396 ↗

(On Diff #26959)

Please add spaces around =

399 ↗

(On Diff #26959)

ditto.

[clayborg]

My vote would be to build a Host::GetSignals(), have lldb-server use it, add the signal information to the qHostInfo packet. Then all signal info becomes the responsibility of the lldb-server we connect to which can use the current lldb_private::Host layer to determine the info correctly. The implementation becomes a #define fest for all the different hosts, but at least it is clean.

Greg

[labath]

I agree with Greg. Situation is even more complicated when you take the realtime signals into account. Currently, the numbers for these differ between linux and android *and* they also differ between x86, arm, arm64 (haven't checked mips). Getting these right on the client will be very tricky.

[mohit.bhakkad]

For now closing it with a TODO, as there are dependencies on MIPS signals.

lldb/trunk/source/Host/linux/Host.cpp

 #endif
 #include "lldb/Core/DataBufferHeap.h"
 #include "lldb/Core/DataExtractor.h"
 
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "Plugins/Process/Linux/ProcFileReader.h"
 #include "Plugins/Process/Utility/LinuxSignals.h"
-
+#include "Plugins/Process/Utility/MipsLinuxSignals.h"
 using namespace lldb;
 using namespace lldb_private;
 
 typedef enum ProcessStateFlags
 {
     eProcessStateRunning           = (1u << 0), // Running
     eProcessStateSleeping          = (1u << 1), // Sleeping in an interruptible wait
     eProcessStateWaiting           = (1u << 2), // Waiting in an uninterruptible disk sleep
Host::GetEnvironment (StringList &env)
     char **host_env = environ;
     char *env_entry;
     size_t i;
     for (i=0; (env_entry = host_env[i]) != NULL; ++i)
         env.AppendString(env_entry);
     return i;
 }
 
+// TODO: Generalize this with a function Host::GetSignals() as discussed at http://reviews.llvm.org/D10180 
 const lldb_private::UnixSignalsSP&
 Host::GetUnixSignals ()
 {
+    ArchSpec target_arch = HostInfoBase::GetArchitecture();
+    if(target_arch.GetTriple ().getArch () == llvm::Triple::mips64 ||
+       target_arch.GetTriple ().getArch () == llvm::Triple::mips64el ||
+       target_arch.GetTriple ().getArch () == llvm::Triple::mips ||
+       target_arch.GetTriple ().getArch () == llvm::Triple::mipsel) {
+        static const lldb_private::UnixSignalsSP s_unix_signals_sp (new process_linux::MipsLinuxSignals ());
+        return s_unix_signals_sp;
+    }
+    else
+    {
         static const lldb_private::UnixSignalsSP s_unix_signals_sp (new process_linux::LinuxSignals ());
         return s_unix_signals_sp;
     }
 
+}
+
 Error
 Host::ShellExpandArguments (ProcessLaunchInfo &launch_info)
 {
     return Error("unimplemented");
 }