[Lldb-commits] [lldb] r202028 - Patch for fixing the handling of hardware breakpoints.
Deepak Panickal
deepak at codeplay.com
Mon Feb 24 03:50:47 PST 2014
Author: panickal
Date: Mon Feb 24 05:50:46 2014
New Revision: 202028
URL: http://llvm.org/viewvc/llvm-project?rev=202028&view=rev
Log:
Patch for fixing the handling of hardware breakpoints.
Differential Revision: http://llvm-reviews.chandlerc.com/D2826
Modified:
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp?rev=202028&r1=202027&r2=202028&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp Mon Feb 24 05:50:46 2014
@@ -2675,15 +2675,10 @@ GDBRemoteCommunicationClient::GetThreadS
uint8_t
GDBRemoteCommunicationClient::SendGDBStoppointTypePacket (GDBStoppointType type, bool insert, addr_t addr, uint32_t length)
{
- switch (type)
- {
- case eBreakpointSoftware: if (!m_supports_z0) return UINT8_MAX; break;
- case eBreakpointHardware: if (!m_supports_z1) return UINT8_MAX; break;
- case eWatchpointWrite: if (!m_supports_z2) return UINT8_MAX; break;
- case eWatchpointRead: if (!m_supports_z3) return UINT8_MAX; break;
- case eWatchpointReadWrite: if (!m_supports_z4) return UINT8_MAX; break;
- }
-
+ // Check if the stub is known not to support this breakpoint type
+ if (!SupportsGDBStoppointPacket(type))
+ return UINT8_MAX;
+ // Construct the breakpoint packet
char packet[64];
const int packet_len = ::snprintf (packet,
sizeof(packet),
@@ -2692,28 +2687,35 @@ GDBRemoteCommunicationClient::SendGDBSto
type,
addr,
length);
-
+ // Check we havent overwritten the end of the packet buffer
assert (packet_len + 1 < (int)sizeof(packet));
StringExtractorGDBRemote response;
+ // Try to send the breakpoint packet, and check that it was correctly sent
if (SendPacketAndWaitForResponse(packet, packet_len, response, true) == PacketResult::Success)
{
+ // Receive and OK packet when the breakpoint successfully placed
if (response.IsOKResponse())
return 0;
- else if (response.IsErrorResponse())
+
+ // Error while setting breakpoint, send back specific error
+ if (response.IsErrorResponse())
return response.GetError();
- }
- else
- {
- switch (type)
+
+ // Empty packet informs us that breakpoint is not supported
+ if (response.IsUnsupportedResponse())
{
+ // Disable this breakpoint type since it is unsupported
+ switch (type)
+ {
case eBreakpointSoftware: m_supports_z0 = false; break;
case eBreakpointHardware: m_supports_z1 = false; break;
case eWatchpointWrite: m_supports_z2 = false; break;
case eWatchpointRead: m_supports_z3 = false; break;
case eWatchpointReadWrite: m_supports_z4 = false; break;
+ }
}
}
-
+ // Signal generic faliure
return UINT8_MAX;
}
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=202028&r1=202027&r2=202028&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Mon Feb 24 05:50:46 2014
@@ -2310,70 +2310,106 @@ Error
ProcessGDBRemote::EnableBreakpointSite (BreakpointSite *bp_site)
{
Error error;
- assert (bp_site != NULL);
+ assert(bp_site != NULL);
- Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_BREAKPOINTS));
+ // Get logging info
+ Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_BREAKPOINTS));
user_id_t site_id = bp_site->GetID();
+
+ // Get the breakpoint address
const addr_t addr = bp_site->GetLoadAddress();
+
+ // Log that a breakpoint was requested
if (log)
- log->Printf ("ProcessGDBRemote::EnableBreakpointSite (size_id = %" PRIu64 ") address = 0x%" PRIx64, site_id, (uint64_t)addr);
+ log->Printf("ProcessGDBRemote::EnableBreakpointSite (size_id = %" PRIu64 ") address = 0x%" PRIx64, site_id, (uint64_t)addr);
+ // Breakpoint already exists and is enabled
if (bp_site->IsEnabled())
{
if (log)
- log->Printf ("ProcessGDBRemote::EnableBreakpointSite (size_id = %" PRIu64 ") address = 0x%" PRIx64 " -- SUCCESS (already enabled)", site_id, (uint64_t)addr);
+ log->Printf("ProcessGDBRemote::EnableBreakpointSite (size_id = %" PRIu64 ") address = 0x%" PRIx64 " -- SUCCESS (already enabled)", site_id, (uint64_t)addr);
return error;
}
- else
+
+ // Get the software breakpoint trap opcode size
+ const size_t bp_op_size = GetSoftwareBreakpointTrapOpcode(bp_site);
+
+ // SupportsGDBStoppointPacket() simply checks a boolean, indicating if this breakpoint type
+ // is supported by the remote stub. These are set to true by default, and later set to false
+ // only after we receive an unimplemented response when sending a breakpoint packet. This means
+ // initially that unless we were specifically instructed to use a hardware breakpoint, LLDB will
+ // attempt to set a software breakpoint. HardwareRequired() also queries a boolean variable which
+ // indicates if the user specifically asked for hardware breakpoints. If true then we will
+ // skip over software breakpoints.
+ if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware) && (!bp_site->HardwareRequired()))
{
- const size_t bp_op_size = GetSoftwareBreakpointTrapOpcode (bp_site);
+ // Try to send off a software breakpoint packet ($Z0)
+ if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointSoftware, true, addr, bp_op_size) == 0)
+ {
+ // The breakpoint was placed successfully
+ bp_site->SetEnabled(true);
+ bp_site->SetType(BreakpointSite::eExternal);
+ return error;
+ }
+
+ // SendGDBStoppointTypePacket() will return an error if it was unable to set this
+ // breakpoint. We need to differentiate between a error specific to placing this breakpoint
+ // or if we have learned that this breakpoint type is unsupported. To do this, we
+ // must test the support boolean for this breakpoint type to see if it now indicates that
+ // this breakpoint type is unsupported. If they are still supported then we should return
+ // with the error code. If they are now unsupported, then we would like to fall through
+ // and try another form of breakpoint.
+ if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware))
+ return error;
- if (bp_site->HardwareRequired())
+ // We reach here when software breakpoints have been found to be unsupported. For future
+ // calls to set a breakpoint, we will not attempt to set a breakpoint with a type that is
+ // known not to be supported.
+ if (log)
+ log->Printf("Software breakpoints are unsupported");
+
+ // So we will fall through and try a hardware breakpoint
+ }
+
+ // The process of setting a hardware breakpoint is much the same as above. We check the
+ // supported boolean for this breakpoint type, and if it is thought to be supported then we
+ // will try to set this breakpoint with a hardware breakpoint.
+ if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware))
+ {
+ // Try to send off a hardware breakpoint packet ($Z1)
+ if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointHardware, true, addr, bp_op_size) == 0)
{
- // Try and set hardware breakpoint, and if that fails, fall through
- // and set a software breakpoint?
- if (m_gdb_comm.SupportsGDBStoppointPacket (eBreakpointHardware))
- {
- if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointHardware, true, addr, bp_op_size) == 0)
- {
- bp_site->SetEnabled(true);
- bp_site->SetType (BreakpointSite::eHardware);
- }
- else
- {
- error.SetErrorString("failed to set hardware breakpoint (hardware breakpoint resources might be exhausted or unavailable)");
- }
- }
- else
- {
- error.SetErrorString("hardware breakpoints are not supported");
- }
+ // The breakpoint was placed successfully
+ bp_site->SetEnabled(true);
+ bp_site->SetType(BreakpointSite::eHardware);
return error;
}
- else if (m_gdb_comm.SupportsGDBStoppointPacket (eBreakpointSoftware))
+
+ // Check if the error was something other then an unsupported breakpoint type
+ if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware))
{
- if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointSoftware, true, addr, bp_op_size) == 0)
- {
- bp_site->SetEnabled(true);
- bp_site->SetType (BreakpointSite::eExternal);
- return error;
- }
+ // Unable to set this hardware breakpoint
+ error.SetErrorString("failed to set hardware breakpoint (hardware breakpoint resources might be exhausted or unavailable)");
+ return error;
}
- return EnableSoftwareBreakpoint (bp_site);
+ // We will reach here when the stub gives an unsported response to a hardware breakpoint
+ if (log)
+ log->Printf("Hardware breakpoints are unsupported");
+
+ // Finally we will falling through to a #trap style breakpoint
}
- if (log)
+ // Don't fall through when hardware breakpoints were specifically requested
+ if (bp_site->HardwareRequired())
{
- const char *err_string = error.AsCString();
- log->Printf ("ProcessGDBRemote::EnableBreakpointSite () error for breakpoint at 0x%8.8" PRIx64 ": %s",
- bp_site->GetLoadAddress(),
- err_string ? err_string : "NULL");
+ error.SetErrorString("hardware breakpoints are not supported");
+ return error;
}
- // We shouldn't reach here on a successful breakpoint enable...
- if (error.Success())
- error.SetErrorToGenericError();
- return error;
+
+ // As a last resort we want to place a manual breakpoint. An instruction
+ // is placed into the process memory using memory write packets.
+ return EnableSoftwareBreakpoint(bp_site);
}
Error
@@ -2399,7 +2435,7 @@ ProcessGDBRemote::DisableBreakpointSite
break;
case BreakpointSite::eHardware:
- if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointSoftware, false, addr, bp_op_size))
+ if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointHardware, false, addr, bp_op_size))
error.SetErrorToGenericError();
break;
More information about the lldb-commits
mailing list