[Lldb-commits] [PATCH] D134882: [lldb] Move breakpoint hit reset code to Target::CreateProcess
labath updated this revision to Diff 464700.
labath added a comment.
use CleanupProcess instead
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134882/new/
https://reviews.llvm.org/D134882
Files:
lldb/source/Target/Process.cpp
lldb/source/Target/Target.cpp
lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
Index: lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
@@ -63,3 +63,54 @@
self.dbg.GetSelectedTarget().GetProcess().Kill()
lldbutil.expect_state_changes(self, self.dbg.GetListener(),
self.process(), [lldb.eStateExited])
+def test_breakpoint_count(self):
+"""
+Test that breakpoint count gets reset for each new connection.
+"""
+class MyResponder(MockGDBServerResponder):
+
+def __init__(self):
+super().__init__()
+self.continued = False
+
+def qfThreadInfo(self):
+return "m47"
+
+def qsThreadInfo(self):
+return "l"
+
+def setBreakpoint(self, packet):
+return "OK"
+
+def readRegister(self, reg):
+# Pretend we're at the breakpoint after we've been resumed.
+return "3412" if self.continued else
"4747"
+
+def cont(self):
+self.continued = True
+return "T05thread=47;reason:breakpoint"
+
+# Connect to the first process and set our breakpoint.
+self.server.responder = MyResponder()
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+
+bkpt = target.BreakpointCreateByAddress(0x1234)
+self.assertTrue(bkpt.IsValid())
+self.assertEqual(bkpt.GetNumLocations(), 1)
+
+# "continue" the process. It should hit our breakpoint.
+process.Continue()
+self.assertState(process.GetState(), lldb.eStateStopped)
+self.assertEqual(bkpt.GetHitCount(), 1)
+
+# Now kill it, and connect again.
+process.Kill()
+self.server.stop()
+self.server = MockGDBServer(self.server_socket_class())
+self.server.start()
+
+process = self.connect(target)
+
+# The hit count should be reset.
+self.assertEqual(bkpt.GetHitCount(), 0)
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -177,6 +177,7 @@
// clean up needs some help from the process.
m_breakpoint_list.ClearAllBreakpointSites();
m_internal_breakpoint_list.ClearAllBreakpointSites();
+ ResetBreakpointHitCounts();
// Disable watchpoints just on the debugger side.
std::unique_lock lock;
this->GetWatchpointList().GetListMutex(lock);
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2761,18 +2761,15 @@
}
Status Process::WillLaunch(Module *module) {
- GetTarget().ResetBreakpointHitCounts();
return DoWillLaunch(module);
}
Status Process::WillAttachToProcessWithID(lldb::pid_t pid) {
- GetTarget().ResetBreakpointHitCounts();
return DoWillAttachToProcessWithID(pid);
}
Status Process::WillAttachToProcessWithName(const char *process_name,
bool wait_for_launch) {
- GetTarget().ResetBreakpointHitCounts();
return DoWillAttachToProcessWithName(process_name, wait_for_launch);
}
Index: lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
@@ -63,3 +63,54 @@
self.dbg.GetSelectedTarget().GetProcess().Kill()
lldbutil.expect_state_changes(self, self.dbg.GetListener(),
self.process(), [lldb.eStateExited])
+def test_breakpoint_count(self):
+"""
+Test that breakpoint count gets reset for each new connection.
+"""
+class MyResponder(MockGDBServerResponder):
+
+def __init__(self):
+super().__init__()
+self.continued = False
+
+def qfThreadInfo(self):
+return "m47"
+
+def qsThreadInfo(self):
+return "l"
+
+def setBreakpoint(self, packet):
+return "OK"
+
+def readRegister(self, reg):
+# Pretend we're
[Lldb-commits] [PATCH] D134882: [lldb] Move breakpoint hit reset code to Target::CreateProcess
jingham added a comment. The WillXXX have the virtue that they are at least self-documenting "Before you do XXX this will be called." As it stands somebody working on Target could be forgiven for thinking Target::CreateProcess was just a convenience method, and redo it somewhere inline. If we are going to require CreateProcess as the only way to do this, we should at least state that somewhere. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134882/new/ https://reviews.llvm.org/D134882 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D134882: [lldb] Move breakpoint hit reset code to Target::CreateProcess
labath added a comment. I'm not sure what kind of guarantees are you looking for. `m_process_sp` is a private member of the Target class, so it's not like just anyone can come in and change it. There's no way to stop code from inside the Target class from changing it without going through the `CreateProcess` method, but the same can be said about the calls to `WillXXX` methods in the process class. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134882/new/ https://reviews.llvm.org/D134882 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D134882: [lldb] Move breakpoint hit reset code to Target::CreateProcess
jingham added a comment. If there's some guarantee that Target::CreateProcess HAS to be the way that a process is created, then this is fine. Is there such a guarantee - the function is pretty trivial so somebody might be tempted to do the work in some other place but miss the breakpoint resetting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134882/new/ https://reviews.llvm.org/D134882 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D134882: [lldb] Move breakpoint hit reset code to Target::CreateProcess
labath created this revision.
labath added reviewers: fdeazeve, jingham.
Herald added a project: All.
labath requested review of this revision.
Herald added a project: LLDB.
This ensures it is run regardless of the method we use to initiate the
session (previous version did not handle connects).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D134882
Files:
lldb/source/Target/Process.cpp
lldb/source/Target/Target.cpp
lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
Index: lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
@@ -63,3 +63,54 @@
self.dbg.GetSelectedTarget().GetProcess().Kill()
lldbutil.expect_state_changes(self, self.dbg.GetListener(),
self.process(), [lldb.eStateExited])
+def test_breakpoint_count(self):
+"""
+Test that breakpoint count gets reset for each new connection.
+"""
+class MyResponder(MockGDBServerResponder):
+
+def __init__(self):
+super().__init__()
+self.continued = False
+
+def qfThreadInfo(self):
+return "m47"
+
+def qsThreadInfo(self):
+return "l"
+
+def setBreakpoint(self, packet):
+return "OK"
+
+def readRegister(self, reg):
+# Pretend we're at the breakpoint after we've been resumed.
+return "3412" if self.continued else
"4747"
+
+def cont(self):
+self.continued = True
+return "T05thread=47;reason:breakpoint"
+
+# Connect to the first process and set our breakpoint.
+self.server.responder = MyResponder()
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+
+bkpt = target.BreakpointCreateByAddress(0x1234)
+self.assertTrue(bkpt.IsValid())
+self.assertEqual(bkpt.GetNumLocations(), 1)
+
+# "continue" the process. It should hit our breakpoint.
+process.Continue()
+self.assertState(process.GetState(), lldb.eStateStopped)
+self.assertEqual(bkpt.GetHitCount(), 1)
+
+# Now kill it, and connect again.
+process.Kill()
+self.server.stop()
+self.server = MockGDBServer(self.server_socket_class())
+self.server.start()
+
+process = self.connect(target)
+
+# The hit count should be reset.
+self.assertEqual(bkpt.GetHitCount(), 0)
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -209,6 +209,7 @@
if (!listener_sp)
listener_sp = GetDebugger().GetListener();
DeleteCurrentProcess();
+ ResetBreakpointHitCounts();
m_process_sp = Process::FindPlugin(shared_from_this(), plugin_name,
listener_sp, crash_file, can_connect);
return m_process_sp;
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2761,18 +2761,15 @@
}
Status Process::WillLaunch(Module *module) {
- GetTarget().ResetBreakpointHitCounts();
return DoWillLaunch(module);
}
Status Process::WillAttachToProcessWithID(lldb::pid_t pid) {
- GetTarget().ResetBreakpointHitCounts();
return DoWillAttachToProcessWithID(pid);
}
Status Process::WillAttachToProcessWithName(const char *process_name,
bool wait_for_launch) {
- GetTarget().ResetBreakpointHitCounts();
return DoWillAttachToProcessWithName(process_name, wait_for_launch);
}
Index: lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
@@ -63,3 +63,54 @@
self.dbg.GetSelectedTarget().GetProcess().Kill()
lldbutil.expect_state_changes(self, self.dbg.GetListener(),
self.process(), [lldb.eStateExited])
+def test_breakpoint_count(self):
+"""
+Test that breakpoint count gets reset for each new connection.
+"""
+class MyResponder(MockGDBServerResponder):
+
+def __init__(self):
+super().__init__()
+self.continued = False
+
+def qfThreadInfo(self):
+return "m47"
+
+def qsThreadInfo(self):
+return "l"
+
+def
