labath added a comment.
I have two high-level questions about this:
- what should be the relative priority of executable ModuleSP vs the launch
info? IIUC, in the current implementation the module takes precedence, but it
seems to me it would be more natural if the launch info came out on top. One of
the reasons I'm thinking about this is because you currently cannot re-run
executables that use exec(2) (I mean, you can, but it will rarely produce the
desired results). This happens because we use the post-exec executable, but the
rest of the launch info refers to the main one. If the executable module was
not the primary source of truth here, then maybe the we could somehow use this
mechanism to improve the exec story as well (by storing the original executable
in the launch info or something).
- what happens if the launch_info path happens to refer to a host executable?
Will we still launch the remote one or will we try to copy the local one
instead? I'm not sure of the current behavior, but I would like to avoid the
behavior depending on the presence of local files.
================
Comment at: lldb/source/Commands/CommandObjectProcess.cpp:269
+ if (!exe_module_sp) {
+ result.AppendError("Could not get executable module after launch.");
+ return false;
----------------
This probably shouldn't be a hard error. If we fail to obtain the executable
module, I'd think we should still be able to continue with asm debugging. (In
fact, since you don't clean up the process here, that's probably what will
happen regardless of the error state.)
================
Comment at:
lldb/test/API/functionalities/gdb_remote_client/TestNoLocalFile.py:30
+ ascii = bytearray.fromhex(a_arr[2]).decode()
+ self.testcase.assertEqual(ascii, self.testcase.absent_file,
"We sent the right path")
+ return "OK"
----------------
the responder will run on a separate thread, so a failing assertion will not
immediately terminate the test, but rather confuse the hell out of everyone. It
be better to just record the data here, and do the assertion on the main thread.
================
Comment at:
lldb/test/API/functionalities/gdb_remote_client/TestNoLocalFile.py:69
+ self.server.responder = MyResponder(self)
+ target = self.dbg.CreateTargetWithFileAndArch(None, "x86_64")
+ launch_info = target.GetLaunchInfo()
----------------
I would use the CreateTarget overload which takes a platform name argument.
Otherwise, this will pick a random platform on non-darwin hosts and then it may
get confused by the darwin-specific responses.
================
Comment at:
lldb/test/API/functionalities/gdb_remote_client/TestNoLocalFile.py:79-83
+ # We need to fetch the connected event:
+ event = lldb.SBEvent()
+ got_event = self.dbg.GetListener().WaitForEventForBroadcaster(30,
process.GetBroadcaster(), event)
+ self.assertTrue(got_event, "Got an initial event after connect")
+ self.assertEqual(process.GetState(), lldb.eStateConnected, "Unexpected
state.")
----------------
we have `lldbutil.expect_state_changes` for this
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113521/new/
https://reviews.llvm.org/D113521
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
