[Driver] Warn when using msan on Android #122540

Sharjeel-Khan · 2025-01-10T22:21:13Z

Msan is not supported on Android as mentioned in google/sanitizers#1381. We proactively give the warning saying it is unsupported to fix android/ndk#1958.

github-actions · 2025-01-10T22:21:31Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-01-10T22:22:02Z

@llvm/pr-subscribers-clang-driver

Author: Sharjeel Khan (Sharjeel-Khan)

Changes

Msan is not supported on Android as mentioned in google/sanitizers#1381. We proactively give the warning saying it is unsupported to fix android/ndk#1958.

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

2 Files Affected:

(modified) clang/lib/Driver/SanitizerArgs.cpp (+10)
(modified) clang/test/Driver/fsanitize.c (+5)

diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index a0d6919c6dc8d0..04b39d29e7342f 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -530,6 +530,16 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
       }
       Add &= Supported;
 
+      // Msan is not supported on Android
+      if ((Add & SanitizerKind::Memory) && TC.getTriple().isAndroid()) {
+        if (DiagnoseErrors) {
+          D.Diag(diag::warn_drv_unsupported_option_for_target)
+                  << "-fsanitize=memory" << Triple.str();
+        }
+        DiagnosedKinds |= SanitizerKind::Memory;
+        Add &= ~SanitizerKind::Memory;
+      }
+
       // Test for -fno-rtti + explicit -fsanitizer=vptr before expanding groups
       // so we don't error out if -fno-rtti and -fsanitize=undefined were
       // passed.
diff --git a/clang/test/Driver/fsanitize.c b/clang/test/Driver/fsanitize.c
index 1d3caec748d77a..55b51ed29b0e65 100644
--- a/clang/test/Driver/fsanitize.c
+++ b/clang/test/Driver/fsanitize.c
@@ -399,6 +399,11 @@
 // RUN: %clang --target=arm-linux-androideabi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ANDROID-NO-ASAN
 // CHECK-ANDROID-NO-ASAN: "-mrelocation-model" "pic"
 
+// RUN: %clang --target=aarch64-linux-android -fsanitize=memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MSAN-ANDROID
+// RUN: %clang --target=i386-linux-android -fsanitize=memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MSAN-ANDROID
+// RUN: %clang --target=x86_64-linux-android -fsanitize=memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MSAN-ANDROID
+// CHECK-MSAN-ANDROID: ignoring '-fsanitize=memory' option as it is not currently supported for target
+
 // RUN: %clang --target=x86_64-linux-gnu %s -fsanitize=undefined -### 2>&1 | FileCheck %s --check-prefix=CHECK-RECOVER-UBSAN
 // RUN: %clang --target=x86_64-linux-gnu %s -fsanitize=undefined -fsanitize-recover -### 2>&1 | FileCheck %s --check-prefix=CHECK-RECOVER-UBSAN
 // RUN: %clang --target=x86_64-linux-gnu %s -fsanitize=undefined -fsanitize-recover=all -### 2>&1 | FileCheck %s --check-prefix=CHECK-RECOVER-UBSAN

llvmbot · 2025-01-10T22:22:02Z

@llvm/pr-subscribers-clang

Author: Sharjeel Khan (Sharjeel-Khan)

Changes

Msan is not supported on Android as mentioned in google/sanitizers#1381. We proactively give the warning saying it is unsupported to fix android/ndk#1958.

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

2 Files Affected:

(modified) clang/lib/Driver/SanitizerArgs.cpp (+10)
(modified) clang/test/Driver/fsanitize.c (+5)

diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index a0d6919c6dc8d0..04b39d29e7342f 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -530,6 +530,16 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
       }
       Add &= Supported;
 
+      // Msan is not supported on Android
+      if ((Add & SanitizerKind::Memory) && TC.getTriple().isAndroid()) {
+        if (DiagnoseErrors) {
+          D.Diag(diag::warn_drv_unsupported_option_for_target)
+                  << "-fsanitize=memory" << Triple.str();
+        }
+        DiagnosedKinds |= SanitizerKind::Memory;
+        Add &= ~SanitizerKind::Memory;
+      }
+
       // Test for -fno-rtti + explicit -fsanitizer=vptr before expanding groups
       // so we don't error out if -fno-rtti and -fsanitize=undefined were
       // passed.
diff --git a/clang/test/Driver/fsanitize.c b/clang/test/Driver/fsanitize.c
index 1d3caec748d77a..55b51ed29b0e65 100644
--- a/clang/test/Driver/fsanitize.c
+++ b/clang/test/Driver/fsanitize.c
@@ -399,6 +399,11 @@
 // RUN: %clang --target=arm-linux-androideabi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ANDROID-NO-ASAN
 // CHECK-ANDROID-NO-ASAN: "-mrelocation-model" "pic"
 
