labath added a comment. In D75877#1914755 <https://reviews.llvm.org/D75877#1914755>, @JDevlieghere wrote:
> In D75877#1913959 <https://reviews.llvm.org/D75877#1913959>, @labath wrote: > > > A more principled way to make this work would be to intercept (record) the > > Host::FindProcesses api. That way other functionality pertaining to running > > processes (e.g. the "platform process list" command) would also work. But > > if this is all you care about right now, then maybe this is fine... > > > We could totally add a provider for that. I didn't because it seemed like > overkill but if you're on board I also prefer that over a random PID. In general, I am in favor of doing the capture at the lowest level possible. For this particular feature/bug, it is overkill, but OTOH, this will also make it possible to support things other things without adding hacks into random pieces of code. > > >> The part that worries me more is the test. There are a lot of subtleties >> involved in making attach tests (and attach-by-name tests in particular) >> work reliably everywhere. I think this should be a dotest test, as there we >> already have some machinery to do these kinds of things >> (`lldb_enable_attach`, `wait_for_file_on_target`), and python is generally >> much better at complex control flow (I am very much against subshells and >> background processes in lit tests). Reproducers make this somewhat >> complicated because you cannot use the liblldb instance already loaded into >> the python process. But maybe you could run lldb in a subprocess similar to >> how the pexpect tests do it? > > The problem is the `SBDebugger::Initialize()` that's called from the SWIG > bindings, as soon as you import lldb it's already too late for the > reproducers. I'm working on being able to capture/replay the test suite and > currently I have the following hack: > > if 'DOTEST_CAPTURE_PATH' in os.environ: > SBReproducer.Capture(os.environ['DOTEST_CAPTURE_PATH']) > SBDebugger.Initialize() > > > If you're fine with having that in `python.swig` unconditionally we could > make a dotest-test work. I'm not sure how that would help because for the test you need to run lldb both in capture and replay mode, and I don't think you can currently do that within a single process. It would be cool if that was possible, but even then we'd have the impendance mismatch because we'd need to run `SBDebugger.Initialize` inside a specific test method, whereas normally it gets run much earlier. That's why I was talking about subprocesses in the previous patch. The test would only be responsible for building the inferior and driving the whole thing, while capture/replay would happen inside separate processes: self.spawnSubproces(randomized_inferior_name, [token_path]) lldbutil.wait_for_file_on_target(token_path) self.spawnSubprocess(lldbtest_config.lldbExec, ['--capture', reproducer, '-n', randomized_inferior_name, ...]) ... self.spawnSubprocess(lldbtest_config.lldbExec, ['--replay', reproducer]) ================ Comment at: lldb/test/Shell/Reproducer/TestAttach.test:7 +# RUN: %clang_host %S/Inputs/sleep.c -g -o %t/attach.out +# RUN: python -c 'import os; os.system("%t/attach.out &")' + ---------------- JDevlieghere wrote: > labath wrote: > > How is this different from a plain `%t/attach.out &` ? > Should that work? That's the first thing I tried and lit complained about > invalid syntax. I've seen tests do that (`RUN: setsid %run %t/LFSIGUSR -merge=1 -merge_control_file=%t/MCF %t/C1 %t/C2 2>%t/log & export PID=$!` in `./compiler-rt/test/fuzzer/merge-sigusr.test`), but as I said, I don't think that is a good idea, so I don't really want to encourange it... Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75877/new/ https://reviews.llvm.org/D75877 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
