Skip to content
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

[Driver] Enable ASan on Solaris/SPARC #107403

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

rorth
Copy link
Collaborator

@rorth rorth commented Sep 5, 2024

Once PR #107223 lands, ASan can be enabled on Solaris/SPARC. This patch does just that. As on Solaris/x86, the dynamic ASan runtime lib needs to be linked with -z now to avoid an AsanInitInternal cycle.

Tested on sparcv9-sun-solaris2.11 and sparc64-unknown-linux-gnu.

Once PR#107223 lands, ASan can be enabled on Solaris/SPARC.  This patch
does just that.  As on Solaris/x86, the dynamic ASan runtime lib needs to
be linked with `-z now` to avoid an `AsanInitInternal` cycle.

Tested on `sparcv9-sun-solaris2.11` and `sparc64-unknown-linux-gnu`.

This is not yet ready to be committed, though: even with the corresponding
`compiler-rt` patch, 36 tests still `FAIL`.

For Linux/sparc64, nothing needs to be done since `clang` enables ASan by
default, irrespective of target.  On top of that, test results are way
worse, with quite a number of tests hanging completely.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Sep 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 5, 2024

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Rainer Orth (rorth)

Changes

Once PR #107223 lands, ASan can be enabled on Solaris/SPARC. This patch does just that. As on Solaris/x86, the dynamic ASan runtime lib needs to be linked with -z now to avoid an AsanInitInternal cycle.

Tested on sparcv9-sun-solaris2.11 and sparc64-unknown-linux-gnu.

This is not yet ready to be committed, though: even with the corresponding compiler-rt patch, 36 tests still FAIL.

For Linux/sparc64, nothing needs to be done since clang enables ASan by default, irrespective of target. On top of that, test results are way worse, with quite a number of tests hanging completely.


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

1 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Solaris.cpp (+4-4)
diff --git a/clang/lib/Driver/ToolChains/Solaris.cpp b/clang/lib/Driver/ToolChains/Solaris.cpp
index e82ed2ca79ffd6..8d4ab40ce9f1de 100644
--- a/clang/lib/Driver/ToolChains/Solaris.cpp
+++ b/clang/lib/Driver/ToolChains/Solaris.cpp
@@ -265,8 +265,7 @@ void solaris::Linker::ConstructJob(Compilation &C, const JobAction &JA,
       }
     }
     // Avoid AsanInitInternal cycle, Issue #64126.
