[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

labath added a comment. In D133858#3818755 , @jingham wrote: > If we use Target::CreateProcess to handle this task, how do we ensure that > that will always get called any time we make a new process? I'm not really sure how to answer that. Clearly some

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

jingham added a comment. If we use Target::CreateProcess to handle this task, how do we ensure that that will always get called any time we make a new process? That function doesn't do all that much special, it's only a couple lines long so it could easily get inlined somewhere. Repository:

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

labath added a comment. In D133858#3816105 , @fdeazeve wrote: > In D133858#3805630 , @labath wrote: > >> I am afraid that this patch misses one method of initiating a debug session >> -- connecting to a running d

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

fdeazeve added a comment. In D133858#3805630 , @labath wrote: > I am afraid that this patch misses one method of initiating a debug session > -- connecting to a running debug server (`process connect`, > `SBTarget::ConnectRemote`). The breakpoint hit co

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

labath added a comment. I am afraid that this patch misses one method of initiating a debug session -- connecting to a running debug server (`process connect`, `SBTarget::ConnectRemote`). The breakpoint hit counts don't get reset in case of a reconnect. This isn't a particularly common use case

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

This revision was automatically updated to reflect the committed changes. Closed by commit rG974958749827: [lldb] Reset breakpoint hit count before new runs (authored by fdeazeve). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133858/new/ https://re

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This version looks good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133858/new/ https://reviews.llvm.org/D133858 ___

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

jingham added a comment. The more we gather up the "things you have to do before an operation starts" or after it ends, etc. into WillDoOperation methods, the less ad hoc code you have in the callers that you have to remember to copy over if you are introducing a different way of doing the same

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

fdeazeve added a comment. In D133858#3791290 , @mib wrote: > I don't think it's necessary to add the `DoWillAttachToProcessWithID` method > and override it in each process plugin. I think you could have just reset the > `hit_counter` in the top-level `P

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

mib added a comment. In D133858#3790013 , @jingham wrote: > Resetting the hit counts on rerun is a more useful behavior, so this is all > fine. But the Target is the one that does all the breakpoint management, so > I think the resetting should go thro

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

fdeazeve updated this revision to Diff 460176. fdeazeve added a comment. Also applied Jim's comment to the LocationList class, as this involves a single lock, instead of one lock per location. If this is wrong, please let me know and I'll revert. Repository: rG LLVM Github Monorepo CHANGES SI

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

fdeazeve added inline comments. Comment at: lldb/source/Breakpoint/Breakpoint.cpp:334 + for (size_t i = 0; i < GetNumLocations(); ++i) +GetLocationAtIndex(i)->ResetHitCount(); +} @jingham actually, should I apply the same idea to BreakpointLocationList? Re

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

fdeazeve added a comment. In D133858#3790013 , @jingham wrote: > Resetting the hit counts on rerun is a more useful behavior, so this is all > fine. But the Target is the one that does all the breakpoint management, so > I think the resetting should go

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

fdeazeve updated this revision to Diff 460165. fdeazeve added a comment. Addressed review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133858/new/ https://reviews.llvm.org/D133858 Files: lldb/include/lldb/Breakpoint/Breakpoint.h lldb

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. Resetting the hit counts on rerun is a more useful behavior, so this is all fine. But the Target is the one that does all the breakpoint management, so I think the resetting shoul

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

fdeazeve added inline comments. Comment at: lldb/source/Target/Process.cpp:2763-2766 +static void ResetHitCounts(Process &Proc) { + for (const auto &BP : Proc.GetTarget().GetBreakpointList().Breakpoints()) +BP->ResetHitCount(); +} JDevlieghere wrote: > Can t

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

JDevlieghere added inline comments. Comment at: lldb/source/Target/Process.cpp:2763-2766 +static void ResetHitCounts(Process &Proc) { + for (const auto &BP : Proc.GetTarget().GetBreakpointList().Breakpoints()) +BP->ResetHitCount(); +} Can this be a private m

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

fdeazeve added inline comments. Comment at: lldb/source/Breakpoint/Breakpoint.cpp:332 +void Breakpoint::ResetHitCount() { + m_hit_counter.Reset(); + for (size_t i = 0; i < GetNumLocations(); ++i) fdeazeve wrote: > By the way, I don' think this counter is ever i

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

fdeazeve added inline comments. Herald added a subscriber: JDevlieghere. Comment at: lldb/source/Breakpoint/Breakpoint.cpp:332 +void Breakpoint::ResetHitCount() { + m_hit_counter.Reset(); + for (size_t i = 0; i < GetNumLocations(); ++i) By the way, I don' think

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

fdeazeve created this revision. Herald added a project: All. fdeazeve requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. A common debugging pattern is to set a breakpoint that only stops after a number of hits is recorded. The current implemen