[Lldb-commits] [lldb] 4c54399 - [lldb] [Process/FreeBSDRemote] Explicitly copy dbregs to new threads
Michał Górny via lldb-commits
lldb-commits at lists.llvm.org
Tue Nov 10 05:18:21 PST 2020
Author: Michał Górny
Date: 2020-11-10T14:18:03+01:00
New Revision: 4c54399b7eaa487e91c32a0d9c2537611c25aee7
URL: https://github.com/llvm/llvm-project/commit/4c54399b7eaa487e91c32a0d9c2537611c25aee7
DIFF: https://github.com/llvm/llvm-project/commit/4c54399b7eaa487e91c32a0d9c2537611c25aee7.diff
LOG: [lldb] [Process/FreeBSDRemote] Explicitly copy dbregs to new threads
Explicitly copy dbregs to new threads to ensure that watchpoints
are propagated properly. Fixes the test failure due to apparent kernel
race between reporting a new thread and resuming main thread execution
that makes implicit inheritance of dbregs unreliable. By copying them
explicitly, we ensure that the new thread correctly respects watchpoints
that were set after the thread was created but before it was reported.
The code is copied from the NetBSD plugin and modernized to use
llvm::Error.
Differential Revision: https://reviews.llvm.org/D91032
Added:
Modified:
lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.h
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
lldb/test/API/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py
Removed:
################################################################################
diff --git a/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp b/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
index 8f0936c169b9..fd3c05ab411e 100644
--- a/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -199,7 +199,25 @@ void NativeProcessFreeBSD::MonitorSIGTRAP(lldb::pid_t pid) {
if (info.pl_flags & (PL_FLAG_BORN | PL_FLAG_EXITED)) {
if (info.pl_flags & PL_FLAG_BORN) {
LLDB_LOG(log, "monitoring new thread, tid = {0}", info.pl_lwpid);
- AddThread(info.pl_lwpid);
+ NativeThreadFreeBSD &t = AddThread(info.pl_lwpid);
+
+ // Technically, the FreeBSD kernel copies the debug registers to new
+ // threads. However, there is a non-negligible delay between acquiring
+ // the DR values and reporting the new thread during which the user may
+ // establish a new watchpoint. In order to ensure that watchpoints
+ // established during this period are propagated to new threads,
+ // explicitly copy the DR value at the time the new thread is reported.
+ //
+ // See also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=250954
+
+ llvm::Error error = t.CopyWatchpointsFrom(
+ static_cast<NativeThreadFreeBSD &>(*GetCurrentThread()));
+ if (error) {
+ LLDB_LOG(log, "failed to copy watchpoints to new thread {0}: {1}",
+ info.pl_lwpid, llvm::toString(std::move(error)));
+ SetState(StateType::eStateInvalid);
+ return;
+ }
} else /*if (info.pl_flags & PL_FLAG_EXITED)*/ {
LLDB_LOG(log, "thread exited, tid = {0}", info.pl_lwpid);
RemoveThread(info.pl_lwpid);
diff --git a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.h b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.h
index 47201cf16777..0000484beac9 100644
--- a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.h
+++ b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.h
@@ -29,6 +29,8 @@ class NativeRegisterContextFreeBSD
static NativeRegisterContextFreeBSD *
CreateHostNativeRegisterContextFreeBSD(const ArchSpec &target_arch,
NativeThreadProtocol &native_thread);
+ virtual llvm::Error
+ CopyHardwareWatchpointsFrom(NativeRegisterContextFreeBSD &source) = 0;
protected:
virtual NativeProcessFreeBSD &GetProcess();
diff --git a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
index 4e0fb280ceff..fc1ef0381f78 100644
--- a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
+++ b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -1169,4 +1169,19 @@ int NativeRegisterContextFreeBSD_x86_64::GetDR(int num) const {
}
}
+llvm::Error NativeRegisterContextFreeBSD_x86_64::CopyHardwareWatchpointsFrom(
+ NativeRegisterContextFreeBSD &source) {
+ auto &r_source = static_cast<NativeRegisterContextFreeBSD_x86_64 &>(source);
+ Status res = r_source.ReadRegisterSet(DBRegSet);
+ if (!res.Fail()) {
+ // copy dbregs only if any watchpoints were set
+ if ((r_source.m_dbr.dr[7] & 0xFF) == 0)
+ return llvm::Error::success();
+
+ m_dbr = r_source.m_dbr;
+ res = WriteRegisterSet(DBRegSet);
+ }
+ return res.ToError();
+}
+
#endif // defined(__x86_64__)
diff --git a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
index d2abf453dfaf..2ec96e344880 100644
--- a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
+++ b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
@@ -50,6 +50,9 @@ class NativeRegisterContextFreeBSD_x86_64
Status WriteAllRegisterValues(const lldb::DataBufferSP &data_sp) override;
+ llvm::Error
+ CopyHardwareWatchpointsFrom(NativeRegisterContextFreeBSD &source) override;
+
private:
// Private member types.
enum { GPRegSet, FPRegSet, XSaveRegSet, DBRegSet };
diff --git a/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp b/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
index 3c80f113b197..43494871be07 100644
--- a/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
@@ -165,14 +165,14 @@ std::string NativeThreadFreeBSD::GetName() {
}
if (error != 0) {
len = 0;
- LLDB_LOG(log, "tid = {0} in state {1} failed to get thread name: {2}", GetID(),
- m_state, strerror(errno));
+ LLDB_LOG(log, "tid = {0} in state {1} failed to get thread name: {2}",
+ GetID(), m_state, strerror(errno));
}
kp.resize(len / sizeof(struct kinfo_proc));
break;
}
- for (auto& procinfo : kp) {
+ for (auto &procinfo : kp) {
if (procinfo.ki_tid == static_cast<lwpid_t>(GetID()))
return procinfo.ki_tdname;
}
@@ -273,3 +273,14 @@ Status NativeThreadFreeBSD::RemoveHardwareBreakpoint(lldb::addr_t addr) {
return Status("Clearing hardware breakpoint failed.");
}
+
+llvm::Error
+NativeThreadFreeBSD::CopyWatchpointsFrom(NativeThreadFreeBSD &source) {
+ llvm::Error s = GetRegisterContext().CopyHardwareWatchpointsFrom(
+ source.GetRegisterContext());
+ if (!s) {
+ m_watchpoint_index_map = source.m_watchpoint_index_map;
+ m_hw_break_index_map = source.m_hw_break_index_map;
+ }
+ return s;
+}
diff --git a/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h b/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
index e4d494174736..4e997b3fb4bb 100644
--- a/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
+++ b/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
@@ -64,7 +64,7 @@ class NativeThreadFreeBSD : public NativeThreadProtocol {
void SetRunning();
void SetStepping();
- Status CopyWatchpointsFrom(NativeThreadFreeBSD &source);
+ llvm::Error CopyWatchpointsFrom(NativeThreadFreeBSD &source);
// Member Variables
lldb::StateType m_state;
diff --git a/lldb/test/API/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py b/lldb/test/API/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py
index aac83f9c3b75..542a5d811064 100644
--- a/lldb/test/API/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py
+++ b/lldb/test/API/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py
@@ -22,7 +22,6 @@ def test_watchpoint_before_thread_start(self):
"""Test that we can hit a watchpoint we set before starting another thread"""
self.do_watchpoint_test("Before running the thread")
- @expectedFailureAll(oslist=["freebsd"])
def test_watchpoint_after_thread_launch(self):
"""Test that we can hit a watchpoint we set after launching another thread"""
self.do_watchpoint_test("After launching the thread")
More information about the lldb-commits
mailing list