[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added inline comments.


================
Comment at: clang/lib/Basic/Sanitizers.cpp:134-143
+  if ((A &&
+       A->getOption().matches(clang::driver::options::OPT_mexecute_only)) ||
+      (std::find(Features.begin(), Features.end(), "+execute-only") !=
+       Features.end())) {
+    // The execute-only output is supported only on ARMv6T2 and ARMv7 and above.
+    if (llvm::ARM::parseArchVersion(Triple.getArchName()) > 7 ||
+        llvm::ARM::parseArch(Triple.getArchName()) ==
----------------
Why do we need to check //both// of `-mexecute-only` and the `+execute-only` target feature? It looks to me as if the code in `Driver/ToolChains/Arch/ARM.cpp` that handles `-mexecute-only` ends up setting the same target feature anyway. And it checks the supported architecture versions first.

Would it not be better to //just// check the target feature here, and avoid having a duplicated copy of the rest of this logic which needs to be kept in sync with the ARM driver?

Does something go wrong if you do it that way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158614