[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha updated this revision to Diff 135711. tatyana-krasnukha added a comment. Have no idea how to know about working around breakpoints for read and write separately. Just rely on support of Z-packets for now. This expectation could be fair, however... In

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

ted added a comment. In https://reviews.llvm.org/D39967#1017754, @labath wrote: > I believe lldb-server does not work around breakpoint opcodes during writing > (it does for reading), but that can be fixed, of course. It should. If it doesn't, then a write over a breakpoint would clobber it.

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

labath added a comment. I believe lldb-server does not work around breakpoint opcodes during writing (it does for reading), but that can be fixed, of course. https://reviews.llvm.org/D39967 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha added a comment. Got it. Yes, you are right. At least debugserver works around breakpoints by himself while writing memory. I will handle this case. https://reviews.llvm.org/D39967 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

clayborg added a comment. In https://reviews.llvm.org/D39967#1017732, @ted wrote: > Traditionally, remote gdbservers handle memory operations that overlap a > software breakpoint. Writes get the new value saved off, and reads get the > breakpoint opcode replaced by the saved value. Indeed.

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

ted added a comment. Traditionally, remote gdbservers handle memory operations that overlap a software breakpoint. Writes get the new value saved off, and reads get the breakpoint opcode replaced by the saved value. https://reviews.llvm.org/D39967

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha added a comment. May be I misunderstand your question, but I checked, that all targets of both lldb-server and debugserver read opcode from memory and save it, when enable software breakpoint. https://reviews.llvm.org/D39967 ___

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

clayborg added a comment. If any of our lldb_private::Process subclasses handle this already, then we need a new virtual function on lldb_private::Process like: virtual bool lldb_private::Process::MemoryWorksAroundSoftwareBreakpoints(bool read); If "read" is true then we are asking about

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

clayborg added a comment. So the question still remains: does this need to be done when debugging with debugserver or lldb-server? I can't remember if memory write works around breakpoint opcodes in debugserver and/or lldb-server. If it does, then this code is only needed for targets that

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha updated this revision to Diff 135674. tatyana-krasnukha added a comment. Change ForEach return type (bool -> void) https://reviews.llvm.org/D39967 Files: include/lldb/Breakpoint/BreakpointSiteList.h source/Breakpoint/BreakpointSiteList.cpp source/Target/Process.cpp

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha added inline comments. Comment at: include/lldb/Breakpoint/BreakpointSiteList.h:130 + using ModifyingCallback = std::function; + bool ForEach(const ModifyingCallback ); clayborg wrote: > I was a bit vague with my

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

labath added inline comments. Comment at: include/lldb/Breakpoint/BreakpointSiteList.h:159-161 + class Guard : private std::unique_lock { +using std::unique_lock::unique_lock; + }; zturner wrote: > Err, I meant to just deleted the `Guard` class entirely

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha updated this revision to Diff 135537. tatyana-krasnukha added a comment. Fix naming https://reviews.llvm.org/D39967 Files: include/lldb/Breakpoint/BreakpointSiteList.h source/Breakpoint/BreakpointSiteList.cpp source/Target/Process.cpp unittests/Process/CMakeLists.txt

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha updated this revision to Diff 135533. tatyana-krasnukha added a comment. Remove Guard class completely https://reviews.llvm.org/D39967 Files: include/lldb/Breakpoint/BreakpointSiteList.h source/Breakpoint/BreakpointSiteList.cpp source/Target/Process.cpp

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha added inline comments. Comment at: source/Target/Process.cpp:2437 + for (auto _site : enabled_bp_sites_in_range) +update_status(DisableSoftwareBreakpoint(bp_site.get())); xiaobai wrote: > If disabling any breakpoint fails, the status's

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

zturner added inline comments. Comment at: include/lldb/Breakpoint/BreakpointSiteList.h:159-161 + class Guard : private std::unique_lock { +using std::unique_lock::unique_lock; + }; Err, I meant to just deleted the `Guard` class entirely and return

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha added a comment. In https://reviews.llvm.org/D39967#1016533, @zturner wrote: > It might be true that allowing the use of manual `lock` and `unlock` is > unsafe, but adding additional code also has some cost. Just look: only 3 additional lines of code;)

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha updated this revision to Diff 135527. tatyana-krasnukha added a comment. Update according suggestions above https://reviews.llvm.org/D39967 Files: include/lldb/Breakpoint/BreakpointSiteList.h source/Breakpoint/BreakpointSiteList.cpp source/Target/Process.cpp

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

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 _bp =

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

zturner added a comment. Personally I think it would be clearer to just use `std::unique_lock<>`. Already it's locking the mutex twice, once with a `lock_guard` and once when creating a `BreakpointSiteList::Guard`. Which works I guess because it's a recursive mutex, but it's still confusing.

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

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 _bp =

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

zturner added inline comments. Comment at: include/lldb/Breakpoint/BreakpointSiteList.h:159-175 + class Guard final { +std::recursive_mutex *m_mutex; - typedef void (*BreakpointSiteSPMapFunc)(lldb::BreakpointSiteSP , - void

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha added a comment. In https://reviews.llvm.org/D39967#1015860, @clayborg wrote: > So one issue with this is some things, like GDB servers, might not need you > to disable breakpoints in a range because they actually set the breakpoint > for you and you don't know what opcode

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha updated this revision to Diff 135518. tatyana-krasnukha added a comment. Use Disable|EnableBreakpointSite instead of Disable|EnableSoftwareBreakpoint. Make BreakpointSiteList::ForEach callbacks return bool. https://reviews.llvm.org/D39967 Files:

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

labath added inline comments. Comment at: include/lldb/Breakpoint/BreakpointSiteList.h:159-175 + class Guard final { +std::recursive_mutex *m_mutex; - typedef void (*BreakpointSiteSPMapFunc)(lldb::BreakpointSiteSP , - void

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

clayborg added a comment. So one issue with this is some things, like GDB servers, might not need you to disable breakpoints in a range because they actually set the breakpoint for you and you don't know what opcode they used. I am hoping this works with that and doesn't cause us to manually

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha updated this revision to Diff 135397. tatyana-krasnukha added a comment. diff with more context https://reviews.llvm.org/D39967 Files: include/lldb/Breakpoint/BreakpointSiteList.h source/Breakpoint/BreakpointSiteList.cpp source/Target/Process.cpp

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha updated this revision to Diff 135387. tatyana-krasnukha added a comment. pass callbacks by const-reference https://reviews.llvm.org/D39967 Files: include/lldb/Breakpoint/BreakpointSiteList.h source/Breakpoint/BreakpointSiteList.cpp source/Target/Process.cpp