ro added a comment. In D85309#2198505 <https://reviews.llvm.org/D85309#2198505>, @MaskRay wrote:
> Please find a suitable test file (clang/test/Driver/solaris-*.c) and add some > tests there (for example, solaris-ld.c) > The challenge is that CLANG_ENABLE_GLD is not a default configuration, so > leaving out tests may be fine. On top of that, GNU `ld` may or may not be installed on a particular Solaris systems. So even if the configuration were dynamic instead of hardcoding GNU `ld`, one would still have to make the test conditional on its presence on the actual system. Same as the somewhat hacky test in D84029 <https://reviews.llvm.org/D84029>. That said, I don't consider this patch even remotely ready for inclusion. It was primarily meant to show what I'd been using to test that configuration. It has been useful in pinpointing differences between Solaris `ld` and GNU `ld` testresults. However, there are quite a number of unexpected asan failures that need to be investigated first. I'm also not happy with selecting the linker flavour at build time: maybe this can be made dynamic (e.g. parsing `ld -V` output) to allow for switching between Solaris `ld` and GNU `ld` at runtime. ================ Comment at: clang/CMakeLists.txt:281 +option(CLANG_ENABLE_GLD "Default to GNU ld." OFF) + ---------------- MaskRay wrote: > Mention SOLARIS in the variable name? Users on other OSes don't need to read > the description of this variable. > > It will also make the abbreviation `GLD` more legitimate. GLD isn't a > well-recognized abbreviation for GNU ld. As I said, I've only done it this way to allow selecting GNU `ld` at all. This would become moot if linker flavour detection were implemented at runtime. ================ Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:252 + // FIXME: Hack around /usr/gnu/bin/ld being configure with --with-sysroot. + return "/vol/gcc/bin/gld-2.35"; + //return "/usr/gnu/bin/ld"; ---------------- MaskRay wrote: > The hard-coded version doesn't sound great. Certainly not: this is a site-local path used to hack around a configuration error in the current bundled GNU `ld` on Solaris. That one is being fixed as we speak. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85309/new/ https://reviews.llvm.org/D85309 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
