This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Add sanitizer support for AArch64 ILP32
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Fri, 23 Feb 2018 15:39:03 +0100
- Subject: Re: [PATCH] Add sanitizer support for AArch64 ILP32
- Authentication-results: sourceware.org; auth=none
- References: <1519396039-22827-1-git-send-email-adhemerval.zanella@linaro.org>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Fri, Feb 23, 2018 at 11:27:19AM -0300, Adhemerval Zanella wrote:
> This patch adds asan support for aarch64 ilp32. It is based on 'official'
> glibc support branch [1] (which is in turn based on Linux branch [2]).
>
> Main changes for libsanitizer is the kernel ABI support for internal
> syscalls. Different than x32, ILP32 tries to leverage 32-bits syscalls
> with kernel ABI for 64 bit argument passing (ftruncate for instance)
> are passed using the new Linux generic way for 32 bits (by splitting
> high and low in two registers).
>
> So instead of adding more adhoc defines to ILP32 this patch extends the
> SANITIZER_USES_CANONICAL_LINUX_SYSCALLS to represent both 64 bits
> argument syscalls (value of 1) and 32 bits (value of 2).
>
> The shadow offset used is similar to the arm one (29).
>
> I did a sanity check on aarch64-linux-gnu and x86_64-linux-gnu without
> any regressions.
>
> PS: I sent this change llvm-commit, but it got stalled because llvm
> lack aarch64 ilp32 support and noone could confirm no breakage due
> missing buildbot. Also the idea is not sync it back to libsanitizer.
It will be a nightmare for merges from upstream, but because the upstream is
so uncooperative probably there is no other way.
> gcc/
>
> * gcc/config/aarch64/aarch64.c (aarch64_asan_shadow_offset): Add
No gcc/ prefixes in gcc/ChangeLog.
> TARGET_ILP32 support.
>
> libsanitizer/
Nor libsanitizer/ prefixes in libsaniter/ChangeLog.
> --- a/libsanitizer/sanitizer_common/sanitizer_internal_defs.h
> +++ b/libsanitizer/sanitizer_common/sanitizer_internal_defs.h
> @@ -117,9 +117,9 @@ typedef signed long long sptr; // NOLINT
> typedef unsigned long uptr; // NOLINT
> typedef signed long sptr; // NOLINT
> #endif // defined(_WIN64)
> -#if defined(__x86_64__)
> -// Since x32 uses ILP32 data model in 64-bit hardware mode, we must use
> -// 64-bit pointer to unwind stack frame.
> +#if defined(__x86_64__) || SANITIZER_AARCH64_ILP32
> +// Since x32 adn AArch64 ILP32 use ILP32 data model in 64-bit hardware mode,
s/adn/and/
> @@ -445,11 +467,7 @@ uptr internal_execve(const char *filename, char *const argv[],
> // ----------------- sanitizer_common.h
> bool FileExists(const char *filename) {
> struct stat st;
> -#if SANITIZER_USES_CANONICAL_LINUX_SYSCALLS
> - if (internal_syscall(SYSCALL(newfstatat), AT_FDCWD, filename, &st, 0))
> -#else
> if (internal_stat(filename, &st))
> -#endif
> return false;
> // Sanity check: filename is a regular file.
> return S_ISREG(st.st_mode);
I'm uneasy about this hunk, you change behavior not just on aarch64 ilp32,
but for many other targets too. Please don't.
Do you really want it for GCC8? We are in stage4 and this isn't a
regression...
Jakub