[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.
Alex Langford via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Thu Feb 22 14:37:58 PST 2018
xiaobai added inline comments.
================
Comment at: source/Breakpoint/BreakpointSiteList.cpp:191
if (lower != m_bp_site_list.begin()) {
- collection::const_iterator prev_pos = lower;
- prev_pos--;
+ auto prev_pos = std::prev(lower);
const BreakpointSiteSP &prev_bp = (*prev_pos).second;
----------------
If `m_bp_site_list` is empty, `prev_pos` could be `m_bp_site_list.end()`, I believe, so you would have to check for that here. This is assuming that this method can be invoked when `m_bp_site_list` is empty, which I'm not entirely sure about.
================
Comment at: source/Target/Process.cpp:1817
+ m_breakpoint_site_list.ForEach([this](BreakpointSite &bp_site) {
// bp_site->SetEnabled(true);
+ DisableBreakpointSite(&bp_site);
----------------
Good opportunity to delete dead code?
================
Comment at: source/Target/Process.cpp:2437
+ for (auto &bp_site : enabled_bp_sites_in_range)
+ update_status(DisableSoftwareBreakpoint(bp_site.get()));
----------------
If disabling any breakpoint fails, the status's error string will be "failed to re-enable one or more breakpoints". This is kind of confusing, imo.
================
Comment at: unittests/Process/common/ProcessWriteMemoryTest.cpp:1
+//===-- ProcessorTraceMonitorTest.cpp ------------------------- -*- C++ -*-===//
+//
----------------
`ProcessTraceMonitorTest.cpp` -> `ProcessWriteMemoryTest.cpp`
https://reviews.llvm.org/D39967
More information about the lldb-commits
mailing list