[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



More information about the lldb-commits mailing list