alvinhochun 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: > alvinhochun wrote: > > mstorsjo wrote: > > > alvinhochun wrote: > > > > mstorsjo wrote: > > > > > 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. > > > > Re different options on msvc vs mingw, how about using the `%if` > > > > substitution (https://www.llvm.org/docs/TestingGuide.html)? We can use > > > > it to check for the `windows-msvc` feature to apply options > > > > conditionally. > > > Oh, I guess that would work too - it does litter the individual tests > > > with the conditions instead of hiding it in `build.py`, but probably is > > > acceptable too. > > The commands should become this: > > > > ``` > > # RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll %if > > windows-msvc %{-Wl,-implib:%t.shlib.lib} > > # RUN: %clang_host -g0 -O0 %S/Inputs/main.c -o %t.main.exe %if windows-msvc > > %{%t.shlib.lib} %else %{%t.shlib.dll} > > ``` > This form works, and keeps it to just one conditional: > ``` > # RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll %if > windows-msvc %{-Wl,-implib:%t.shlib.lib%} %else > %{-Wl,--out-implib=%t.shlib.lib%} > # RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.lib -o %t.main.exe > ``` Oh yeah, this does feel nicer. Have you already tested it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134581/new/ https://reviews.llvm.org/D134581 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
