[Lldb-commits] [PATCH] Patch for fixing the handling of hardware breakpoints
Deepak Panickal
deepak2427 at gmail.com
Wed Feb 19 06:50:56 PST 2014
Hardware breakpoints were never being used, unless specifically requested by the user. We have modified it such that a software breakpoint is tried first, if that fails, we try to set a hardware breakpoint and if else, we try to handle the breakpoint from LLDB.
There is now a clean fall-through so that all breakpoint methods are tried. This fixes working with a target with only hardware breakpoint support.
http://llvm-reviews.chandlerc.com/D2826
Files:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2635,19 +2635,13 @@
return false;
}
-
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),
@@ -2656,18 +2650,16 @@
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())
- return response.GetError();
- }
- else
- {
+ // otherwise we assume that the breakpoint could not be set
switch (type)
{
case eBreakpointSoftware: m_supports_z0 = false; break;
@@ -2676,8 +2668,13 @@
case eWatchpointRead: m_supports_z3 = false; break;
case eWatchpointReadWrite: m_supports_z4 = false; break;
}
+ // empty packet informs us that breakpoint is not supported
+ if (response.IsUnsupportedResponse())
+ // mark unsupported breakpoint as an error, so we can fall through
+ // to another breakpoint type if possible
+ return UINT8_MAX;
+ return response.GetError();
}
-
return UINT8_MAX;
}
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2279,68 +2279,64 @@
Error error;
assert (bp_site != NULL);
+ // 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);
+ // 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);
return error;
}
- else
- {
- const size_t bp_op_size = GetSoftwareBreakpointTrapOpcode (bp_site);
- if (bp_site->HardwareRequired())
+ // get the software breakpoint trap opcode size
+ const size_t bp_op_size = GetSoftwareBreakpointTrapOpcode(bp_site);
+
+ // condition to use software breakpoints (Z0)
+ bool useSW = (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware));
+ useSW &= !(bp_site->HardwareRequired());
+
+ // do we use a software breakpoint
+ if (useSW)
+ {
+ if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointSoftware, 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");
- }
+ bp_site->SetEnabled(true);
+ bp_site->SetType(BreakpointSite::eExternal);
return error;
}
- else if (m_gdb_comm.SupportsGDBStoppointPacket (eBreakpointSoftware))
- {
- if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointSoftware, true, addr, bp_op_size) == 0)
- {
- bp_site->SetEnabled(true);
- bp_site->SetType (BreakpointSite::eExternal);
- return error;
- }
- }
-
- return EnableSoftwareBreakpoint (bp_site);
}
- if (log)
+ // condition to use hardware breakpoints (Z1)
+ bool useHW = (bp_site->HardwareRequired());
+ useHW |= !(m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware));
+ useHW &= (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware));
+
+ // do we use a hardware breakpoint
+ if (useHW)
{
- 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");
+ if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointHardware, true, addr, bp_op_size) == 0)
+ {
+ bp_site->SetEnabled(true);
+ bp_site->SetType(BreakpointSite::eHardware);
+ return error;
+ }
+ // error setting hardware breakpoint
+ error.SetErrorString("failed to set hardware breakpoint (hardware breakpoint resources might be exhausted or unavailable)");
}
- // 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. we placing a breakpoint
+ // instruction into the process memory using memory write packets
+ return EnableSoftwareBreakpoint(bp_site);
}
Error
@@ -2366,7 +2362,7 @@
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;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2826.1.patch
Type: text/x-patch
Size: 7543 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140219/19f4b529/attachment.bin>
More information about the lldb-commits
mailing list