Allow using sanitizer runtimes on RISC-V #564

arichardson · 2021-08-18T15:44:19Z

Currently purecap and non-purecap runtime libs use the same library names and are installed to the same directory.
To work around this problem suffix the purecap ones with "c".

This also includes various fixes to allow building sanitizer_common for purecap again.

arichardson · 2021-09-09T13:20:53Z

Will update after the next upstream merge.

jrtc27 · 2021-09-09T14:10:42Z

I figured that made most sense for this one

We currently attempt to link libclang_rt-<lib>-riscv{32,64}.a for both purecap and non-purecap. Since there is no per-ABI subdirectory for the compiler-rt libraries, we should be using a different basename for the purecap runtime libs.

We have to use a different architecture suffix for purecap since not all sanitizers are supported. We already have one for MIPS but the previous logic to select it based on the target triple no longer works. This change should be more future proof and sets the RISC-V architecture suffixes to riscv{32,64}c.

This has regressed since the last merge.

We have to avoid calling renameat2 and clone on FreeBSD. Additionally, the mcontext structure has different members. Reviewed By: jrtc27, luismarques Differential Revision: https://reviews.llvm.org/D103886 (cherry picked from commit b475ce3)

Currently without save.S and restore.S as those need CTSRD-CHERI#533

Since both the plain RISC-V and purecap RISC-V compiler-rt libraries will be installed to the same directory we have to use a different name for the purecap ones. This patch uses a "c" suffix for the purecap ones.

arichardson · 2021-09-30T15:30:07Z

rebased after merge.

jrtc27 · 2021-10-24T16:34:19Z

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

