[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet
ramana-nvr added a comment. Sorry, I am not helpful to you in providing a unit test case for this patch. I am still learning about test suite framework and/or trying to get the lldb test suite up and running. Regarding how I got to this, it is not that I have run into some issue due to the original code. For my private port, I had to create a thread ID list similar to m_thread_ids and during code browsing for that I happened to see the code in concern and observed that clearing m_thread_pcs is wrong there. After the process gets stopped, while trying to set the thread stop info, we also update the thread list (ProcessGDBRemote::SetThreadStopInfo() ---> Process::UpdateThreadListIfNeeded() ---> ProcessGDBRemote::UpdateThreadList()) and here we can reach UpdateThreadIDList() if m_thread_ids.empty() is true i.e. if we somehow failed to populate m_thread_ids while parsing the stop info packet and need to populate the m_thread_ids now. Repository: rL LLVM https://reviews.llvm.org/D48868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D48865: [LLDB] CommandObjectThreadUntil::DoExecute() sets the wrong selected thread ID
ramana-nvr updated this revision to Diff 155114.
ramana-nvr added a comment.
The error messages now refer to the thread ID instead of the thread index.
https://reviews.llvm.org/D48865
Files:
source/Commands/CommandObjectThread.cpp
Index: source/Commands/CommandObjectThread.cpp
===
--- source/Commands/CommandObjectThread.cpp
+++ source/Commands/CommandObjectThread.cpp
@@ -1183,8 +1183,8 @@
thread->GetStackFrameAtIndex(m_options.m_frame_idx).get();
if (frame == nullptr) {
result.AppendErrorWithFormat(
-"Frame index %u is out of range for thread %u.\n",
-m_options.m_frame_idx, m_options.m_thread_idx);
+"Frame index %u is out of range for thread id %" PRIu64 ".\n",
+m_options.m_frame_idx, thread->GetID());
result.SetStatus(eReturnStatusFailed);
return false;
}
@@ -1201,9 +1201,8 @@
if (line_table == nullptr) {
result.AppendErrorWithFormat("Failed to resolve the line table for "
- "frame %u of thread index %u.\n",
- m_options.m_frame_idx,
- m_options.m_thread_idx);
+ "frame %u of thread id %" PRIu64 ".\n",
+ m_options.m_frame_idx, thread->GetID());
result.SetStatus(eReturnStatusFailed);
return false;
}
@@ -1279,13 +1278,13 @@
new_plan_sp->SetOkayToDiscard(false);
} else {
result.AppendErrorWithFormat(
-"Frame index %u of thread %u has no debug information.\n",
-m_options.m_frame_idx, m_options.m_thread_idx);
+ "Frame index %u of thread id %" PRIu64 " has no debug
information.\n",
+ m_options.m_frame_idx, thread->GetID());
result.SetStatus(eReturnStatusFailed);
return false;
}
- process->GetThreadList().SetSelectedThreadByID(m_options.m_thread_idx);
+ process->GetThreadList().SetSelectedThreadByID(thread->GetID());
StreamString stream;
Status error;
Index: source/Commands/CommandObjectThread.cpp
===
--- source/Commands/CommandObjectThread.cpp
+++ source/Commands/CommandObjectThread.cpp
@@ -1183,8 +1183,8 @@
thread->GetStackFrameAtIndex(m_options.m_frame_idx).get();
if (frame == nullptr) {
result.AppendErrorWithFormat(
-"Frame index %u is out of range for thread %u.\n",
-m_options.m_frame_idx, m_options.m_thread_idx);
+"Frame index %u is out of range for thread id %" PRIu64 ".\n",
+m_options.m_frame_idx, thread->GetID());
result.SetStatus(eReturnStatusFailed);
return false;
}
@@ -1201,9 +1201,8 @@
if (line_table == nullptr) {
result.AppendErrorWithFormat("Failed to resolve the line table for "
- "frame %u of thread index %u.\n",
- m_options.m_frame_idx,
- m_options.m_thread_idx);
+ "frame %u of thread id %" PRIu64 ".\n",
+ m_options.m_frame_idx, thread->GetID());
result.SetStatus(eReturnStatusFailed);
return false;
}
@@ -1279,13 +1278,13 @@
new_plan_sp->SetOkayToDiscard(false);
} else {
result.AppendErrorWithFormat(
-"Frame index %u of thread %u has no debug information.\n",
-m_options.m_frame_idx, m_options.m_thread_idx);
+ "Frame index %u of thread id %" PRIu64 " has no debug information.\n",
+ m_options.m_frame_idx, thread->GetID());
result.SetStatus(eReturnStatusFailed);
return false;
}
- process->GetThreadList().SetSelectedThreadByID(m_options.m_thread_idx);
+ process->GetThreadList().SetSelectedThreadByID(thread->GetID());
StreamString stream;
Status error;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet
ramana-nvr updated this revision to Diff 154947.
ramana-nvr added a comment.
For now, leaving the m_thread_pcs.clear() in UpdateThreadIDList() unchanged as
it is not causing any problems.
https://reviews.llvm.org/D48868
Files:
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1523,7 +1523,6 @@
size_t
ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(std::string )
{
m_thread_ids.clear();
- m_thread_pcs.clear();
size_t comma_pos;
lldb::tid_t tid;
while ((comma_pos = value.find(',')) != std::string::npos) {
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1523,7 +1523,6 @@
size_t
ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(std::string ) {
m_thread_ids.clear();
- m_thread_pcs.clear();
size_t comma_pos;
lldb::tid_t tid;
while ((comma_pos = value.find(',')) != std::string::npos) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet
ramana-nvr added a comment. Sorry, I wasn't clear in my previous comment. In https://reviews.llvm.org/D48868#1158236, @jasonmolenda wrote: > I don't have strong feelings about removing the m_thread_pcs() in > UpdateThreadIDList(), but I think I would prefer leaving it in to changing it > unless we know it's causing problems. Does this cause problems in your > environment? No, it did not cause any problems in our environment. But it is redundant as UpdateThreadPCsFromStopReplyThreadsValue(), which gets called immediately after clearing m_thread_pcs in UpdateThreadIDList(), will anyway clear the old m_thread_pcs (line 1545 without my changes) before populating them from the thread-pcs: received in the stop reply packet. https://reviews.llvm.org/D48868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet
ramana-nvr added a comment. In https://reviews.llvm.org/D48868#1156416, @jasonmolenda wrote: > If jThreadsInfo isn't implemented, we fall back to looking for the > thread-pcs: and threads: key-value pairs in the stop packet that we received. > > We look for the threads: list and if it is present, we call > UpdateThreadIDsFromStopReplyThreadsValue which clears m_thread_pcs again (bug > #2) and then adds any threads listed to the m_thread_ids. Yes, we shouldn't be clearing the m_thread_pcs in UpdateThreadIDsFromStopReplyThreadsValue() otherwise we would loose the thread PCs populated from stop reply thread-pcs: list just before the UpdateThreadIDsFromStopReplyThreadsValue() call. The other m_thread_pcs.clear() in UpdateThreadIDList() is redundant as we are anyway clearing m_thread_pcs in UpdateThreadPCsFromStopReplyThreadsValue(). https://reviews.llvm.org/D48868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D48865: [LLDB] CommandObjectThreadUntil::DoExecute() sets the wrong selected thread ID
ramana-nvr updated this revision to Diff 154059.
https://reviews.llvm.org/D48865
Files:
source/Commands/CommandObjectThread.cpp
Index: source/Commands/CommandObjectThread.cpp
===
--- source/Commands/CommandObjectThread.cpp
+++ source/Commands/CommandObjectThread.cpp
@@ -1183,7 +1183,7 @@
thread->GetStackFrameAtIndex(m_options.m_frame_idx).get();
if (frame == nullptr) {
result.AppendErrorWithFormat(
-"Frame index %u is out of range for thread %u.\n",
+"Frame index %u is out of range for thread index %u.\n",
m_options.m_frame_idx, m_options.m_thread_idx);
result.SetStatus(eReturnStatusFailed);
return false;
@@ -1279,13 +1279,13 @@
new_plan_sp->SetOkayToDiscard(false);
} else {
result.AppendErrorWithFormat(
-"Frame index %u of thread %u has no debug information.\n",
+"Frame index %u of thread index %u has no debug information.\n",
m_options.m_frame_idx, m_options.m_thread_idx);
result.SetStatus(eReturnStatusFailed);
return false;
}
- process->GetThreadList().SetSelectedThreadByID(m_options.m_thread_idx);
+ process->GetThreadList().SetSelectedThreadByID(thread->GetID());
StreamString stream;
Status error;
Index: source/Commands/CommandObjectThread.cpp
===
--- source/Commands/CommandObjectThread.cpp
+++ source/Commands/CommandObjectThread.cpp
@@ -1183,7 +1183,7 @@
thread->GetStackFrameAtIndex(m_options.m_frame_idx).get();
if (frame == nullptr) {
result.AppendErrorWithFormat(
-"Frame index %u is out of range for thread %u.\n",
+"Frame index %u is out of range for thread index %u.\n",
m_options.m_frame_idx, m_options.m_thread_idx);
result.SetStatus(eReturnStatusFailed);
return false;
@@ -1279,13 +1279,13 @@
new_plan_sp->SetOkayToDiscard(false);
} else {
result.AppendErrorWithFormat(
-"Frame index %u of thread %u has no debug information.\n",
+"Frame index %u of thread index %u has no debug information.\n",
m_options.m_frame_idx, m_options.m_thread_idx);
result.SetStatus(eReturnStatusFailed);
return false;
}
- process->GetThreadList().SetSelectedThreadByID(m_options.m_thread_idx);
+ process->GetThreadList().SetSelectedThreadByID(thread->GetID());
StreamString stream;
Status error;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D48865: [LLDB] CommandObjectThreadUntil::DoExecute() sets the wrong selected thread ID
ramana-nvr added a reviewer: jingham. ramana-nvr added a comment. In https://reviews.llvm.org/D48865#1150995, @jingham wrote: > check should also use thread->GetID not m_options.m_thread_idx. This is all > error reporting, so it's not a big deal. But having the error be "failed to > resolve line table for frame 5 of thread 4294967295" would be confusing. > Could you change those uses as well? On the trunk, the error message will be "Failed to resolve the line table for frame 5 of thread index 3", which I think is okay. Changed couple of others. https://reviews.llvm.org/D48865 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet
ramana-nvr created this revision.
ramana-nvr added a reviewer: labath.
The function ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(),
which is being called after the thread PCs are updated, is clearing the thread
PC list and that is wrong.
https://reviews.llvm.org/D48868
Files:
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1523,7 +1523,6 @@
size_t
ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(std::string )
{
m_thread_ids.clear();
- m_thread_pcs.clear();
size_t comma_pos;
lldb::tid_t tid;
while ((comma_pos = value.find(',')) != std::string::npos) {
@@ -1598,7 +1597,6 @@
StringExtractorGDBRemote _info = m_stop_packet_stack[i];
const std::string _info_str = stop_info.GetStringRef();
-m_thread_pcs.clear();
const size_t thread_pcs_pos = stop_info_str.find(";thread-pcs:");
if (thread_pcs_pos != std::string::npos) {
const size_t start = thread_pcs_pos + strlen(";thread-pcs:");
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1523,7 +1523,6 @@
size_t
ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(std::string ) {
m_thread_ids.clear();
- m_thread_pcs.clear();
size_t comma_pos;
lldb::tid_t tid;
while ((comma_pos = value.find(',')) != std::string::npos) {
@@ -1598,7 +1597,6 @@
StringExtractorGDBRemote _info = m_stop_packet_stack[i];
const std::string _info_str = stop_info.GetStringRef();
-m_thread_pcs.clear();
const size_t thread_pcs_pos = stop_info_str.find(";thread-pcs:");
if (thread_pcs_pos != std::string::npos) {
const size_t start = thread_pcs_pos + strlen(";thread-pcs:");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D48865: [LLDB] CommandObjectThreadUntil::DoExecute() sets the wrong selected thread ID
ramana-nvr created this revision. ramana-nvr added a reviewer: labath. For the 'thread until' command, the selected thread ID, to perform the operation on, could be of the current thread or the specified thread. https://reviews.llvm.org/D48865 Files: source/Commands/CommandObjectThread.cpp Index: source/Commands/CommandObjectThread.cpp === --- source/Commands/CommandObjectThread.cpp +++ source/Commands/CommandObjectThread.cpp @@ -1285,7 +1285,7 @@ return false; } - process->GetThreadList().SetSelectedThreadByID(m_options.m_thread_idx); + process->GetThreadList().SetSelectedThreadByID(thread->GetID()); StreamString stream; Status error; Index: source/Commands/CommandObjectThread.cpp === --- source/Commands/CommandObjectThread.cpp +++ source/Commands/CommandObjectThread.cpp @@ -1285,7 +1285,7 @@ return false; } - process->GetThreadList().SetSelectedThreadByID(m_options.m_thread_idx); + process->GetThreadList().SetSelectedThreadByID(thread->GetID()); StreamString stream; Status error; ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D48704: [LLDB] Fix for "Bug 37950: ExecutionContext::GetByteOrder() always returns endian::InlHostByteOrder()"
ramana-nvr updated this revision to Diff 153258.
ramana-nvr added a comment.
Created the patch with more context.
https://reviews.llvm.org/D48704
Files:
source/Target/ExecutionContext.cpp
Index: source/Target/ExecutionContext.cpp
===
--- source/Target/ExecutionContext.cpp
+++ source/Target/ExecutionContext.cpp
@@ -188,9 +188,9 @@
lldb::ByteOrder ExecutionContext::GetByteOrder() const {
if (m_target_sp && m_target_sp->GetArchitecture().IsValid())
-m_target_sp->GetArchitecture().GetByteOrder();
+return m_target_sp->GetArchitecture().GetByteOrder();
if (m_process_sp)
-m_process_sp->GetByteOrder();
+return m_process_sp->GetByteOrder();
return endian::InlHostByteOrder();
}
Index: source/Target/ExecutionContext.cpp
===
--- source/Target/ExecutionContext.cpp
+++ source/Target/ExecutionContext.cpp
@@ -188,9 +188,9 @@
lldb::ByteOrder ExecutionContext::GetByteOrder() const {
if (m_target_sp && m_target_sp->GetArchitecture().IsValid())
-m_target_sp->GetArchitecture().GetByteOrder();
+return m_target_sp->GetArchitecture().GetByteOrder();
if (m_process_sp)
-m_process_sp->GetByteOrder();
+return m_process_sp->GetByteOrder();
return endian::InlHostByteOrder();
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D48704: [LLDB] Fix for "Bug 37950: ExecutionContext::GetByteOrder() always returns endian::InlHostByteOrder()"
ramana-nvr created this revision.
ramana-nvr added reviewers: davide, lldb-commits.
ramana-nvr added a project: LLDB.
https://reviews.llvm.org/D48704
Files:
source/Target/ExecutionContext.cpp
Index: source/Target/ExecutionContext.cpp
===
--- source/Target/ExecutionContext.cpp
+++ source/Target/ExecutionContext.cpp
@@ -188,9 +188,9 @@
lldb::ByteOrder ExecutionContext::GetByteOrder() const {
if (m_target_sp && m_target_sp->GetArchitecture().IsValid())
-m_target_sp->GetArchitecture().GetByteOrder();
+return m_target_sp->GetArchitecture().GetByteOrder();
if (m_process_sp)
-m_process_sp->GetByteOrder();
+return m_process_sp->GetByteOrder();
return endian::InlHostByteOrder();
}
Index: source/Target/ExecutionContext.cpp
===
--- source/Target/ExecutionContext.cpp
+++ source/Target/ExecutionContext.cpp
@@ -188,9 +188,9 @@
lldb::ByteOrder ExecutionContext::GetByteOrder() const {
if (m_target_sp && m_target_sp->GetArchitecture().IsValid())
-m_target_sp->GetArchitecture().GetByteOrder();
+return m_target_sp->GetArchitecture().GetByteOrder();
if (m_process_sp)
-m_process_sp->GetByteOrder();
+return m_process_sp->GetByteOrder();
return endian::InlHostByteOrder();
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