+// RUN: %clang --target=aarch64-linux-android -fsanitize=memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MSAN-ANDROID
+// RUN: %clang --target=i386-linux-android -fsanitize=memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MSAN-ANDROID
+// RUN: %clang --target=x86_64-linux-android -fsanitize=memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MSAN-ANDROID
+// CHECK-MSAN-ANDROID: ignoring '-fsanitize=memory' option as it is not currently supported for target
+
 // RUN: %clang --target=x86_64-linux-gnu %s -fsanitize=undefined -### 2>&1 | FileCheck %s --check-prefix=CHECK-RECOVER-UBSAN
 // RUN: %clang --target=x86_64-linux-gnu %s -fsanitize=undefined -fsanitize-recover -### 2>&1 | FileCheck %s --check-prefix=CHECK-RECOVER-UBSAN
 // RUN: %clang --target=x86_64-linux-gnu %s -fsanitize=undefined -fsanitize-recover=all -### 2>&1 | FileCheck %s --check-prefix=CHECK-RECOVER-UBSAN

Msan is not supported on Android as mentioned in google/sanitizers#1381. We proactively give the warning saying it is unsupported to fix android/ndk#1958.

github-actions · 2025-01-10T22:27:48Z

✅ With the latest revision this PR passed the C/C++ code formatter.

Sharjeel-Khan · 2025-01-11T01:42:44Z

Aditya mentioned an error is better than a warning because Mac errors

carlocab · 2025-01-11T11:28:31Z

There is already handling of unsupported sanitisers just above:

llvm-project/clang/lib/Driver/SanitizerArgs.cpp

Lines 523 to 530 in 77ef5a6

    
           if (SanitizerMask KindsToDiagnose = Add & ~Supported & ~DiagnosedKinds) { 
        
             if (DiagnoseErrors) { 
        
               std::string Desc = describeSanitizeArg(Arg, KindsToDiagnose); 
        
               D.Diag(diag::err_drv_unsupported_opt_for_target) 
        
                   << Desc << TC.getTriple().str(); 
        
             } 
        
             DiagnosedKinds |= KindsToDiagnose; 
        
           }

Is it not possible to incorporate the lack of support for msan on Android there?

Sharjeel-Khan · 2025-01-11T19:37:39Z

I can do it by moving these checks into Toolchains/Linux.cpp so it triggers that code:

llvm-project/clang/lib/Driver/ToolChains/Linux.cpp

Lines 809 to 865 in 2914ba1

    
           SanitizerMask Linux::getSupportedSanitizers() const { 
        
             const bool IsX86 = getTriple().getArch() == llvm::Triple::x86; 
        
             const bool IsX86_64 = getTriple().getArch() == llvm::Triple::x86_64; 
        
             const bool IsMIPS = getTriple().isMIPS32(); 
        
             const bool IsMIPS64 = getTriple().isMIPS64(); 
        
             const bool IsPowerPC64 = getTriple().getArch() == llvm::Triple::ppc64 || 
        
                                      getTriple().getArch() == llvm::Triple::ppc64le; 
        
             const bool IsAArch64 = getTriple().getArch() == llvm::Triple::aarch64 || 
        
                                    getTriple().getArch() == llvm::Triple::aarch64_be; 
        
             const bool IsArmArch = getTriple().getArch() == llvm::Triple::arm || 
        
                                    getTriple().getArch() == llvm::Triple::thumb || 
        
                                    getTriple().getArch() == llvm::Triple::armeb || 
        
                                    getTriple().getArch() == llvm::Triple::thumbeb; 
        
             const bool IsLoongArch64 = getTriple().getArch() == llvm::Triple::loongarch64; 
        
             const bool IsRISCV64 = getTriple().getArch() == llvm::Triple::riscv64; 
        
             const bool IsSystemZ = getTriple().getArch() == llvm::Triple::systemz; 
        
             const bool IsHexagon = getTriple().getArch() == llvm::Triple::hexagon; 
        
             SanitizerMask Res = ToolChain::getSupportedSanitizers(); 
        
             Res |= SanitizerKind::Address; 
        
             Res |= SanitizerKind::PointerCompare; 
        
             Res |= SanitizerKind::PointerSubtract; 
        
             Res |= SanitizerKind::Realtime; 
        
             Res |= SanitizerKind::Fuzzer; 
        
             Res |= SanitizerKind::FuzzerNoLink; 
        
             Res |= SanitizerKind::KernelAddress; 
        
             Res |= SanitizerKind::Memory; 
        
             Res |= SanitizerKind::Vptr; 
        
             Res |= SanitizerKind::SafeStack; 
        
             if (IsX86_64 || IsMIPS64 || IsAArch64 || IsLoongArch64) 
        
               Res |= SanitizerKind::DataFlow; 
        
             if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsArmArch || IsPowerPC64 || 
        
                 IsRISCV64 || IsSystemZ || IsHexagon || IsLoongArch64) 
        
               Res |= SanitizerKind::Leak; 
        
             if (IsX86_64 || IsMIPS64 || IsAArch64 || IsPowerPC64 || IsSystemZ || 
        
                 IsLoongArch64 || IsRISCV64) 
        
               Res |= SanitizerKind::Thread; 
        
             if (IsX86_64 || IsAArch64) 
        
               Res |= SanitizerKind::Type; 
        
             if (IsX86_64 || IsSystemZ || IsPowerPC64) 
        
               Res |= SanitizerKind::KernelMemory; 
        
             if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsMIPS || IsArmArch || 
        
                 IsPowerPC64 || IsHexagon || IsLoongArch64 || IsRISCV64) 
        
               Res |= SanitizerKind::Scudo; 
        
             if (IsX86_64 || IsAArch64 || IsRISCV64) { 
        
               Res |= SanitizerKind::HWAddress; 
        
             } 
        
             if (IsX86_64 || IsAArch64) { 
        
               Res |= SanitizerKind::KernelHWAddress; 
        
             } 
        
             if (IsX86_64) 
        
               Res |= SanitizerKind::NumericalStability; 
        
             // Work around "Cannot represent a difference across sections". 
        
             if (getTriple().getArch() == llvm::Triple::ppc64) 
        
               Res &= ~SanitizerKind::Function; 
        
             return Res; 
        
           }

