jingham added a comment.

I don't think CreateProcess needs a HijackListener, does it?  It doesn't 
generate events, it just make the connection to the server.  It is always 
followed by Launch or DebugProcess, which are the ones that do the real work.



================
Comment at: lldb/include/lldb/Target/Target.h:587
 
   // If listener_sp is null, the listener of the owning Debugger object will be
   // used.
----------------
Probably worth saying what the hijack_listener is for as well.


================
Comment at: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py:14
         '''Create the Visual Studio Code debug adaptor'''
-        self.assertTrue(os.path.exists(self.lldbVSCodeExec),
-                        'lldb-vscode must exist')
+        self.assertTrue(is_exe(self.lldbVSCodeExec), 'lldb-vscode must exist')
         log_file_path = self.getBuildArtifact('vscode.txt')
----------------
I don't know if it's worth working harder for this, but it would be confusing 
if lldb-vscode existed but wasn't executable, and we gave an error of 
"lldb-vscode must exist". Maybe just say "must exist and be executable"...


================
Comment at: lldb/source/Target/Target.cpp:3038
+  // manually, without the platform).
+  if (synchronous_execution && !launch_info.GetHijackListener())
+    launch_info.SetHijackListener(
----------------
In this context sync vrs. async is really about whether the caller expects to 
get a stop event for the first "real user stop", or just have Launch return 
when we are at the first user event w/o the caller having to spin the event 
loop.  A real user stop is generally "process hit a breakpoint" or "process 
crashed", etc.

That means we should hijack the events up to the stop at the entry point in 
both sync and async modes.  The odd case is when "stop at entry" is set.  In 
that case the "launch success" stop is also the first "real user stop". So in 
the "stop at entry && async" mode we need to arrange for there to be a stop 
event that the caller can wait on.  

The cleanest way to do this would be to always hijack the events, and then in 
the case where stop at entry is set in async mode, rebroadcast the stop event 
to the regular listener.  You could have the DebugProcess & Launch return the 
last event SP for this purpose.

The way the old code did it was to just undo the hijack, and return w/o calling 
WaitForProcessToStop (line 3084 & following) under the assumption that this 
would leave the last stop event unfetched and available for the caller.  But 
I'm not sure that's guaranteed to work.


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

https://reviews.llvm.org/D119548

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

Reply via email to