[all-commits] [llvm/llvm-project] 41d0b2: [lldb] Avoid moving ThreadPlanSP from plans vector

[Dave Lee via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 41d0b20cc90f2aea25b4306f6c8f6c258ca3d377
      https://github.com/llvm/llvm-project/commit/41d0b20cc90f2aea25b4306f6c8f6c258ca3d377
  Author: Dave Lee <davelee.com at gmail.com>
  Date:   2021-08-01 (Sun, 01 Aug 2021)

  Changed paths:
    M lldb/include/lldb/Target/ThreadPlan.h
    M lldb/include/lldb/Target/ThreadPlanCallFunction.h
    M lldb/include/lldb/Target/ThreadPlanCallUserExpression.h
    M lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h
    M lldb/source/Target/ThreadPlan.cpp
    M lldb/source/Target/ThreadPlanCallFunction.cpp
    M lldb/source/Target/ThreadPlanCallUserExpression.cpp
    M lldb/source/Target/ThreadPlanStack.cpp
    M lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp

  Log Message:
  -----------
  [lldb] Avoid moving ThreadPlanSP from plans vector

Change `ThreadPlanStack::PopPlan` and `::DiscardPlan` to not do the following:

1. Move the last plan, leaving a moved `ThreadPlanSP` in the plans vector
2. Operate on the last plan
3. Pop the last plan off the plans vector

This leaves a period of time where the last element in the plans vector has been moved. I am not sure what, if any, guarantees there are when doing this, but it seems like it would/could leave a null `ThreadPlanSP` in the container. There are asserts in place to prevent empty/null `ThreadPlanSP` instances from being pushed on to the stack, and so this could break that invariant during multithreaded access to the thread plan stack.

An open question is whether this use of `std::move` was the result of a measure performance problem.

Differential Revision: https://reviews.llvm.org/D106171