Is this a good solution as there is not a specific one for Android?

thurstond · 2025-01-11T20:19:41Z

these checks into Toolchains/Linux.cpp so it triggers that code:

IMO adding the Android msan check into Toolchains/Linux.cpp would be cleaner because it's:

only a 1 or 2 line change (compared with the current ~10 lines)
this file handles all the broad compatibility checks. (SanitizerArgs.cpp uses err_drv_unsupported_opt_for_target only for niche checks.)

github-actions · 2025-01-12T09:36:18Z

@Sharjeel-Khan Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

llvm-ci · 2025-01-12T10:03:28Z

LLVM Buildbot has detected a new failure on builder clang-aarch64-sve-vls running on linaro-g3-03 while building clang at step 6 "build stage 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/143/builds/4702

Here is the relevant piece of the build log for the reference

Step 6 (build stage 1) failure: 'ninja' (failure)
...
[93/191] Linking CXX executable bin/mlir-opt
[94/191] Linking CXX executable bin/llvm-opt-fuzzer
[95/191] Linking CXX executable bin/llvm-lto
[96/191] Linking CXX executable bin/clang-nvlink-wrapper
[97/191] Linking CXX executable bin/mlir-transform-opt
[98/191] Generating ../../bin/llvm-ranlib
[99/191] Generating ../../bin/llvm-lib
[100/191] Generating ../../bin/llvm-dlltool
[101/191] Generating ../../bin/llvm-otool
[102/191] Copying llvm-locstats into /home/tcwg-buildbot/worker/clang-aarch64-sve-vls/stage1/./bin
command timed out: 1200 seconds without output running [b'ninja'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1408.493035

Msan is not supported on Android as mentioned in google/sanitizers#1381. We proactively give the warning saying it is unsupported to fix android/ndk#1958.

llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Jan 10, 2025

Warn when using msan on Android

c8d2c21

Msan is not supported on Android as mentioned in google/sanitizers#1381. We proactively give the warning saying it is unsupported to fix android/ndk#1958.

Sharjeel-Khan force-pushed the msan_android branch from 4a1f619 to c8d2c21 Compare January 10, 2025 22:24

Fixed format for SanitizerArgs

035c7d9

Sharjeel-Khan changed the title ~~Warn when using msan on Android~~ [Driver] Warn when using msan on Android Jan 11, 2025

carlocab requested review from vitalybuka, thurstond and kstoimenov January 11, 2025 11:29

Move the check into Linux.cpp

3e42b05

thurstond approved these changes Jan 12, 2025

View reviewed changes

carlocab approved these changes Jan 12, 2025

View reviewed changes

carlocab merged commit fdfe7e7 into llvm:main Jan 12, 2025
8 checks passed

Sharjeel-Khan deleted the msan_android branch January 14, 2025 21:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Driver] Warn when using msan on Android #122540

[Driver] Warn when using msan on Android #122540

Sharjeel-Khan commented Jan 10, 2025

github-actions bot commented Jan 10, 2025

llvmbot commented Jan 10, 2025

llvmbot commented Jan 10, 2025

github-actions bot commented Jan 10, 2025 •

edited

Loading

Sharjeel-Khan commented Jan 11, 2025

carlocab commented Jan 11, 2025

Sharjeel-Khan commented Jan 11, 2025 •

edited

Loading

thurstond commented Jan 11, 2025

github-actions bot commented Jan 12, 2025

llvm-ci commented Jan 12, 2025

[Driver] Warn when using msan on Android #122540

[Driver] Warn when using msan on Android #122540

Conversation

Sharjeel-Khan commented Jan 10, 2025

github-actions bot commented Jan 10, 2025

llvmbot commented Jan 10, 2025

llvmbot commented Jan 10, 2025

github-actions bot commented Jan 10, 2025 • edited Loading

Sharjeel-Khan commented Jan 11, 2025

carlocab commented Jan 11, 2025

Sharjeel-Khan commented Jan 11, 2025 • edited Loading

thurstond commented Jan 11, 2025

github-actions bot commented Jan 12, 2025

llvm-ci commented Jan 12, 2025

github-actions bot commented Jan 10, 2025 •

edited

Loading

Sharjeel-Khan commented Jan 11, 2025 •

edited

Loading