[PATCH] D101335: [ASan][Darwin] Fix `TestCases/Darwin/init_for_dlopen.cpp` when running in a standalone build.

[Dan Liew via Phabricator via llvm-commits]

delcypher added a comment.

In D101335#2720966 <https://reviews.llvm.org/D101335#2720966>, @yln wrote:

> Would it be possible (and wouldn't it be better?) to set `config.compiler_rt_libdir` to the right path in the first place?   This way we wouldn't get into the "path mismatch" state that we warn about.

I did look into this. I didn't go down that route because it's problematic.

> I can see that `compiler_rt_libdir` is used in places (other than computing `shared_libasan_path`) as well.
>
> First set by cmake `compiler-rt/cmake/Modules/AddCompilerRT.cmake`.  Ideally, we would pick the right path there, right?
>
>   # Configure lit configuration files, including compiler-rt specific variables.
>   function(configure_compiler_rt_lit_site_cfg input output)
>     set_llvm_build_mode()
>   
>     get_compiler_rt_output_dir(${COMPILER_RT_DEFAULT_TARGET_ARCH} output_dir)
>   
>     string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} COMPILER_RT_RESOLVED_TEST_COMPILER ${COMPILER_RT_TEST_COMPILER})
>     string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} COMPILER_RT_RESOLVED_LIBRARY_OUTPUT_DIR ${output_dir})
>   
>     configure_lit_site_cfg(${input} ${output})
>   endfunction()

So making the change at the CMake level has two problems.

1. If you're testing an externally built toolchain with a standalone compiler-rt configure (what I'm doing) then you risk accidentally borking the toolchain if you invoke a build. If `COMPILER_RT_RESOLVED_LIBRARY_OUTPUT_DIR` points into the externally built toolchain's resource directory then building the Sanitizers would overwrite the libraries in the toolchain. That doesn't seem desirable. What happens right now if that you can do a build of the Sanitizer runtimes and they just end up in the build directory but won't actually used by testing. This isn't great either but it at least it isn't modifying an external toolchain.

2. Changing this part of the CMake is incredibly risky because the way this code works differs between Apple and non-Apple platforms. Given that there's a much more targeted and lower risk option (i.e. this patch) I went for that instead.

> There also seems to be another config hook in `compiler-rt/unittests/lit.common.unit.configured.in`:
>
>   # LLVM tools dir and build mode can be passed in lit parameters,
>   # so try to apply substitution.
>   try:
>     config.llvm_tools_dir = config.llvm_tools_dir % lit_config.params
>     config.compiler_rt_libdir = config.compiler_rt_libdir % lit_config.params
>     config.llvm_build_mode = config.llvm_build_mode % lit_config.params
>   except KeyError as e:
>     key, = e.args
>     lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % (key, key))
>
> Can we use or change one of these existing mechanisms?

I don't think we can use that mechanism. Looking at it more I don't know how anyone is supposed to use that. In order for this to work the strings like `config.llvm_tools_dir` would need to contain python string interpolation keys (e.g. `%(some_key_name)s`). These strings don't contain this so I'm not sure how that code ever worked.

Thinking about this more a more general solution would be to modify `config.compiler_rt_libdir` from `test/lit.common.cfg.py` if we are doing a standalone build and we are building using clang. We might still run into problems here though because some people might expect the tests to use libraries built as part of the standalone compiler-rt build (the builtins tests expect this) rather than the libraries in the external toolchain's resource directory (the sanitizer tests expect this).

So unfortunately going down this path is going to be messy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101335