[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added inline comments.


================
Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:7
+# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.dll -o %t.main.exe
+# RUN: %lldb -b -o "#before" -o "target modules list" -o "b main" -o run \
----------------
Unfortunately, this aspect of the test doesn't work as is on Windows.

By default (in msvc builds), clang invokes link.exe here, but link.exe can't take `%t.shlib.dll` as input file to the linker, as it requires an import library.

If we would assume that lld is available, we could run the linking command with `-fuse-ld=lld` - however that on its own isn't enough, as lld only allows linking against DLLs when run with the `-lldmingw` flag. We can add `-fuse-ld=lld -Wl,-lldmingw` here to fix that, but that would break testing in mingw environments, as `-lldmingw` isn't a valid option on the mingw lld driver.

Likewise, we could create a proper import library by adding `-Wl,-implib:%t.shlib.lib` on the first command above, but that doesn't work in mingw mode either, as it would have to be `-Wl,--out-implib=%t.shlib.lib` instead.

In practice, running the lldb tests in mingw mode is not entirely supported, while they do pass cleanly in MSVC mode (and there's a buildbot testing this configuration) - but I would like to work towards making things work better in the mingw configuration too.

There's a different substitution, `%build`, which invokes the `lldb/test/Shell/helpers/build.py` script, which abstracts a bunch of boilerplate details mostly relevant for windows targets (like creating PDB debug info files); the easiest at this point would probably be to extend that script with options for creating import libraries.

For testing with mingw, I'm currently using this out of tree patch for that script too:
```
diff --git a/lldb/test/Shell/helper/build.py b/lldb/test/Shell/helper/build.py
index 96684b7b3e66..f138b00bee9e 100755
--- a/lldb/test/Shell/helper/build.py
+++ b/lldb/test/Shell/helper/build.py
@@ -191,7 +191,10 @@ def find_toolchain(compiler, tools_dir):
     if compiler == 'any':
         priorities = []
         if sys.platform == 'win32':
-            priorities = ['clang-cl', 'msvc', 'clang', 'gcc']
+            if 'gcc' in sys.version.lower():
+                priorities = ['clang', 'gcc', 'clang-cl', 'msvc']
+            else:
+                priorities = ['clang-cl', 'msvc', 'clang', 'gcc']
         else:
             priorities = ['clang', 'gcc', 'clang-cl']
         for toolchain in priorities:
```
(This is a bit hacky, since it uses the build type of the python interpreter to switch the preference between clang-cl and clang.)

Alternatively, we could maybe add a separate substitution that expands into either `-implib:` or `--out-implib=`, but I'm not sure if we can guess which one will be the right one either, while the build.py helper script probably can do better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134581