[PATCH] D91605: [sanitizers] Implement GetTls on Solaris

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 19 17:20:34 PST 2020


MaskRay added inline comments.


================
Comment at: clang/tools/driver/CMakeLists.txt:123
+
+check_linker_flag("-Wl,-z,relax=transtls" LINKER_SUPPORTS_Z_RELAX_TRANSTLS)
----------------
ro wrote:
> MaskRay wrote:
> > GNU ld reports a warning instead of an error when an unknown `-z` is seen. The warning remains a warning even with `--fatal-warnings`.
> > GNU ld reports a warning instead of an error when an unknown `-z` is seen. The warning remains a warning even with `--fatal-warnings`.
> 
> Thanks for reminding me about that misfeature of GNU `ld`.  I guess `check_linker_flags` needs to be updated to handle that.
> In the case at hand, it won't matter either way: the flag is only passed to `ld`, which on Solaris is guaranteed to be the native linker.  Once (if at all) I get around to completing D85309, I can deal with that.  For now, other targets won't see linker warnings about this flag, other than when the flag is used at build time.
OK. Then I guess you can condition this when the OS is Solaris?


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp:461
+// $SRC/cmd/sgs/include/rtld.h.
+typedef struct {
+  Link_map rt_public;
----------------
In C++ you can just write `struct .... { ... }`, not need for typedef


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91605



More information about the llvm-commits mailing list