[libcxx-commits] [PATCH] D100221: [libcxx] [test] Add a separate 'windows-dll' feature to check for

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
Comment at: libcxx/utils/libcxx/test/features.py:153
   Feature(name='windows', when=lambda cfg: '_WIN32' in compilerMacros(cfg)),
+  Feature(name='windows-dll', when=lambda cfg: '_WIN32' in compilerMacros(cfg) and cfg.enable_shared),
   Feature(name='linux', when=lambda cfg: '__linux__' in compilerMacros(cfg)),
----------------
ldionne wrote:
> mstorsjo wrote:
> > ldionne wrote:
> > > We don't want to use `.enable_shared` here. `.enable_shared` is part of the old config which I'm trying to get rid of one step at a time.
> > > 
> > > Is there a way to implicitly detect that we're linking against the DLL without being passed that information through `cfg.enable_shared`?
> > > 
> > > Alternatively, something we could explore is add a feature that tells us whether the library is linked statically or dynamically (regardless of the platform). Your feature check would then become `windows && libcxx-linked-dynamically`.
> > > Is there a way to implicitly detect that we're linking against the DLL without being passed that information through cfg.enable_shared?
> > 
> > We could source the headers and see if `_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS` is defined or not; it's a bit less direct but should be reliable in practice. (The fact that that define indicates static/shared mode is a windows specific bit though.)
> > 
> > > Alternatively, something we could explore is add a feature that tells us whether the library is linked statically or dynamically (regardless of the platform). Your feature check would then become windows && libcxx-linked-dynamically.
> > 
> > I guess that would work - but that boils down to the original issue - I'm not allowed to use the "old config", but I'm not quite familiar enough with the separation between old and new here, and what the actually allowed way of passing info to the new test configure system is.
> > 
> > I presume we're able to introspect the build env by looking for defines and such, but e.g. for non-windows configurations, the fact whether we're building isn't visible in headers. How would you suggest we find the information in that case, and pass it from the parts of the system where we know for sure (where we have the `enable_shared` variable today)?
> Anything in `libcxx/test/configs/legacy.cfg.in` is the old config, and so is `libcxx/utils/libcxx/test/config.py`. I'm trying to move stuff from `libcxx/utils/libcxx/test/config.py` into `features.py` and `params.py`, with the goal of replacing `legacy.cfg.in` by configs like `libcxx/test/configs/libcxx-trunk-shared.cfg.in`.
> 
> One thing we could do is build a dummy program referencing something from `std::string` (for example) and see whether it depends on any externally defined symbol from libc++. That would satisfy the request for this feature to be automatically detected.
So overall, I agree that the introspection approach is good in the sense that it puts testing the newly built libc++ on equal grounds with testing a third party C++ library... 

When testing a third party library, one would still need to manually set up the equivalent of libcxx-trunk-shared.cfg.in for ones setup right? And there are still going to be a small set of variables that are configured by the build system (for the one set up automatically when building libc++) and manually for third party library testing, for things that are impossible/impractical to introspect. (Not insisting that this variable is such one, just trying to get a grasp of the big picture.) And one important tradeoff is that the introspection setup doesn't do too many expensive tests, as they are run at the start each time when running one or more tests. (I.e. the tests for defines are quick as they rely on the output of `$CC -E -dM` that is just run once, but doing a large number of compile+link tests might end up impractical.)

Testing linking and looking at the result might work, but it sounds a bit cumbersome, when working across multiple platforms.

For darwin, one can use something like `otool -L`, for ELF maybe `readelf -d` and for PE-COFF maybe `[llvm-]objdump -p` (which might not necessarily be available in all setups) to get a list of the shared libs that are loaded by an executable. We can't look for literally [lib]c++.[dll|so|dylib] because a third party tested lib might be named something entirely different - so we'd need to look for specific symbol names. `otool` and `readelf` don't print them, but one could use e.g. `nm -D`, and `objdump -p` prints imported symbols. Then for windows, there's two different C++ symbol mangling schemes, but maybe one would look for e.g. the substring `string` in the symbols, as I think that could be found in both mangling schemes.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100221