Re: 3781e77995e9 - main - riscv: actually enable sanitizers

From: Ravi Pokala <rpokala_at_freebsd.org>
Date: Mon, 14 Mar 2022 00:59:33 UTC
-----Original Message-----
From: Piotr Kubaj <pkubaj@anongoth.pl>
Date: 2022-03-13, Sunday at 15:39
To: Ravi Pokala <rpokala@panasas.com>
Cc: "src-committers@FreeBSD.org" <src-committers@freebsd.org>, "dev-commits-src-all@FreeBSD.org" <dev-commits-src-all@freebsd.org>, "dev-commits-src-main@FreeBSD.org" <dev-commits-src-main@freebsd.org>
Subject: Re: 3781e77995e9 - main - riscv: actually enable sanitizers

    On 22-03-13 21:29:19, Pokala, Ravi wrote:
    > Hi Piotr,
    > 
    > > --- a/lib/Makefile
    > > +++ b/lib/Makefile
    > > @@ -184,7 +184,7 @@ SUBDIR.${MK_STATS}+=	libstats
    > > .if ${COMPILER_TYPE} == "clang" && ${MK_CXX} != "no" && \
    > >      (${MACHINE_CPUARCH} == "aarch64" || ${MACHINE_CPUARCH} == "amd64" || \
    > >      ${MACHINE_CPUARCH} == "arm" || ${MACHINE_CPUARCH} == "i386" || \
    > > -    ${MACHINE_CPUARCH} == "powerpc")
    > > +    ${MACHINE_CPUARCH} == "powerpc" || ${MACHINE_CPUARCH} == "riscv")
    > > _libclang_rt=	libclang_rt
    > > .elif (${MK_ASAN} != "no" || ${MK_UBSAN} != "no") && make(all)
    > > .error Requested build with sanitizers but cannot build runtime libraries!
    > 
    > I believe the conditional now covers all architectures. Couldn't / shouldn't it simply be removed?
    It can, but I left it as it is for MFC'ing purposes.

    13 still supports mips.

Good point. Perhaps some post-MFC cleanup then.

Thanks,

