[Lldb-commits] [PATCH] D75877: [lldb/Reproducers] Fix replay for process attach workflows

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

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...

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?



================
Comment at: lldb/test/Shell/Reproducer/Inputs/sleep.c:5
+int main(int argc, char const *argv[]) {
+  sleep(10);
+  return 0;
----------------
For attach to work reliably on linux, you need to ensure the process declares itself willing to be attached to. This is what the `lldb_enable_attach` macro in dotest inferiors does. Then you also need to ensure that the process has executed that statement before you attempt to attach. This is usually done via some pid file synchronization.


================
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 &")'
+
----------------
How is this different from a plain `%t/attach.out &` ?


================
Comment at: lldb/test/Shell/Reproducer/TestAttach.test:9
+
+# RUN: %lldb --capture --capture-path %t.repro -o 'pro att -n attach.out' -o 'reproducer generate' | FileCheck %s
+# RUN: %lldb --replay %t.repro | FileCheck %s
----------------
Though normally determinism is good, in this case I think it is actually better to generate an unpredictable name for the process to avoid having the test be impacted by parallel test suite runs or leftover zombies. Other attach-by-name tests usually embed some a pid or a timestamp into the process name.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D75877