[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 \
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134581