Ravi (rpokala@)

    > 
    > Thanks,
    > 
    > Ravi (rpokala@)
    > 
    > -----Original Message-----
    > From: <owner-src-committers@freebsd.org> on behalf of Piotr Kubaj <pkubaj@FreeBSD.org>
    > Date: 2022-03-13, Sunday at 05:07
    > To: <src-committers@FreeBSD.org>, <dev-commits-src-all@FreeBSD.org>, <dev-commits-src-main@FreeBSD.org>
    > Subject: git: 3781e77995e9 - main - riscv: actually enable sanitizers
    > 
    >     The branch main has been updated by pkubaj (ports committer):
    > 
    >     URL: https://cgit.FreeBSD.org/src/commit/?id=3781e77995e9b81afcbf7b3dc84b33b864c48794
    > 
    >     commit 3781e77995e9b81afcbf7b3dc84b33b864c48794
    >     Author:     Piotr Kubaj <pkubaj@FreeBSD.org>
    >     AuthorDate: 2022-03-13 03:31:35 +0000
    >     Commit:     Piotr Kubaj <pkubaj@FreeBSD.org>
    >     CommitDate: 2022-03-13 12:06:57 +0000
    > 
    >         riscv: actually enable sanitizers
    > 
    >         1. Backport https://github.com/llvm/llvm-project/commit/b475ce39e8b1de3a70ea242473f136a567be46e3.
    >         2. Enable libclang_rt for riscv.
    > 
    >         Previous commit missed it.
    > 
    >         MFC after:      3 days
    >         Reviewed by:    dim
    >         Differential Revision: https://reviews.freebsd.org/D34543
    >     ---
    >      .../lib/sanitizer_common/sanitizer_linux.cpp       | 22 +++++++++++++++++-----
    >      lib/Makefile                                       |  2 +-
    >      2 files changed, 18 insertions(+), 6 deletions(-)
    > 
    >     diff --git a/contrib/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp b/contrib/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
    >     index daa07c15490e..769e8029dfcb 100644
    >     --- a/contrib/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
    >     +++ b/contrib/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
    >     @@ -416,7 +416,7 @@ uptr internal_unlink(const char *path) {
    >      }
    > 
    >      uptr internal_rename(const char *oldpath, const char *newpath) {
    >     -#if defined(__riscv)
    >     +#if defined(__riscv) && defined(__linux__)
    >        return internal_syscall(SYSCALL(renameat2), AT_FDCWD, (uptr)oldpath, AT_FDCWD,
    >                                (uptr)newpath, 0);
    >      #elif SANITIZER_USES_CANONICAL_LINUX_SYSCALLS
    >     @@ -1218,7 +1218,8 @@ void ForEachMappedRegion(link_map *map, void (*cb)(const void *, uptr)) {
    >      }
    >      #endif
    > 
    >     -#if defined(__x86_64__) && SANITIZER_LINUX
    >     +#if SANITIZER_LINUX
    >     +#if defined(__x86_64__)
    >      // We cannot use glibc's clone wrapper, because it messes with the child
    >      // task's TLS. It writes the PID and TID of the child task to its thread
    >      // descriptor, but in our case the child task shares the thread descriptor with
    >     @@ -1557,7 +1558,7 @@ uptr internal_clone(int (*fn)(void *), void *child_stack, int flags, void *arg,
    >                   : "cr0", "cr1", "memory", "ctr", "r0", "r27", "r28", "r29");
    >        return res;
    >      }
    >     -#elif defined(__i386__) && SANITIZER_LINUX
    >     +#elif defined(__i386__)
    >      uptr internal_clone(int (*fn)(void *), void *child_stack, int flags, void *arg,
    >                          int *parent_tidptr, void *newtls, int *child_tidptr) {
    >        int res;
    >     @@ -1622,7 +1623,7 @@ uptr internal_clone(int (*fn)(void *), void *child_stack, int flags, void *arg,
    >                             : "memory");
    >        return res;
    >      }
    >     -#elif defined(__arm__) && SANITIZER_LINUX
    >     +#elif defined(__arm__)
    >      uptr internal_clone(int (*fn)(void *), void *child_stack, int flags, void *arg,
    >                          int *parent_tidptr, void *newtls, int *child_tidptr) {
    >        unsigned int res;
    >     @@ -1688,7 +1689,8 @@ uptr internal_clone(int (*fn)(void *), void *child_stack, int flags, void *arg,
    >                             : "memory");
    >        return res;
    >      }
    >     -#endif  // defined(__x86_64__) && SANITIZER_LINUX
    >     +#endif
    >     +#endif  // SANITIZER_LINUX
    > 
    >      #if SANITIZER_LINUX
    >      int internal_uname(struct utsname *buf) {
    >     @@ -1918,7 +1920,11 @@ SignalContext::WriteFlag SignalContext::GetWriteFlag() const {
    >        u32 instr = *(u32 *)pc;
    >        return (instr >> 21) & 1 ? WRITE: READ;
    >      #elif defined(__riscv)
    >     +#if SANITIZER_FREEBSD
    >     +  unsigned long pc = ucontext->uc_mcontext.mc_gpregs.gp_sepc;
    >     +#else
    >        unsigned long pc = ucontext->uc_mcontext.__gregs[REG_PC];
    >     +#endif
    >        unsigned faulty_instruction = *(uint16_t *)pc;
    > 
    >      #if defined(__riscv_compressed)
    >     @@ -2137,9 +2143,15 @@ static void GetPcSpBp(void *context, uptr *pc, uptr *sp, uptr *bp) {
    >        *sp = ucontext->uc_mcontext.gregs[15];
    >      #elif defined(__riscv)
    >        ucontext_t *ucontext = (ucontext_t*)context;
    >     +#    if SANITIZER_FREEBSD
    >     +  *pc = ucontext->uc_mcontext.mc_gpregs.gp_sepc;
    >     +  *bp = ucontext->uc_mcontext.mc_gpregs.gp_s[0];
    >     +  *sp = ucontext->uc_mcontext.mc_gpregs.gp_sp;
    >     +#    else
    >        *pc = ucontext->uc_mcontext.__gregs[REG_PC];
    >        *bp = ucontext->uc_mcontext.__gregs[REG_S0];
    >        *sp = ucontext->uc_mcontext.__gregs[REG_SP];
    >     +#    endif
    >      #else
    >      # error "Unsupported arch"
    >      #endif
    >     diff --git a/lib/Makefile b/lib/Makefile
    >     index d34f40616396..816374552f0a 100644
    >     --- a/lib/Makefile
    >     +++ b/lib/Makefile
    >     @@ -184,7 +184,7 @@ SUBDIR.${MK_STATS}+=	libstats
    >      .if ${COMPILER_TYPE} == "clang" && ${MK_CXX} != "no" && \
    >          (${MACHINE_CPUARCH} == "aarch64" || ${MACHINE_CPUARCH} == "amd64" || \
    >          ${MACHINE_CPUARCH} == "arm" || ${MACHINE_CPUARCH} == "i386" || \
    >     -    ${MACHINE_CPUARCH} == "powerpc")
    >     +    ${MACHINE_CPUARCH} == "powerpc" || ${MACHINE_CPUARCH} == "riscv")
    >      _libclang_rt=	libclang_rt
    >      .elif (${MK_ASAN} != "no" || ${MK_UBSAN} != "no") && make(all)
    >      .error Requested build with sanitizers but cannot build runtime libraries!
    >