-    if (ToolChain.getTriple().isX86() && SA.needsSharedRt() &&
-        SA.needsAsanRt()) {
+    if (SA.needsSharedRt() && SA.needsAsanRt()) {
       CmdArgs.push_back("-z");
       CmdArgs.push_back("now");
     }
@@ -333,10 +332,11 @@ Solaris::Solaris(const Driver &D, const llvm::Triple &Triple,
 }
 
 SanitizerMask Solaris::getSupportedSanitizers() const {
+  const bool IsSparc = getTriple().getArch() == llvm::Triple::sparc;
   const bool IsX86 = getTriple().getArch() == llvm::Triple::x86;
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
-  // FIXME: Omit X86_64 until 64-bit support is figured out.
-  if (IsX86) {
+  // FIXME: Omit SparcV9 and X86_64 until 64-bit support is figured out.
+  if (IsSparc || IsX86) {
     Res |= SanitizerKind::Address;
     Res |= SanitizerKind::PointerCompare;
     Res |= SanitizerKind::PointerSubtract;

rorth added a commit to rorth/llvm-project that referenced this pull request Sep 5, 2024
With PR llvm#107223 and PR llvm#107403, ASan testing can be enabled on SPARC.  This
patch does so, 32-bit only on Solaris (there is no 64-bit support even in
GCC), and both 32 and 64-bit on Linux (although GCC only support 32-bit
here).

Apart from the obvious CMake changes, this patch includes a couple of
testcase adjustments necessary for SPARC:
- In `asan_oob_test.cpp`, the `OOB_int` subtest needs to be disabled: it
  performs unaligned accesses that cannot work on a strict-alignment target
  like SPARC.
- `asan_test.cpp` needs to disable subtests that depend on support for
  `__builtin_setjmp` and `__builtin_longjmp`.
- `zero_page_pc.cpp` reports `0x5` as the faulting address on access to
  `0x4`.  I don't really know why, but it's consistent between Solaris and
  Linux.

Tested on `sparcv9-sun-solaris2.11` and `sparc64-unknown-linux-gnu`.

Solaris results are not too bad (36 `FAIL`s) while the Linux ones are quite
bad, in particular because quite a number of tests just hang.

For those reasons, I'm just posting the patch for reference in case someone
else wants to try this on Linux.
@rorth
Copy link
Collaborator Author

rorth commented Sep 25, 2024

Ping? ASan results on Solaris/sparcv9 are now one failure away from clean with current PR #107223.

@vitalybuka
Copy link
Collaborator

Curios, why Asan when there is Sparc ADI?

@vitalybuka
Copy link
Collaborator

Also if there is a feature similar to ARM TBI, hwasan is a better option.

@rorth rorth changed the title [WIP][Driver] Enable ASan on Solaris/SPARC [Driver] Enable ASan on Solaris/SPARC Sep 25, 2024
@rorth
Copy link
Collaborator Author

rorth commented Sep 25, 2024

Curios, why Asan when there is Sparc ADI?

Various reasons:

  • Because I can, and gcc already does ;-)
  • Symmetry with other targets.
  • ADI is 64-bit only, while ASan could support both 32 and 64-bit (currently only the former, though).
  • ADI toolchain support is quite mixed right now. E.g. the GDB ADI support (though done by Oracle employees) is Linux/sparc64-only.

@rorth rorth merged commit d01e336 into llvm:main Sep 25, 2024
11 checks passed
augusto2112 pushed a commit to augusto2112/llvm-project that referenced this pull request Sep 26, 2024
Once PR llvm#107223 lands, ASan can be enabled on Solaris/SPARC. This patch
does just that. As on Solaris/x86, the dynamic ASan runtime lib needs to
be linked with `-z now` to avoid an `AsanInitInternal` cycle.

Tested on `sparcv9-sun-solaris2.11` and `sparc64-unknown-linux-gnu`.
xgupta pushed a commit to xgupta/llvm-project that referenced this pull request Oct 4, 2024
Once PR llvm#107223 lands, ASan can be enabled on Solaris/SPARC. This patch
does just that. As on Solaris/x86, the dynamic ASan runtime lib needs to
be linked with `-z now` to avoid an `AsanInitInternal` cycle.

Tested on `sparcv9-sun-solaris2.11` and `sparc64-unknown-linux-gnu`.
rorth added a commit that referenced this pull request Nov 4, 2024
With PR #107223 and PR #107403, ASan testing can be enabled on SPARC.
This patch does so, 32-bit only on both Solaris and Linux. There is no
64-bit support even in GCC.

Apart from the obvious CMake changes, this patch includes a couple of
testcase adjustments necessary for SPARC:
- In `asan_oob_test.cpp`, the `OOB_int` subtest needs to be disabled: it
performs unaligned accesses that cannot work on a strict-alignment
target like SPARC.
- `asan_test.cpp` needs to disable subtests that depend on support for
`__builtin_setjmp` and `__builtin_longjmp`.
- `zero_page_pc.cpp` reports `0x5` as the faulting address on access to
`0x4`. I don't really know why, but it's consistent between Solaris and
Linux.

Tested on `sparcv9-sun-solaris2.11` and `sparc64-unknown-linux-gnu`.
PhilippRados pushed a commit to PhilippRados/llvm-project that referenced this pull request Nov 6, 2024
With PR llvm#107223 and PR llvm#107403, ASan testing can be enabled on SPARC.
This patch does so, 32-bit only on both Solaris and Linux. There is no
64-bit support even in GCC.

Apart from the obvious CMake changes, this patch includes a couple of
testcase adjustments necessary for SPARC:
- In `asan_oob_test.cpp`, the `OOB_int` subtest needs to be disabled: it
performs unaligned accesses that cannot work on a strict-alignment
target like SPARC.
- `asan_test.cpp` needs to disable subtests that depend on support for
`__builtin_setjmp` and `__builtin_longjmp`.
- `zero_page_pc.cpp` reports `0x5` as the faulting address on access to
`0x4`. I don't really know why, but it's consistent between Solaris and
Linux.

Tested on `sparcv9-sun-solaris2.11` and `sparc64-unknown-linux-gnu`.
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.

5 participants