jingham added inline comments.

================
Comment at: lldb/test/API/functionalities/exec/TestExec.py:91-93
+        # Single step to create a thread plan. We have to make sure that after 
exec
+        # we clear all existing thread plans.
+        thread.StepInstruction(False)
----------------
wallace wrote:
> labath wrote:
> > A separate test case would be better, as (even with the comment) a random 
> > future modification of the test could negate this.
> Sure! I'll wait for @jingham to chime first
> Sure! I'll wait for @jingham to chime first

I'm a little confused about this crash.  

Before lldb lets the target proceed, it copies the thread list before 
proceeding into a temporary list.  Whenever you stop in the debugger, the first 
thing that happens is ProcessGDBRemote gets the ThreadID's from the stub and 
figures out which of those thread IDs still exist after the stop.  If the 
process plugin sees a ThreadID that it also saw before the stop, then it should 
copy the ThreadSP of the thread over from the "seen at previous stop" list to 
the current thread list.  At that point the Thread should be bona fide

Then all the ShouldStop type questions iterate over the current thread list, so 
if somebody asks the plans of the exec Thread from above which it found in the 
current thread list, ThreadPlan::GetThread will look it up by ID in the current 
thread list, and find and return that valid thread.

OTOH, if the thread ID didn't still exist, the process plugin should discard it 
as part of processing the stop, and nobody should be asking its thread plans 
questions since again you should go through the current thread list and it 
won't be on that list.  

I don't see under what circumstances you are getting a query to the thread plan 
for a bogus thread in this instance.

So I'm a little worried that just throwing away the threads when we know we've 
exec'ed is papering over some flaw lower down in the stack.  Can you give some 
more details on how exactly this is crashing, I don't have a Linux machine 
handy to try stuff out on?

Note, I'm also not at all clear what the StepInstruction in the test is for.  
At that point in the test, the target will have stopped at your first 
breakpoint in main.cpp, which is fair ways (more than one instruction) before 
the exec in main.cpp.

When you stopped before the step, the thread you stopped on will have one 
ThreadPlan on its thread plan stack: the ThreadPlanBase.  That's the plan which 
handles all the "unexpected" stops - ones that aren't governed by a thread plan 
which is actually driving the thread For instance that handles hitting an 
unrelated breakpoint while doing a step-over, etc.  It is always present, and 
can't be popped.  

Doing a thread.StepInstruction at that point pushes the "StepOverInstruction" 
thread plan, runs the process till the StepOverInstruction thread plan says it 
is done, and then pops that thread plan.  So when you stop after the 
instruction step, the thread plan stack for this thread should be the same as 
it was before the instruction step.

If this step-i is necessary to trigger the bug, then again I'm worried we're 
working around some logic goof rather than finding and fixing the actual 
problem.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93874/new/

https://reviews.llvm.org/D93874

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to