Skip to content

[Clang][Sanitizers] Enable NSAN on X86_64 only #95885

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

alexander-shaposhnikov
Copy link
Collaborator

@alexander-shaposhnikov alexander-shaposhnikov commented Jun 18, 2024

This is a follow-up to #93783.
The current set of patches covers only x86_64,
therefore we should not enable this flag on arm64 yet.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Jun 18, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 18, 2024

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Alexander Shaposhnikov (alexander-shaposhnikov)

Changes

This is a follow-up to #93783.
The current set of patches covers only x86_64 (in particular, on compiler-rt's side),
therefore we should now enable this flag on arm64.


Full diff: https://github.com/llvm/llvm-project/pull/95885.diff

3 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Darwin.cpp (+3-1)
  • (modified) clang/lib/Driver/ToolChains/Linux.cpp (+1-1)
  • (modified) clang/test/Driver/fsanitize.c (+4-4)
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index f742db7668cd2..0ae356ca91b22 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -3457,7 +3457,6 @@ SanitizerMask Darwin::getSupportedSanitizers() const {
   Res |= SanitizerKind::PointerCompare;
   Res |= SanitizerKind::PointerSubtract;
   Res |= SanitizerKind::Leak;
-  Res |= SanitizerKind::NumericalStability;
   Res |= SanitizerKind::Fuzzer;
   Res |= SanitizerKind::FuzzerNoLink;
   Res |= SanitizerKind::ObjCCast;
@@ -3474,6 +3473,9 @@ SanitizerMask Darwin::getSupportedSanitizers() const {
        isTargetTvOSSimulator() || isTargetWatchOSSimulator())) {
     Res |= SanitizerKind::Thread;
   }
+
+  if (IsX86_64)
+    Res |= SanitizerKind::NumericalStability;
   return Res;
 }
 
diff --git a/clang/lib/Driver/ToolChains/Linux.cpp b/clang/lib/Driver/ToolChains/Linux.cpp
index 2222dea431c3c..49e029e7c9ab7 100644
--- a/clang/lib/Driver/ToolChains/Linux.cpp
+++ b/clang/lib/Driver/ToolChains/Linux.cpp
@@ -826,7 +826,7 @@ SanitizerMask Linux::getSupportedSanitizers() const {
   if (IsX86_64 || IsAArch64) {
     Res |= SanitizerKind::KernelHWAddress;
   }
-  if (IsX86_64 || IsAArch64)
+  if (IsX86_64)
     Res |= SanitizerKind::NumericalStability;
 
   // Work around "Cannot represent a difference across sections".
diff --git a/clang/test/Driver/fsanitize.c b/clang/test/Driver/fsanitize.c
index 08e9c78f9d1d2..db14f6e195c64 100644
--- a/clang/test/Driver/fsanitize.c
+++ b/clang/test/Driver/fsanitize.c
@@ -462,8 +462,8 @@
 // RUN: %clang --target=x86_64-linux-gnu -fsanitize=numerical %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NSAN-X86-64-LINUX
 // CHECK-NSAN-X86-64-LINUX: "-fsanitize=numerical"
 
-// RUN: %clang --target=aarch64-unknown-linux-gnu -fsanitize=numerical %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NSAN-AARCH64-LINUX
-// CHECK-NSAN-AARCH64-LINUX: "-fsanitize=numerical"
+// RUN: not %clang --target=aarch64-unknown-linux-gnu -fsanitize=numerical %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NSAN-AARCH64-LINUX
+// CHECK-NSAN-AARCH64-LINUX: error: unsupported option '-fsanitize=numerical' for target 'aarch64-unknown-linux-gnu'
 
 // RUN: not %clang --target=mips-unknown-linux -fsanitize=numerical %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NSAN-MIPS-LINUX
 // CHECK-NSAN-MIPS-LINUX: error: unsupported option '-fsanitize=numerical' for target 'mips-unknown-linux'
@@ -471,8 +471,8 @@
 // RUN: %clang --target=x86_64-apple-macos -fsanitize=numerical %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NSAN-X86-64-MACOS
 // CHECK-NSAN-X86-64-MACOS: "-fsanitize=numerical"
 
-// RUN: %clang --target=arm64-apple-macos -fsanitize=numerical %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NSAN-ARM64-MACOS
-// CHECK-NSAN-ARM64-MACOS: "-fsanitize=numerical"
+// RUN: not %clang --target=arm64-apple-macos -fsanitize=numerical %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NSAN-ARM64-MACOS
+// CHECK-NSAN-ARM64-MACOS: error: unsupported option '-fsanitize=numerical' for target 'arm64-apple-macos'
 
 // RUN: %clang --target=x86_64-apple-darwin -fsanitize=thread %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-X86-64-DARWIN
 // CHECK-TSAN-X86-64-DARWIN-NOT: unsupported option

@MaskRay
Copy link
Member

MaskRay commented Jun 18, 2024

If the code is generic, it might already work on aarch64... It seems sufficient to avoid compiler-rt runtime testing.

@alexander-shaposhnikov
Copy link
Collaborator Author

~90% of the functionality would work on aarch64, but there are a few x86_64-specific quirks (__float128, x86-64-specific assumptions about long double in the helper routines) that need to be refactored (beyond my current bandwidth).

@MaskRay
Copy link
Member

MaskRay commented Jun 18, 2024

~90% of the functionality would work on aarch64, but there are a few x86_64-specific quirks (__float128, x86-64-specific assumptions about long double in the helper routines) that need to be refactored (beyond my current bandwidth).

I am happy to investigate them. I guess we don't need to exclude driver now? xray compiler instrumentation works for Mach-O but the runtime is not yet, we don't disable the driver for Mach-O

@alexander-shaposhnikov
Copy link
Collaborator Author

yeah, but I kind of wanted to be defensive - people will try the tool and I wanted to avoid any potential confusions / overpromises.

@alexander-shaposhnikov alexander-shaposhnikov merged commit c638ba1 into llvm:main Jun 18, 2024
7 checks passed
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
This is a follow-up to llvm#93783.
The current set of patches covers only x86_64,
therefore we should not enable this flag on arm64 yet.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants