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

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

fdeazeve added a comment.

In D133858#3805630 <https://reviews.llvm.org/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 counts don't get reset in case of a reconnect. This isn't a particularly common use case (and the only reason I've noticed it is that for `PlatformQemuUser`, all "launches" are actually "connects" under the hood <https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp#L227>), but I've verified that this problem can be reproduced by issuing connect commands manually (on the regular host platform). I'm pretty sure that was not intentional.
>
> Fixing this by adding another callout to `ResetBreakpointHitCounts` would be easy enough, but I'm also thinking if there isn't a better place from which to call this function, one that would capture all three scenarios in a single statement. I think that one such place could be `Target::CreateProcess`. This function is called by all three code paths, and it's a very good indicator that we will be starting a new debug session.
>
> What do you think?

My understanding is that there is an obligation of calling the WillXX methods before calling XX, so by placing the reset code in the WillXX functions we can rely on that guarantee. Right now, the same is implicit for "one must call Target::CreateProcess before launching or attaching". We could instead define a WillConnect and have the usual contract for that.

The code is fairly new to me, so I'm not confident enough to make a judgement call here. Thoughts?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133858/new/

https://reviews.llvm.org/D133858