[MIPS] Use qfThreadID if qC packet is not supported by target

[jaydeep]

Summary

The patch uses qfThreadID to get the thread IDs if qC packet is not supported by target.

[clayborg]

Any other OSs other than Linux have process IDs being the same as thread IDs? The code added should be properly made conditional based on linux and any other OSs where this holds true.

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

1462–1476

The threads ID is not the same as a process ID on any Apple based OS. We would need make this check conditional based on the OSs that have thread IDs being the same as process IDs. Not sure if that is only linux?

[jaydeep]

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

1462–1476

We are connected to a bare-iron target (IASim in this case). We need to check whether the target OS is Unknown.

[clayborg]

That is fine for unknown or for any known OSs where the pid == tid for the main thread.

[jaydeep]

Addressed review comments.

[jingham]

That seems good, except take out the use of auto in the first diff. We try not to use "auto" for trivially nameable types like lldb::pid_t. When you are declaring a variable where you would have to scratch your head more than twice to figure out how to write the type of the thing, then auto is a real help. But for simple types it doesn't offer any benefit and makes reading the code a little bit harder...

[jaydeep]

Address review comments

[jaydeep]

Could you please find some time to review this?
Thanks

[clayborg]

I don't see how passing the target architecture's OS helps here since you aren't checking it for linux or any of the other OS's where pid == tid. This arch is likely to be unset during many of these calls which makes the test just assume any time the OS is unknown it will just assume pid == tid.

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

1463–1465

How does checking the "ostype" with unknown help us to determine if this is an OS where pid == tid? What if the user makes their target with:

(lldb) target create --arch x86_64-pc-linux ...

Then this code doesn't trigger?

[jaydeep]

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

1463–1465

In this case the target we are connected to is a bare-iron which does not support qC packet. Here pid == tid and only way to get this is using qfThreadID. If arch is likely to be unset during many of these calls then we need to find alternate way to implement this.

[clayborg]

Lets just not send the OS then since it is very unreliable when the target if first attaching or launching and just always back up to using qfThreadID if first qProcessInfo and then qC are not implemented. We need some way to get the pid, so lets just do it.

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

1101

Revert this and don't send the OS, see main comment below.

1428

Revert this and don't send the OS, see main comment below.

1464

remove the ostype check, see main comment below.

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

112 ↗

(On Diff #31213)

Revert this and don't send the OS, see main comment below.

source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

761–762 ↗

(On Diff #31213)

Revert this and don't send the OS, see main comment below.

1016–1017 ↗

(On Diff #31213)

Revert this and don't send the OS, see main comment below.

2323–2324 ↗

(On Diff #31213)

Revert this and don't send the OS, see main comment below.

[jaydeep]

Addressed review comments.

[clayborg]

Looks good.

[jaydeep]

Closed by commit rL244866

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

while (state == eStateRunning)
                 {
                 case 'T':
                 case 'S':
                     {
                         if (process->GetStopID() == 0)
                         {
                             if (process->GetID() == LLDB_INVALID_PROCESS_ID)
                             {
                                 lldb::pid_t pid = GetCurrentProcessID ();
                                 if (pid != LLDB_INVALID_PROCESS_ID)
                                     process->SetID (pid);
                             }
                             process->BuildDynamicRegisterInfo (true);
                         }
 
                         // Privately notify any internal threads that we have stopped
                         // in case we wanted to interrupt our process, yet we might
)
     {
         if (log)
             log->Printf ("SendInterrupt () - not running");
     }
     return true;
 }
 
 lldb::pid_t
 GDBRemoteCommunicationClient::GetCurrentProcessID (bool allow_lazy)
 {
     if (allow_lazy && m_curr_pid_is_valid == eLazyBoolYes)
         return m_curr_pid;
     
     // First try to retrieve the pid via the qProcessInfo request.
     GetCurrentProcessInfo (allow_lazy);
     if (m_curr_pid_is_valid == eLazyBoolYes)
     {
else
                     if (m_curr_pid != LLDB_INVALID_PROCESS_ID)
                     {
                         m_curr_pid_is_valid = eLazyBoolYes;
                         return m_curr_pid;
                     }
                 }
             }
         }
+
+        // If we don't get a response for $qC, check if $qfThreadID gives us a result.
+        if (m_curr_pid == LLDB_INVALID_PROCESS_ID)
+        {
+            std::vector<lldb::tid_t> thread_ids;
+            bool sequence_mutex_unavailable;
+            size_t size;
+            size = GetCurrentThreadIDs (thread_ids, sequence_mutex_unavailable);
+            if (size && sequence_mutex_unavailable == false)
+            {
+                m_curr_pid = thread_ids.front();
+                m_curr_pid_is_valid = eLazyBoolYes;
+                return m_curr_pid;
+            }
+        }
     }
     
     return LLDB_INVALID_PROCESS_ID;
 }
 
 bool
 GDBRemoteCommunicationClient::GetLaunchSuccess (std::string &error_str)
 {