[sanitizer] Add hexagon support to sanitizer-common

[bcain]

Summary

Adds build support for hexagon linux to sanitizer common.

[bcain]

clang/lib/Driver/ToolChains/Hexagon.cpp

297–298 ↗

(On Diff #366476)

I will try to determine which sanitizers require libunwind so that it's only added when appropriate.

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

162

Most of the feedback from clang-format checks don't seem to really be the format we use here. We should probably apply a more specific .clang-format file if these aren't useful.

compiler-rt/lib/sanitizer_common/sanitizer_syscall_linux_hexagon.inc

125

@sidneym Can you confirm that this implementation looks correct? I thought the convention was -1 for failed syscalls.

[sidneym]

compiler-rt/lib/sanitizer_common/sanitizer_syscall_linux_hexagon.inc

125

Maybe it should be <0 instead.

[vitalybuka]

Can you cut this into separate peaces, at least patch per sanitizer for "asan lsan ubsan scudo scudo_standalone cfi safestack"?
Some common peaces could be probably extracted as well.

compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h

106

please clang-format entire patch

compiler-rt/lib/sanitizer_common/sanitizer_syscall_linux_hexagon.inc

131

Please fix clang-tidy which can be fixed.

compiler-rt/test/asan/CMakeLists.txt

19 ↗

(On Diff #366476)

why only this test changes?

[vitalybuka]

Do we have public bot for hexagon?

[bcain]

In D108083#2947907, @vitalybuka wrote:

Can you cut this into separate peaces, at least patch per sanitizer for "asan lsan ubsan scudo scudo_standalone cfi safestack"?

Sure, that's no problem. While I'm breaking it up, us there a guideline/preference to whether the clang changes can be combined in one of these patches, or should those be on their own as well?

Some common peaces could be probably extracted as well.

compiler-rt/test/asan/CMakeLists.txt

19 ↗

(On Diff #366476)

Sorry, I shouldnt have included this, I'll omit it. I'd like to land the build changes before test ones.

[bcain]

In D108083#2947914, @vitalybuka wrote:

Do we have public bot for hexagon?

I think we have one intended to for checking hexagon-unknown-elf and perhaps not one for hexagon-unknown-linux-musl. I believe that only the Linux target uses the compiler-rt libraries, though.

We can help create a bot for hexagon-unknown-linux-musl. What's the process?

[bcain]

Decomposed to just the sanitizer-common portion. Now it's an 8 patch series, this is number 1.

[vitalybuka]

Please correct me if I am wrong, but LLVM does not use [PATCH *] tags for series. Please remove them from title and git patches.
You can link dependent patches into Phabricator "stack" above, but most extracted patches are orthogonal, and can be summited in any order.

[vitalybuka]

Sorry, approved accidentally.

[vitalybuka]

We can help create a bot for hexagon-unknown-linux-musl. What's the process?

Thank you. https://llvm.org/docs/HowToAddABuilder.html

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

162

clang-format pre-merge check uses .clang-format files.
If you apply it it locally make sure to use -style=file.

Actually this place is ugly. Current .clang-format will indent when the rest of the block is not.
So up to you, keep it as-is, or clang-format entire block 152-165 (ideally in a separate patch)

compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cpp

25

but please fix ones like this which do not require significant reformatting of unchanged code.

[bcain]

In D108083#2949815, @vitalybuka wrote:

We can help create a bot for hexagon-unknown-linux-musl. What's the process?

Thank you. https://llvm.org/docs/HowToAddABuilder.html

Oh I think I misunderstood:

Volunteers can provide their build machines to work as build workers to public LLVM Buildbot.

I was prepared to write / test / enable a build recipe but I don't have access to a public system that can be added as a worker. But I can work with our team to get the ball rolling, it just doesn't seem like it would be a quick process.

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

162

I did apply clang-format-diff via pre-commit hook and I don't believe it made significant changes, but this is after that formatting. I prefer to keep it as-is because the style near here seems to match.

compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cpp

25

Ok that's no problem.

[bcain]

compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cpp

25

but please fix ones like this which do not require significant reformatting of unchanged code.

Ok that's no problem.

But it's a little odd -- following the format would pull the or operator onto the previous line, which would deviate from the style used in the rest of the file.

Still happy to make the change, it just doesn't seem like it fits.

[vitalybuka]

In D108083#2949934, @bcain wrote:

In D108083#2949815, @vitalybuka wrote:

We can help create a bot for hexagon-unknown-linux-musl. What's the process?

Thank you. https://llvm.org/docs/HowToAddABuilder.html

Oh I think I misunderstood:

Volunteers can provide their build machines to work as build workers to public LLVM Buildbot.

I was prepared to write / test / enable a build recipe but I don't have access to a public system that can be added as a worker. But I can work with our team to get the ball rolling, it just doesn't seem like it would be a quick process.

Please try to do so. Even infrequent builds, like one per day, are helpful. Without that implementation will likely degrade overtime.

compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cpp

25

Goal is to avoid manual labor as much as possible even if clang-format result are a little bit inconsistent.
I guess code around was added before we started using clang-format.

[vitalybuka]

The last snapshot uploaded without "arc diff", so it does not show "context" code around, which is inconvenient for review.

[bcain]

added context

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp

94

please use clang-format version here.
I don't mind if you blindly clang-format entire patch.

compiler-rt/lib/sanitizer_common/sanitizer_syscall_linux_hexagon.inc

88

should this use u64 like other __internal_syscall files?

125

Why it's not -4095 like in other versions?

[bcain]

compiler-rt/lib/sanitizer_common/sanitizer_syscall_linux_hexagon.inc

88

Sorry I didn't notice this review comment before.

No, u64 doesn't seem appropriate - they're 32-bit registers. I can change it to u32 if that makes more sense.

[bcain]

applied clang-format-diff, s/long/u32/ for syscall funcs.

[bcain]

fixed iserror()

compiler-rt/lib/sanitizer_common/sanitizer_syscall_linux_hexagon.inc

125

Why it's not -4095 like in other versions?

It should be, I misunderstood the intent.

[vitalybuka]

LGTM, but maybe wait for @sidneym feedback, or add as "blocking reviewer".

compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake

 endif()
 
 set(ALL_SANITIZER_COMMON_SUPPORTED_ARCH ${X86} ${X86_64} ${PPC64} ${RISCV64}
-    ${ARM32} ${ARM64} ${MIPS32} ${MIPS64} ${S390X} ${SPARC} ${SPARCV9})
+    ${ARM32} ${ARM64} ${MIPS32} ${MIPS64} ${S390X} ${SPARC} ${SPARCV9}
+    ${HEXAGON})
 set(ALL_ASAN_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${RISCV64}
     ${MIPS32} ${MIPS64} ${PPC64} ${S390X} ${SPARC} ${SPARCV9})
 set(ALL_CRT_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${PPC32} ${PPC64} ${RISCV32} ${RISCV64} ${VE})

compiler-rt/lib/sanitizer_common/sanitizer_common.h

   kModuleArchARMV7S,
   kModuleArchARMV7K,
   kModuleArchARM64,
-  kModuleArchRISCV64
+  kModuleArchRISCV64,
+  kModuleArchHexagon
 };
 
 // Sorts and removes duplicates from the container.

       return "arm64";
     case kModuleArchRISCV64:
       return "riscv64";
+    case kModuleArchHexagon:
+      return "hexagon";
   }
   CHECK(0 && "Invalid module arch");
   return "";

compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h

 #if SANITIZER_FREEBSD || SANITIZER_NETBSD || \
     SANITIZER_MAC || \
     (SANITIZER_SOLARIS && (defined(_LP64) || _FILE_OFFSET_BITS == 64)) || \
-    (SANITIZER_LINUX && defined(__x86_64__))
+    (SANITIZER_LINUX && (defined(__x86_64__) || defined(__hexagon__)))
 typedef u64 OFF_T;
 #else
 typedef uptr OFF_T;

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

 #include "sanitizer_syscall_linux_aarch64.inc"
 #elif SANITIZER_LINUX && defined(__arm__)
 #include "sanitizer_syscall_linux_arm.inc"
+#elif SANITIZER_LINUX && defined(__hexagon__)
+#include "sanitizer_syscall_linux_hexagon.inc"
 #else
 #include "sanitizer_syscall_generic.inc"
 #endif

   *pc = ucontext->uc_mcontext.__gregs[REG_PC];
   *bp = ucontext->uc_mcontext.__gregs[REG_S0];
   *sp = ucontext->uc_mcontext.__gregs[REG_SP];
+#elif defined(__hexagon__)
+  ucontext_t *ucontext = (ucontext_t *)context;
+  *pc = ucontext->uc_mcontext.pc;
+  *bp = ucontext->uc_mcontext.r30;
+  *sp = ucontext->uc_mcontext.r29;
 #else
 # error "Unsupported arch"
 #endif

compiler-rt/lib/sanitizer_common/sanitizer_platform.h

 // mandated by the upstream linux community for all new ports. Other ports
 // may still use legacy syscalls.
 #ifndef SANITIZER_USES_CANONICAL_LINUX_SYSCALLS
-# if (defined(__aarch64__) || defined(__riscv)) && SANITIZER_LINUX
+# if (defined(__aarch64__) || defined(__riscv) || defined(__hexagon__)) && SANITIZER_LINUX
 # define SANITIZER_USES_CANONICAL_LINUX_SYSCALLS 1
 # else
 # define SANITIZER_USES_CANONICAL_LINUX_SYSCALLS 0

compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_linux.cpp

 // are not defined anywhere in userspace headers. Fake them. This seems to work
 // fine with newer headers, too.
 #include <linux/posix_types.h>
-#if defined(__x86_64__) ||  defined(__mips__)
+#if defined(__x86_64__) ||  defined(__mips__) || defined(__hexagon__)
 #include <sys/stat.h>
 #else
 #define ino_t __kernel_ino_t

 #endif
 }  // namespace __sanitizer
 
-#if !defined(__powerpc64__) && !defined(__x86_64__) && !defined(__aarch64__)\
-                            && !defined(__mips__) && !defined(__s390__)\
-                            && !defined(__sparc__) && !defined(__riscv)
+#  if !defined(__powerpc64__) && !defined(__x86_64__) &&                   \
+      !defined(__aarch64__) && !defined(__mips__) && !defined(__s390__) && \
+      !defined(__sparc__) && !defined(__riscv) && !defined(__hexagon__)
 COMPILER_CHECK(struct___old_kernel_stat_sz == sizeof(struct __old_kernel_stat));
 #endif
 

compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h

 #elif SANITIZER_RISCV64
 const unsigned struct_kernel_stat_sz = 128;
 const unsigned struct_kernel_stat64_sz = 0;  // RISCV64 does not use stat64
+#elif defined(__hexagon__)
+const unsigned struct_kernel_stat_sz = 128;
+const unsigned struct_kernel_stat64_sz = 0;
 #endif
 struct __sanitizer_perf_event_attr {
   unsigned type;

   char **gr_mem;
 };
 
-#if defined(__x86_64__) && !defined(_LP64)
+#if (defined(__x86_64__) && !defined(_LP64)) || defined(__hexagon__)
 typedef long long __sanitizer_time_t;
 #else
 typedef long __sanitizer_time_t;

   unsigned short d_reclen;
   // more fields that we don't care about
 };
