[Lldb-commits] [PATCH] D90729: [trace][intel-pt] Scaffold the 'thread trace start | stop' commands
Greg Clayton via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Wed Nov 11 15:14:04 PST 2020
clayborg added inline comments.
Comment at: lldb/include/lldb/Interpreter/CommandObjectDelegate.h:30-32
+ virtual lldb::CommandObjectSP DoGetDelegateCommand() = 0;
+ CommandObject *GetDelegateCommand();
So currently you are storing the command object into m_delegate_command_sp in GetDelegateCommand(). This can cause an issue when you have two debuggers (Xcode creates a new debugger for each debug window). If thread A calls this function, and fills in m_delegate_command_sp, then thread B fills in m_delegate_command_sp from another command interpreter, we could end up using the one from thread B. Since the only reason we are storing the value in m_delegate_command_sp is to keep the shared pointer alive, we should just return the shared pointer. So I would suggest removing DoGetDelegateCommand() and just have:
virtual llvm::Expected<lldb::CommandObjectSP> GetDelegateCommand() = 0;
Don't store it. Each function that calls it will keep it around as long as needed. If we use expected then we don't need to store the m_delegate_error later...
Comment at: lldb/source/Interpreter/CommandObjectDelegate.cpp:16-19
+ m_delegate_command_sp = DoGetDelegateCommand();
+ if (m_delegate_command_sp)
+ return m_delegate_command_sp.get();
+ return nullptr;
This caching isn't really doing anything. If we have two debuggers, each one has its own command interpreter and this can actually cause things to fail. If thread A calls this, then thread B, then thread A continues to use the one populated by thread B. See inline comment in header file.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the lldb-commits