[Lldb-commits] [PATCH] [LLDB][MIPS] Getting correct signals for MIPS Host

[Greg Clayton]

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


> On Jun 2, 2015, at 11:24 AM, Oleksiy Vyalov <ovyalov at google.com> wrote:
> 
> 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).
> 
> 
> REPOSITORY
>  rL LLVM
> 
> ================
> Comment at: source/Host/linux/Host.cpp:396
> @@ -395,3 +395,3 @@
> {
> -    static const lldb_private::UnixSignalsSP s_unix_signals_sp (new process_linux::LinuxSignals ());
> -    return s_unix_signals_sp;
> +    ArchSpec target_arch=HostInfoBase::GetArchitecture();
> +    if(target_arch.GetTriple ().getArch () == llvm::Triple::mips64 ||
> ----------------
> Please add spaces around =
> 
> ================
> Comment at: source/Host/linux/Host.cpp:399
> @@ +398,3 @@
> +       target_arch.GetTriple ().getArch () == llvm::Triple::mips64el ||
> +       target_arch.GetTriple ().getArch ()== llvm::Triple::mips ||
> +       target_arch.GetTriple ().getArch ()== llvm::Triple::mipsel) {
> ----------------
> ditto.
> 
> http://reviews.llvm.org/D10180
> 
> EMAIL PREFERENCES
>  http://reviews.llvm.org/settings/panel/emailpreferences/
> 
>