[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 \
----------------
mstorsjo wrote:
> 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.
I forgot to mention; see https://reviews.llvm.org/D129455 for some earlier discussion about other aspects (symbol table vs dwarf vs PDB) for windows testcases in lldb.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134581