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 \ ---------------- alvinhochun wrote: > 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? Yes, I tested the form I quoted above, and it works with both msvc and mingw based setups. 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
