[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

2018-07-24 Thread Venkata Ramanaiah via Phabricator via lldb-commits
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

Re: [Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

2018-07-16 Thread Jason Molenda via lldb-commits
That's a good point Pavel. I tried to write one (below) but I never saw what the original failure mode was. Venkata, can you help to make a test case that fails before the patch and works after? Or explain what bug was being fixed exactly? I could see that the code was wrong from reading

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

2018-07-16 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a subscriber: ramana-nvr. jasonmolenda added a comment. That's a good point Pavel. I tried to write one (below) but I never saw what the original failure mode was. Venkata, can you help to make a test case that fails before the patch and works after? Or explain what bug

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

2018-07-16 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Could you also add a test case for this? I think it should be possible to test this via the gdb-client (`test/testcases/functionalities/gdb_remote_client/`) test suite. If I understood the previous comments correctly, you'll need to mock a server that sends a

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

2018-07-12 Thread Phabricator via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL336956: Remove incorrect thread-pc-values clearing (authored by jmolenda, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

Re: [Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

2018-07-11 Thread Ramana via lldb-commits
I do not have commit access to LLDB. Could someone please commit the below changes? On Thu, Jul 12, 2018 at 2:32 AM, Jason Molenda via Phabricator < revi...@reviews.llvm.org> wrote: > jasonmolenda accepted this revision. > jasonmolenda added a comment. > This revision is now accepted and ready

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

2018-07-11 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. looks good. https://reviews.llvm.org/D48868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

2018-07-11 Thread Venkata Ramanaiah via Phabricator via lldb-commits
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:

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

2018-07-10 Thread Venkata Ramanaiah via Phabricator via lldb-commits
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 >

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

2018-07-10 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment. I think the m_thread_pcs.clear() in UpdateThreadIDList() is to clear out any old thread pc values that we might have populated in the past. The only way I can see this happening is if the remote stub SOMETIMES returned the thread-pcs: list in the stop packet.

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

2018-07-10 Thread Venkata Ramanaiah via Phabricator via lldb-commits
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

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

2018-07-09 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment. (just to be clear, the m_thread_pcs.clear() in ProcessGDBRemote::UpdateThreadIDList that I called a bug -- today we only have two ways of populating that, jThreadsInfo or the thread-pcs: value in the stop packet. So clearing it unconditionally here, and then

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

2018-07-09 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment. I could imagine there being a bug in this ProcessGDBRemote::UpdateThreadIDList -> ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue sequence when talking to a gdb-remote stub that does not implement the lldb-enhancement jThreadsInfo. If jThreadsInfo

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

2018-07-09 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Could you elaborate on how/why is this wrong? E.g. in which situations does it matter whether we clear the PC list? https://reviews.llvm.org/D48868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

2018-07-03 Thread Venkata Ramanaiah via Phabricator via lldb-commits
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: