[Lldb-commits] [PATCH] D20436: Clean up vestigial remnants of locking primitives
Zachary Turner via lldb-commits
lldb-commits at lists.llvm.org
Mon May 23 11:32:35 PDT 2016
Here is a patch you can apply on top of your patch to fix the compiler and
runtime errors (on Windows at least).
Most of the compiler errors were around the use of duration_cast. I've
never used std::chrono before, but my best guess is that system_time::now()
is a different type on Windows and other platforms, and so explicitly
specifying the type to store it to results in implicit conversion errors.
Apparently this is exactly what duration_cast is for, so I used it.
There was a semantic change in the event handler that caused runtime
errors. Basically in WaitForEventsInternal() if it didn't find an event
the first time, it would wait for a condition variable, then try again.
But in the changed code, it would wait for a condition variable and then
immediately return failure without retrying. So I fixed this and
everything seems to be going smooth.I would encourage Greg or someone else
to actually test this on OSX, but at least on Windows it looks good with
the attached patch.
On Fri, May 20, 2016 at 5:54 PM Zachary Turner <zturner at google.com> wrote:
> zturner added a comment.
>
> I'm still getting a lot of crashes after this, but this will at least fix
> the compiler errors. I will try to look at the crashes on Monday.
>
>
> ================
> Comment at:
> source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:354-355
> @@ -358,5 +353,4 @@
> {
> - // Calculate absolute timeout value
> - TimeValue timeout = TimeValue::Now();
> - timeout.OffsetWithMicroSeconds(timeout_usec);
> + std::chrono::time_point<std::chrono::system_clock,
> std::chrono::microseconds> until =
> + std::chrono::system_clock::now() +
> std::chrono::microseconds(timeout_usec);
>
> ----------------
> Need to use `auto` here, otherwise there's a compiler error.
>
> ================
> Comment at:
> source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:844
> @@ -845,1 +843,3 @@
> + if (m_async_packet_predicate.WaitForValueEqualTo(
> + false, until -
> std::chrono::system_clock::now(), &timed_out))
> {
> ----------------
> this needs to use
> `std::chrono::duration_cast<std::chrono::microseconds>(until -
> std::chrono::system_clock::now())`. Maybe raise this into a temporary
> variable (also make sure to use auto on the result just in case).
>
> ================
> Comment at:
> source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:860
> @@ -859,2 +859,3 @@
> // Make sure we wait until the continue packet
> has been sent again...
> - if (m_private_is_running.WaitForValueEqualTo
> (true, &timeout_time, &timed_out))
> + if
> (m_private_is_running.WaitForValueEqualTo(true, until -
> std::chrono::system_clock::now(),
> +
> &timed_out))
> ----------------
> `duration_cast` again.
>
> ================
> Comment at: source/Target/Process.cpp:5547
> @@ +5546,3 @@
> + log->Printf("Process::RunThreadPlan(): about to wait
> - now is %llu - endpoint is %llu",
> +
> std::chrono::time_point<std::chrono::system_clock,
> std::chrono::microseconds>(
> + std::chrono::system_clock::now())
> ----------------
> Delete the cast here. Just use `std::chrono::system_clock::now()`.
>
> ================
> Comment at: source/Target/Process.cpp:5551
> @@ +5550,3 @@
> + .count(),
> +
> std::chrono::time_point<std::chrono::system_clock,
> std::chrono::microseconds>(timeout)
> + .time_since_epoch()
> ----------------
> Change to `timeout.time_since_epoch()`
>
>
> Repository:
> rL LLVM
>
> http://reviews.llvm.org/D20436
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20160523/af070bbf/attachment-0001.html>
-------------- next part --------------
diff --git a/source/Core/Listener.cpp b/source/Core/Listener.cpp
index 2cda21b..a639eec 100644
--- a/source/Core/Listener.cpp
+++ b/source/Core/Listener.cpp
@@ -415,24 +415,26 @@ Listener::WaitForEventsInternal(const std::chrono::microseconds &timeout,
if (timeout == std::chrono::microseconds(0))
m_events_condition.wait(lock);
- else
- result = m_events_condition.wait_for(lock, timeout);
-
- if (result == std::cv_status::timeout)
- {
- log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EVENTS);
- if (log != nullptr)
- log->Printf("%p Listener::WaitForEventsInternal() timed out for %s", static_cast<void *>(this),
- m_name.c_str());
- }
- else
- {
- log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EVENTS);
- if (log != nullptr)
- log->Printf("%p Listener::WaitForEventsInternal() unknown error for %s", static_cast<void *>(this),
- m_name.c_str());
- }
- return false;
+ else
+ {
+ result = m_events_condition.wait_for(lock, timeout);
+
+ if (result == std::cv_status::timeout)
+ {
+ log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EVENTS);
+ if (log != nullptr)
+ log->Printf("%p Listener::WaitForEventsInternal() timed out for %s", static_cast<void *>(this),
+ m_name.c_str());
+ }
+ else
+ {
+ log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EVENTS);
+ if (log != nullptr)
+ log->Printf("%p Listener::WaitForEventsInternal() unknown error for %s", static_cast<void *>(this),
+ m_name.c_str());
+ }
+ return false;
+ }
}
}
diff --git a/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 3b04332..472dfb6 100644
--- a/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -351,8 +351,7 @@ GDBRemoteCommunication::ReadPacket (StringExtractorGDBRemote &response, uint32_t
GDBRemoteCommunication::PacketResult
GDBRemoteCommunication::PopPacketFromQueue (StringExtractorGDBRemote &response, uint32_t timeout_usec)
{
- std::chrono::time_point<std::chrono::system_clock, std::chrono::microseconds> until =
- std::chrono::system_clock::now() + std::chrono::microseconds(timeout_usec);
+ auto until = std::chrono::system_clock::now() + std::chrono::microseconds(timeout_usec);
while (true)
{
diff --git a/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index d576df7..7ecedaa 100644
--- a/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -837,11 +837,11 @@ GDBRemoteCommunicationClient::SendPacketAndWaitForResponse
if (log)
log->Printf ("async: sent interrupt");
- std::chrono::time_point<std::chrono::system_clock> until =
- std::chrono::system_clock::now() + std::chrono::seconds(m_packet_timeout);
+ using namespace std::chrono;
+ auto until = system_clock::now() + seconds(m_packet_timeout);
if (m_async_packet_predicate.WaitForValueEqualTo(
- false, until - std::chrono::system_clock::now(), &timed_out))
+ false, duration_cast<milliseconds>(until - system_clock::now()), &timed_out))
{
if (log)
log->Printf ("async: got response");
@@ -857,7 +857,7 @@ GDBRemoteCommunicationClient::SendPacketAndWaitForResponse
}
// Make sure we wait until the continue packet has been sent again...
- if (m_private_is_running.WaitForValueEqualTo(true, until - std::chrono::system_clock::now(),
+ if (m_private_is_running.WaitForValueEqualTo(true, duration_cast<milliseconds>(until - system_clock::now()),
&timed_out))
{
if (log)
diff --git a/source/Target/Process.cpp b/source/Target/Process.cpp
index 8a858f4..e3d405c 100644
--- a/source/Target/Process.cpp
+++ b/source/Target/Process.cpp
@@ -5543,14 +5543,11 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, lldb::ThreadPlanSP &thread_pla
{
if (timeout.count())
{
+ using namespace std::chrono;
+ auto now = system_clock::now();
log->Printf("Process::RunThreadPlan(): about to wait - now is %llu - endpoint is %llu",
- std::chrono::time_point<std::chrono::system_clock, std::chrono::microseconds>(
- std::chrono::system_clock::now())
- .time_since_epoch()
- .count(),
- std::chrono::time_point<std::chrono::system_clock, std::chrono::microseconds>(timeout)
- .time_since_epoch()
- .count());
+ duration_cast<microseconds>(now.time_since_epoch()).count(),
+ duration_cast<microseconds>((now + timeout).time_since_epoch()).count());
}
else
{
More information about the lldb-commits
mailing list