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

Reply via email to