@@ -167,7 +167,7 @@ namespace __sanitizer {
 #if !SANITIZER_S390
 uptr internal_mmap(void *addr, usize length, int prot, int flags, int fd,
                   u64 offset) {
-#ifdef __CHERI_PURE_CAPABILITY__
+#if SANITIZER_FREEBSD && defined(__CHERI_PURE_CAPABILITY__)
  return (uptr)mmap(addr, length, prot, flags, fd, offset);


What's wrong with internal_syscall? It returns uptr.

For some reason that didn't work, I really can't remember why. Will need to try again.

Maybe it predates making length a usize, so the varargs nature of syscall(2) broke it?

jrtc27 · 2021-10-24T16:35:06Z

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

@@ -1854,7 +1854,11 @@ SignalContext::WriteFlag SignalContext::GetWriteFlag() const {
  uint32_t op_code;

 # if SANITIZER_FREEBSD
+#  ifdef __CHERI_PURE_CAPABILITY__
+  exception_source = (uint32_t *)ucontext->uc_mcontext.mc_cheriframe.cf_pcc;


Grr split register files

jrtc27 · 2021-10-24T16:36:59Z

compiler-rt/lib/builtins/CMakeLists.txt

@@ -645,10 +644,15 @@ set(riscv32_SOURCES
  riscv/mulsi3.S
  ${riscv_SOURCES}
 )
+set(riscv32c_SOURCES ${riscv64_SOURCES})


Copy-paste error, though the lists happen to be the same

jrtc27 · 2021-10-24T16:37:17Z

compiler-rt/lib/builtins/CMakeLists.txt

@@ -606,7 +606,6 @@ set(mips64_SOURCES ${GENERIC_TF_SOURCES}
 set(mips64el_SOURCES ${GENERIC_TF_SOURCES}
                     ${mips_SOURCES})
 set(mips64c128_SOURCES ${GENERIC_TF_SOURCES} ${mips_SOURCES})
-set(mips64c256_SOURCES ${GENERIC_TF_SOURCES} ${mips_SOURCES})


You won't be missed...

jrtc27 · 2021-10-24T16:37:25Z

compiler-rt/lib/builtins/CMakeLists.txt

 set(riscv64_SOURCES
  riscv/muldi3.S
  ${riscv_SOURCES}
 )
+set(riscv64c_SOURCES ${riscv64_SOURCES})
+# save.S and restore.S need https://github.com/CTSRD-CHERI/llvm-project/pull/533


Can drop this now

jrtc27 · 2021-10-24T16:38:40Z

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

@@ -1934,9 +1934,13 @@ SignalContext::WriteFlag SignalContext::GetWriteFlag() const {
  return (instr >> 21) & 1 ? WRITE: READ;
 #elif defined(__riscv)
 #if SANITIZER_FREEBSD
-  unsigned long pc = ucontext->uc_mcontext.mc_gpregs.gp_sepc;
+#ifdef __CHERI_PURE_CAPABILITY__
+  uptr pc = ucontext->uc_mcontext.mc_capregs.cp_sepcc;


sigh yes we currently do this for purecap...

jrtc27 · 2021-10-24T16:39:15Z

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

@@ -2167,12 +2171,18 @@ static void GetPcSpBp(void *context, uptr *pc, uptr *sp, uptr *bp) {
 # endif
  *bp = ucontext->uc_mcontext.gregs[11];
  *sp = ucontext->uc_mcontext.gregs[15];
-#elif defined(__riscv)
-  ucontext_t *ucontext = (ucontext_t*)context;
+#  elif defined(__riscv)


Keep the upstream formatting to reduce diffs?

jrtc27 · 2021-10-24T16:42:39Z

clang/lib/Driver/ToolChain.cpp

-  if (Triple.isMIPS() && Triple.getEnvironment() == llvm::Triple::CheriPurecap) {
-    assert(Triple.getSubArch() != llvm::Triple::NoSubArch && "purecap triple should have subarch");
-    return Triple.getArchName();
+  if (Triple.isMIPS() && TC.isCheriPurecap()) {


Yay more CheriPurecap environment removal, last I looked at it it was a bit awkward to remove from several places but maybe I should revisit that

jrtc27 · 2021-10-24T16:45:26Z

Are there any implications for cheribuild here?

arichardson · 2021-10-24T17:06:50Z

Are there any implications for cheribuild here?

I believe some currently broken stuff might start working (building both compiler-rt-riscv64 and compiler-rt-riscv64-purecap), otherwise I don't think so.

jrtc27 · 2021-10-24T17:08:46Z

Are there any implications for cheribuild here?

I believe some currently broken stuff might start working (building both compiler-rt-riscv64 and compiler-rt-riscv64-purecap), otherwise I don't think so.

I believe the targets have knowledge about the names of the libraries though so might break or do weird things.

bsdjhb · 2022-07-06T17:30:12Z

compiler-rt/lib/sanitizer_common/sanitizer_deadlock_detector.h

@@ -395,14 +395,14 @@ class DeadlockDetector {
    int unique_tid;
  };

-  uptr current_epoch_;
+  usize current_epoch_;


FWIW, I changed all the indices, etc. in this file to also be usize. The only thing in this file that holds actual pointers is the data_ array.

arichardson requested a review from jrtc27 August 18, 2021 15:44

arichardson marked this pull request as draft September 9, 2021 13:20

arichardson added 9 commits September 30, 2021 16:14

[compiler-rt] Unbreak the MIPS purecap build

9a1a63a

This has regressed since the last merge.

[sanitizer] Fix build on FreeBSD RISC-V

d4dda34

We have to avoid calling renameat2 and clone on FreeBSD. Additionally, the mcontext structure has different members. Reviewed By: jrtc27, luismarques Differential Revision: https://reviews.llvm.org/D103886 (cherry picked from commit b475ce3)

[compiler-rt] Allow building the builtins for RISC-V purecap

6880e5d

Currently without save.S and restore.S as those need CTSRD-CHERI#533

sanitizer_linux_libcdep.cpp: reduce diff to upstream

418cc82

[compiler-rt] Fix a few -Wcheri-provenance warnings

16ac532

[compiler-rt] Fix purecap RISC-V build

de6704f

[CHERI-RISC-V] Use suffixed sanitizer libraries for purecap

f5ac08e

Since both the plain RISC-V and purecap RISC-V compiler-rt libraries will be installed to the same directory we have to use a different name for the purecap ones. This patch uses a "c" suffix for the purecap ones.

arichardson force-pushed the riscv-purecap-compiler-rt branch from 70457d3 to f5ac08e Compare September 30, 2021 15:29

arichardson marked this pull request as ready for review September 30, 2021 15:29

jrtc27 reviewed Oct 24, 2021

View reviewed changes

bsdjhb reviewed Jul 6, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow using sanitizer runtimes on RISC-V #564

Allow using sanitizer runtimes on RISC-V #564

arichardson commented Aug 18, 2021

arichardson commented Sep 9, 2021

jrtc27 commented Sep 9, 2021

arichardson commented Sep 30, 2021

jrtc27 Oct 24, 2021

arichardson Oct 24, 2021

jrtc27 Oct 24, 2021

jrtc27 Oct 24, 2021

jrtc27 Oct 24, 2021

jrtc27 Oct 24, 2021

jrtc27 Oct 24, 2021

jrtc27 Oct 24, 2021

jrtc27 Oct 24, 2021

jrtc27 Oct 24, 2021

jrtc27 commented Oct 24, 2021

arichardson commented Oct 24, 2021

jrtc27 commented Oct 24, 2021

bsdjhb Jul 6, 2022

Allow using sanitizer runtimes on RISC-V #564

Are you sure you want to change the base?

Allow using sanitizer runtimes on RISC-V #564

Conversation

arichardson commented Aug 18, 2021

arichardson commented Sep 9, 2021

jrtc27 commented Sep 9, 2021

arichardson commented Sep 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrtc27 commented Oct 24, 2021

arichardson commented Oct 24, 2021

jrtc27 commented Oct 24, 2021

Choose a reason for hiding this comment