-#elif SANITIZER_ANDROID || defined(__x86_64__)
+#elif SANITIZER_ANDROID || defined(__x86_64__) || defined(__hexagon__)
 struct __sanitizer_dirent {
   unsigned long long d_ino;
   unsigned long long d_off;

 
 #if SANITIZER_LINUX
 #if defined(_LP64) || defined(__x86_64__) || defined(__powerpc__) || \
-    defined(__mips__)
+    defined(__mips__) || defined(__hexagon__)
 typedef unsigned __sanitizer___kernel_uid_t;
 typedef unsigned __sanitizer___kernel_gid_t;
 #else

compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp

 # include <utime.h>
 # include <sys/ptrace.h>
 #if defined(__mips64) || defined(__aarch64__) || defined(__arm__) || \
+    defined(__hexagon__) || \
     SANITIZER_RISCV64
 #  include <asm/ptrace.h>
 #  ifdef __arm__

     defined(__x86_64__) || SANITIZER_RISCV64
 #define SIZEOF_STRUCT_USTAT 32
 #elif defined(__arm__) || defined(__i386__) || defined(__mips__) \
-  || defined(__powerpc__) || defined(__s390__) || defined(__sparc__)
+  || defined(__powerpc__) || defined(__s390__) || defined(__sparc__) \
+  || defined(__hexagon__)
 #define SIZEOF_STRUCT_USTAT 20
 #else
 #error Unknown size of struct ustat

compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cpp

 uptr StackTrace::GetNextInstructionPc(uptr pc) {
 #if defined(__sparc__) || defined(__mips__)
   return pc + 8;
-#elif defined(__powerpc__) || defined(__arm__) || defined(__aarch64__)
+#elif defined(__powerpc__) || defined(__arm__) || defined(__aarch64__) || \
+    defined(__hexagon__)
   return pc + 4;
 #elif SANITIZER_RISCV64
   // Current check order is 4 -> 2 -> 6 -> 8

compiler-rt/lib/sanitizer_common/sanitizer_syscall_linux_hexagon.inc

+//===-- sanitizer_syscall_linux_hexagon.inc ---------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Implementations of internal_syscall and internal_iserror for Linux/hexagon.
+//
+//===----------------------------------------------------------------------===//
+
+#define SYSCALL(name) __NR_ ## name
+
+#define __internal_syscall_LL_E(x) \
+((union { long long ll; long l[2]; }){ .ll = x }).l[0], \
+((union { long long ll; long l[2]; }){ .ll = x }).l[1]
+#define __internal_syscall_LL_O(x) 0, __SYSCALL_LL_E((x))
+
+#define __asm_syscall(...) do { \
+    __asm__ __volatile__ ( "trap0(#1)" \
+    : "=r"(r0) : __VA_ARGS__ : "memory"); \
+    return r0; \
+    } while (0)
+
+#define __internal_syscall0(n) \
+  (__internal_syscall)(n)
+
+static uptr __internal_syscall(long n)
+{
+  register long r6 __asm__("r6") = n;
+  register long r0 __asm__("r0");
+  __asm_syscall("r"(r6));
+}
+
+#define __internal_syscall1(n, a1) \
+  (__internal_syscall)(n, (long)(a1))
+
+static uptr __internal_syscall(long n, long a)
+{
+  register long r6 __asm__("r6") = n;
+  register long r0 __asm__("r0") = a;
+  __asm_syscall("r"(r6), "0"(r0));
+}
+
+#define __internal_syscall2(n, a1, a2) \
+  (__internal_syscall)(n, (long)(a1), (long)(a2))
+
+static uptr __internal_syscall(long n, long a, long b)
+{
+  register long r6 __asm__("r6") = n;
+  register long r0 __asm__("r0") = a;
+  register long r1 __asm__("r1") = b;
+  __asm_syscall("r"(r6), "0"(r0), "r"(r1));
+}
+
+#define __internal_syscall3(n, a1, a2, a3) \
+  (__internal_syscall)(n, (long)(a1), (long)(a2), (long)(a3))
+
+static uptr __internal_syscall(long n, long a, long b, long c)
+{
+  register long r6 __asm__("r6") = n;
+  register long r0 __asm__("r0") = a;
+  register long r1 __asm__("r1") = b;
+  register long r2 __asm__("r2") = c;
+  __asm_syscall("r"(r6), "0"(r0), "r"(r1), "r"(r2));
+}
+
+#define __internal_syscall4(n, a1, a2, a3, a4) \
+  (__internal_syscall)(n, (long)(a1), (long)(a2), (long)(a3), (long)(a4))
+
+static uptr __internal_syscall(long n, long a, long b, long c, long d)
+{
+  register long r6 __asm__("r6") = n;
+  register long r0 __asm__("r0") = a;
+  register long r1 __asm__("r1") = b;
+  register long r2 __asm__("r2") = c;
+  register long r3 __asm__("r3") = d;
+  __asm_syscall("r"(r6), "0"(r0), "r"(r1), "r"(r2), "r"(r3));
+}
+
+#define __internal_syscall5(n, a1, a2, a3, a4, a5) \
+  (__internal_syscall)(n, (long)(a1), (long)(a2), (long)(a3), (long)(a4), \
+                       (long)(a5))
+
+static uptr __internal_syscall(long n, long a, long b, long c, long d, long e)
+{
+  register long r6 __asm__("r6") = n;
+  register long r0 __asm__("r0") = a;
+  register long r1 __asm__("r1") = b;
+  register long r2 __asm__("r2") = c;
+  register long r3 __asm__("r3") = d;
+  register long r4 __asm__("r4") = e;
+  __asm_syscall("r"(r6), "0"(r0), "r"(r1), "r"(r2), "r"(r3), "r"(r4));
+}
+
+#define __internal_syscall6(n, a1, a2, a3, a4, a5, a6) \
+  (__internal_syscall)(n, (long)(a1), (long)(a2), (long)(a3), (long)(a4), \
+                       (long)(a5), (long)(a6))
+
+static uptr __internal_syscall(long n, long a, long b, long c, long d, long e,
+                              long f)
+{
+  register long r6 __asm__("r6") = n;
+  register long r0 __asm__("r0") = a;
+  register long r1 __asm__("r1") = b;
+  register long r2 __asm__("r2") = c;
+  register long r3 __asm__("r3") = d;
+  register long r4 __asm__("r4") = e;
+  register long r5 __asm__("r5") = f;
+  __asm_syscall("r"(r6), "0"(r0), "r"(r1), "r"(r2), "r"(r3), "r"(r4), "r"(r5));
+}
+
+#define __SYSCALL_NARGS_X(a1, a2, a3, a4, a5, a6, a7, a8, n, ...) n
+#define __SYSCALL_NARGS(...) \
+  __SYSCALL_NARGS_X(__VA_ARGS__, 7, 6, 5, 4, 3, 2, 1, 0, )
+#define __SYSCALL_CONCAT_X(a, b) a##b
+#define __SYSCALL_CONCAT(a, b) __SYSCALL_CONCAT_X(a, b)
+#define __SYSCALL_DISP(b, ...) \
+  __SYSCALL_CONCAT(b, __SYSCALL_NARGS(__VA_ARGS__))(__VA_ARGS__)
+
+#define internal_syscall(...) __SYSCALL_DISP(__internal_syscall, __VA_ARGS__)
+
+bool internal_iserror(uptr retval, int *rverrno) {
+  if (retval == (uptr)-1) {
+    if (rverrno)
+      *rverrno = errno;
+    return true;
+  }
+  return false;
+}