Page MenuHomePhabricator

[LLDB] CommandObjectThreadUntil::DoExecute() sets the wrong selected thread ID
AcceptedPublic

Authored by ramana-nvr on Jul 3 2018, 12:07 AM.

Details

Reviewers
jingham
labath
Summary

For the 'thread until' command, the selected thread ID, to perform the operation on, could be of the current thread or the specified thread.

Diff Detail

Event Timeline

ramana-nvr created this revision.Jul 3 2018, 12:07 AM

This is correct, thanks for catching it!

There are a few other places that use m_thread_idx above this change. Everything after the:

if (thread == nullptr) {

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?

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.

ramana-nvr updated this revision to Diff 154059.Jul 4 2018, 1:12 AM

Sorry, I wasn't clear. Suppose that the m_thread_idx coming into DoExecute is LLDB_INVALID_THREAD_ID. That's what happens if you don't select a thread id on the command line, for instance. Then the code around 1164 will set thread to be the default thread. But m_thread_idx will still be LLDB_INVALID_THREAD_ID. Then for instance if the selected frame of thread has no debug info, the error messages will read:

Frame index 0 of thread index 4294967295 has no debug information.

(the value of LLDB_INVALID_THREAD_ID) since the error is still printing m_thread_idx . The errors also need to print thread->GetID()...

The error messages now refer to the thread ID instead of the thread index.

jingham accepted this revision.Jul 12 2018, 10:16 AM

Thanks.

This revision is now accepted and ready to land.Jul 12 2018, 10:16 AM

Could you please add a test for the new behavior as well?

labath resigned from this revision.Aug 9 2019, 2:05 AM