[Lldb-commits] [PATCH] D109797: Fix rendezvous for rebase_exec=true case

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

lgtm, modulo comments.



================
Comment at: lldb/test/API/functionalities/dlopen/TestDlopen.py:1
+import lldb
+from lldbsuite.test.decorators import *
----------------
I'd put this under `functionalities/load_after_attach/TestLoadAfterAttach.py` to better fit into the existing naming scheme, and to emphasize the attach aspect of the test (as that is what really makes this test special).


================
Comment at: lldb/test/API/functionalities/dlopen/b.cpp:1-4
+
+
+int LLDB_DYLIB_EXPORT b_function() { return 500; }
+
----------------
What's up with the whitespace?


================
Comment at: lldb/test/API/functionalities/dlopen/main.cpp:11-17
+void sleepFor(int milliseconds) {
+ #if _WIN32
+    Sleep(milliseconds);
+#else
+    usleep(milliseconds*1000);
+#endif
+}
----------------
`std::this_thread::sleep_for`


================
Comment at: lldb/test/API/functionalities/dlopen/main.cpp:26
+  // dlopen the 'liblib_b.so' shared library.
+  void* h = dlopen(solib, RTLD_LAZY);
+  assert(h && "dlopen failed?");
----------------
emrekultursay wrote:
> labath wrote:
> > see dylib.h and the functions within (the inferior of TestLoadUnload uses them) for a windows-compatible way to load shared libraries.
> Since we are attaching to an already running process, which cannot find the dynamic library unless I pass the full path to `dlopen()`. That's why I couldn't use dylib.h (which doesn't add full path), but created my own version here.
Ok, what I think you're saying is that when we run a process for attaching, we skip the code paths which set ((DY)LD_LIBRARY_)PATH, which is what makes the relative imports work. It shouldn't be too hard to extend the launch infrastructure to do that. Let's commit this in this form, and I'll do that as a follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109797