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 (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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133858

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to