[MIPS] Debug bare-iron targets lacking support for qC /qfThreadInfo

[jaydeep]

Summary

Refer to http://lists.llvm.org/pipermail/lldb-dev/2015-August/008024.html for discussion on this topic.

Bare-iron target like YAMON gdb-stub does not support qProcessInfo, qC, qfThreadInfo, Hg and Hc packets. Reply from ‘?’ packet is as simple as ‘S05’. There is no packet which gives us process or threads information. In such cases, assume pid=tid=1.

[clayborg]

Looks good.

[jaydeep]

Closed by commit http://reviews.llvm.org/rL247773

[labath]

Hi, this commit has caused failure on all remote tests using lldb-server. I have reverted it while I investigate what is happening.

[labath]

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

3579

The problem behind the failures seems to be this: when we connect to the remote lldb-server, it should not have any processes or threads running (we will start those with the A packet later). We verify this in ProcessGDBRemote.cpp:837. This translates to a qfThreadInfo packet, to which our stub responds with OK.

This sounds like a bug in the stub, as the protocol does not mention that as a possibility. I will prepare a patch for that shortly. Could you please wait with this patch a bit until the bugfix lands?

Also, this got me thinking.. What is the response of your stub to the qfThreadInfo packet. If it is something like '?', then we could check for response.IsUnsupportedResponse() and we will avoid an overly broad check for your corner case. What do you think?

[labath]

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

3579

The problem behind the failures seems to be this: when we connect to the remote lldb-server, it should not have any processes or threads running (we will start those with the A packet later). We verify this in ProcessGDBRemote.cpp:837. This translates to a qfThreadInfo packet, to which our stub responds with OK.

Hmm.. the debugserver seems to have this behavior as well. Greg, what do you think should be the right response to this packet when we have no process running? My feeling is that we should just send a "l" packet as if we have just reached the end of the list. What do you think?

[jaydeep]

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

3579

Response from the target is "$#00". I think we should check for IsUnsupportedResponse().

[labath]

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

3579

If IsUnsupportedResponse will do the trick for you then I have no objections, and this can go in straight away.

[clayborg]

If "qfThreadInfo" responds with OK, it seems to imply "this is a supported packet, but I have no threads to report". So it seems like we should assume that there are no threads in this case. If we got $#00 back, then it would mean that qfThreadInfo ins't supported. So if we get "OK" back from "qfThreadInfo", we should stop searching and assume there are no threads (don't try to send a stop reply, etc).

[jaydeep]

Changed "!response.IsNormalResponse()" to "response.IsUnsupportedResponse()" by commit http://reviews.llvm.org/rL247968

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

GDBRemoteCommunicationClient::SetCurrentThread (uint64_t tid)
     StringExtractorGDBRemote response;
     if (SendPacketAndWaitForResponse(packet, packet_len, response, false) == PacketResult::Success)
     {
         if (response.IsOKResponse())
         {
             m_curr_tid = tid;
             return true;
         }
+
+        /*
+         * Connected bare-iron target (like YAMON gdb-stub) may not have support for Hg packet.
+         * The reply from '?' packet could be as simple as 'S05'. There is no packet which can
+         * give us pid and/or tid. Assume pid=tid=1 in such cases.
+        */
+        if (!response.IsNormalResponse() && IsConnected())
+        {
+            m_curr_tid = 1;
+            return true;
+        }
     }
     return false;
 }
 
 bool
 GDBRemoteCommunicationClient::SetCurrentThreadForRun (uint64_t tid)
 {
     if (m_curr_tid_run == tid)
GDBRemoteCommunicationClient::SetCurrentThreadForRun (uint64_t tid)
     StringExtractorGDBRemote response;
     if (SendPacketAndWaitForResponse(packet, packet_len, response, false) == PacketResult::Success)
     {
         if (response.IsOKResponse())
         {
             m_curr_tid_run = tid;
             return true;
         }
+
+        /*
+         * Connected bare-iron target (like YAMON gdb-stub) may not have support for Hc packet.
+         * The reply from '?' packet could be as simple as 'S05'. There is no packet which can
+         * give us pid and/or tid. Assume pid=tid=1 in such cases.
+        */
+        if (!response.IsNormalResponse() && IsConnected())
+        {
+            m_curr_tid_run = 1;
+            return true;
+        }
     }
     return false;
 }
 
 bool
 GDBRemoteCommunicationClient::GetStopReply (StringExtractorGDBRemote &response)
 {
     if (SendPacketAndWaitForResponse("?", 1, response, false) == PacketResult::Success)
if (GetSequenceMutex (locker, "ProcessGDBRemote::UpdateThreadList() failed due to not getting the sequence mutex"))
                     if (tid != LLDB_INVALID_THREAD_ID)
                     {
                         thread_ids.push_back (tid);
                     }
                     ch = response.GetChar();    // Skip the command separator
                 } while (ch == ',');            // Make sure we got a comma separator
             }
         }
+
+        /*
+         * Connected bare-iron target (like YAMON gdb-stub) may not have support for
+         * qProcessInfo, qC and qfThreadInfo packets. The reply from '?' packet could
+         * be as simple as 'S05'. There is no packet which can give us pid and/or tid.
+         * Assume pid=tid=1 in such cases.
+        */
+        if (!response.IsNormalResponse() && thread_ids.size() == 0 && IsConnected())
+        {
+            thread_ids.push_back (1);
+        }
     }
     else
     {
 #if defined (LLDB_CONFIGURATION_DEBUG)
         // assert(!"ProcessGDBRemote::UpdateThreadList() failed due to not getting the sequence mutex");
 #else
         Log *log (ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet (GDBR_LOG_PROCESS | GDBR_LOG_PACKETS));
         if (log)