[RISC-V] Add support for AddressSanitizer on RISC-V GCC

[RISC-V] Add support for AddressSanitizer on RISC-V GCC

[Joshua]

From: cooper.joshua <cooper.joshua@linux.alibaba.com>

    gcc/
    
        * config/riscv/riscv.c (asan_shadow_offset): Implement the offset of asan shadow memory for risc-v.
        (asan_shadow_offset): new macro definition.
---

 gcc/config/riscv/riscv.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index 63b0c38..b85b459 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -5292,6 +5292,14 @@ riscv_gpr_save_operation_p (rtx op)
   return true;
 }
 
+/* Implement TARGET_ASAN_SHADOW_OFFSET.  */
+
+static unsigned HOST_WIDE_INT
+riscv_asan_shadow_offset (void)
+{
+  return HOST_WIDE_INT_1U << 36;
+}
+
 /* Initialize the GCC target structure.  */
 #undef TARGET_ASM_ALIGNED_HI_OP
 #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
@@ -5475,6 +5483,9 @@ riscv_gpr_save_operation_p (rtx op)
 #undef TARGET_NEW_ADDRESS_PROFITABLE_P
 #define TARGET_NEW_ADDRESS_PROFITABLE_P riscv_new_address_profitable_p
 
+#undef TARGET_ASAN_SHADOW_OFFSET
+#define TARGET_ASAN_SHADOW_OFFSET riscv_asan_shadow_offset
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-riscv.h"
-- 
2.7.4

Re: [RISC-V] Add support for AddressSanitizer on RISC-V GCC

[Andrew Waterman]

I'm having trouble understanding why different ports chose their
various constants--e.g., SPARC uses 1<<29 for 32-bit and 1<<43 for
64-bit, whereas x86 uses 1<<29 and 0x7fff8000, respectively.  So I
can't comment on the choice of the constant 1<<36 for RISC-V.  But
isn't it a problem that 1<<36 is not a valid Pmode value for ILP32?


On Wed, Aug 19, 2020 at 1:02 AM Joshua via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: cooper.joshua <cooper.joshua@linux.alibaba.com>
>
>     gcc/
>
>         * config/riscv/riscv.c (asan_shadow_offset): Implement the offset of asan shadow memory for risc-v.
>         (asan_shadow_offset): new macro definition.
> ---
>
>  gcc/config/riscv/riscv.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
> index 63b0c38..b85b459 100644
> --- a/gcc/config/riscv/riscv.c
> +++ b/gcc/config/riscv/riscv.c
> @@ -5292,6 +5292,14 @@ riscv_gpr_save_operation_p (rtx op)
>    return true;
>  }
>
> +/* Implement TARGET_ASAN_SHADOW_OFFSET.  */
> +
> +static unsigned HOST_WIDE_INT
> +riscv_asan_shadow_offset (void)
> +{
> +  return HOST_WIDE_INT_1U << 36;
> +}
> +
>  /* Initialize the GCC target structure.  */
>  #undef TARGET_ASM_ALIGNED_HI_OP
>  #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
> @@ -5475,6 +5483,9 @@ riscv_gpr_save_operation_p (rtx op)
>  #undef TARGET_NEW_ADDRESS_PROFITABLE_P
>  #define TARGET_NEW_ADDRESS_PROFITABLE_P riscv_new_address_profitable_p
>
> +#undef TARGET_ASAN_SHADOW_OFFSET
> +#define TARGET_ASAN_SHADOW_OFFSET riscv_asan_shadow_offset
> +
>  struct gcc_target targetm = TARGET_INITIALIZER;
>
>  #include "gt-riscv.h"
> --
> 2.7.4
>

Re: [RISC-V] Add support for AddressSanitizer on RISC-V GCC

[Kito Cheng]

Hi Andrew:

I am not sure the reason why some targets pick different numbers.
It seems it's not only target dependent but also OS dependent[1].

For RV32, I think using 1<<29 like other 32 bit targets is fine.

[1] https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/asan/asan_mapping.h#L159

Hi Joshua:

Could you update that for RV32, and this patch will be pending until
LLVM accepts the libsanitizer part.

On Wed, Aug 19, 2020 at 4:48 PM Andrew Waterman <andrew@sifive.com> wrote:
>
> I'm having trouble understanding why different ports chose their
> various constants--e.g., SPARC uses 1<<29 for 32-bit and 1<<43 for
> 64-bit, whereas x86 uses 1<<29 and 0x7fff8000, respectively.  So I
> can't comment on the choice of the constant 1<<36 for RISC-V.  But
> isn't it a problem that 1<<36 is not a valid Pmode value for ILP32?
>
>
> On Wed, Aug 19, 2020 at 1:02 AM Joshua via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > From: cooper.joshua <cooper.joshua@linux.alibaba.com>
> >
> >     gcc/
> >
> >         * config/riscv/riscv.c (asan_shadow_offset): Implement the offset of asan shadow memory for risc-v.
> >         (asan_shadow_offset): new macro definition.
> > ---
> >
> >  gcc/config/riscv/riscv.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
> > index 63b0c38..b85b459 100644
> > --- a/gcc/config/riscv/riscv.c
> > +++ b/gcc/config/riscv/riscv.c
> > @@ -5292,6 +5292,14 @@ riscv_gpr_save_operation_p (rtx op)
> >    return true;
> >  }
> >
> > +/* Implement TARGET_ASAN_SHADOW_OFFSET.  */
> > +
> > +static unsigned HOST_WIDE_INT
> > +riscv_asan_shadow_offset (void)
> > +{
> > +  return HOST_WIDE_INT_1U << 36;
> > +}
> > +
> >  /* Initialize the GCC target structure.  */
> >  #undef TARGET_ASM_ALIGNED_HI_OP
> >  #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
> > @@ -5475,6 +5483,9 @@ riscv_gpr_save_operation_p (rtx op)
> >  #undef TARGET_NEW_ADDRESS_PROFITABLE_P
> >  #define TARGET_NEW_ADDRESS_PROFITABLE_P riscv_new_address_profitable_p
> >
> > +#undef TARGET_ASAN_SHADOW_OFFSET
> > +#define TARGET_ASAN_SHADOW_OFFSET riscv_asan_shadow_offset
> > +
> >  struct gcc_target targetm = TARGET_INITIALIZER;
> >
> >  #include "gt-riscv.h"
> > --
> > 2.7.4
> >

Re: [RISC-V] Add support for AddressSanitizer on RISC-V GCC

[Palmer Dabbelt]

On Wed, 19 Aug 2020 02:25:37 PDT (-0700), gcc-patches@gcc.gnu.org wrote:
> Hi Andrew:
>
> I am not sure the reason why some targets pick different numbers.
> It seems it's not only target dependent but also OS dependent[1].
>
> For RV32, I think using 1<<29 like other 32 bit targets is fine.
>
> [1] https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/asan/asan_mapping.h#L159
>
> Hi Joshua:
>
> Could you update that for RV32, and this patch will be pending until
> LLVM accepts the libsanitizer part.

This is ABI, and Linux only supports kasan on rv64 right now so it's
technically undefined.  It's probably best to avoid picking an arbitrary number
for rv32, as we still have some open questions WRT the kernel memory map over
there.  I doubt that will get sorted out for a while, as the rv32 doesn't get a
lot of attention (though hopefully the glibc stuff will help out).

> On Wed, Aug 19, 2020 at 4:48 PM Andrew Waterman <andrew@sifive.com> wrote:
>>
>> I'm having trouble understanding why different ports chose their
>> various constants--e.g., SPARC uses 1<<29 for 32-bit and 1<<43 for
>> 64-bit, whereas x86 uses 1<<29 and 0x7fff8000, respectively.  So I
>> can't comment on the choice of the constant 1<<36 for RISC-V.  But
>> isn't it a problem that 1<<36 is not a valid Pmode value for ILP32?

This is for kasan (not regular asan), which requires some coordination between
the kernel's memory map and the compiler's inline address sanitizer (as you
can't just pick your own memory map).  Essentially what's going on is that
there's an array of valid tags associated with each address, which is checked
in-line by the compiler for performance reasons (IIRC it used to be library
routines).  The compiler needs to know how to map between addresses and tags,
which depends on the kernel's memory map -- essentially baking the kernel's
memory map into the compiler.  That's why the constants seem somewhat
arbitrary.

In order to save memory there's some lossyness in the address->tag mapping.
Most 32-bit ports pick a tag array that's 1/8th of the memory size, which is
where the 29 comes from.  I don't see any reason why that wouldn't be workable
on rv32, but it seems better to make sure that's the case rather than just
making up an ABI :)

>> On Wed, Aug 19, 2020 at 1:02 AM Joshua via Gcc-patches
>> <gcc-patches@gcc.gnu.org> wrote:
>> >
>> > From: cooper.joshua <cooper.joshua@linux.alibaba.com>
>> >
>> >     gcc/
>> >
>> >         * config/riscv/riscv.c (asan_shadow_offset): Implement the offset of asan shadow memory for risc-v.
>> >         (asan_shadow_offset): new macro definition.
>> > ---
>> >
>> >  gcc/config/riscv/riscv.c | 11 +++++++++++
>> >  1 file changed, 11 insertions(+)
>> >
>> > diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
>> > index 63b0c38..b85b459 100644
>> > --- a/gcc/config/riscv/riscv.c
>> > +++ b/gcc/config/riscv/riscv.c
>> > @@ -5292,6 +5292,14 @@ riscv_gpr_save_operation_p (rtx op)
>> >    return true;
>> >  }
>> >
>> > +/* Implement TARGET_ASAN_SHADOW_OFFSET.  */
>> > +
>> > +static unsigned HOST_WIDE_INT
>> > +riscv_asan_shadow_offset (void)
>> > +{
>> > +  return HOST_WIDE_INT_1U << 36;
>> > +}
>> > +
>> >  /* Initialize the GCC target structure.  */
>> >  #undef TARGET_ASM_ALIGNED_HI_OP
>> >  #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
>> > @@ -5475,6 +5483,9 @@ riscv_gpr_save_operation_p (rtx op)
>> >  #undef TARGET_NEW_ADDRESS_PROFITABLE_P
>> >  #define TARGET_NEW_ADDRESS_PROFITABLE_P riscv_new_address_profitable_p
>> >
>> > +#undef TARGET_ASAN_SHADOW_OFFSET
>> > +#define TARGET_ASAN_SHADOW_OFFSET riscv_asan_shadow_offset
>> > +
>> >  struct gcc_target targetm = TARGET_INITIALIZER;
>> >
>> >  #include "gt-riscv.h"
>> > --
>> > 2.7.4
>> >

回复：[RISC-V] Add support for AddressSanitizer on RISC-V GCC

[joshua]

Hi Palmer,

The 64-bit RISC-V Linux port has a minimum of 39-bit virtual addresses, so it should be 1<<36 for 64-bit targets. In the implementation of address sanitizer, we need a shadow memory that is 1/8th of the memory size, which is
where the 36 comes from. I don't think the choice of this value is arbitrary. 

Jun
------------------------------------------------------------------
发件人：Palmer Dabbelt <palmer@dabbelt.com>
发送时间：2020年8月21日(星期五) 00:04
收件人：gcc-patches <gcc-patches@gcc.gnu.org>
抄　送：Kito Cheng <kito.cheng@gmail.com>; Andrew Waterman <andrew@sifive.com>; gcc-patches <gcc-patches@gcc.gnu.org>; cooper.joshua <cooper.joshua@linux.alibaba.com>
主　题：Re: [RISC-V] Add support for AddressSanitizer on RISC-V GCC

On Wed, 19 Aug 2020 02:25:37 PDT (-0700), gcc-patches@gcc.gnu.org wrote:
> Hi Andrew:
>
> I am not sure the reason why some targets pick different numbers.
> It seems it's not only target dependent but also OS dependent[1].
>
> For RV32, I think using 1<<29 like other 32 bit targets is fine.
>
> [1] https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/asan/asan_mapping.h#L159
>
> Hi Joshua:
>
> Could you update that for RV32, and this patch will be pending until
> LLVM accepts the libsanitizer part.

This is ABI, and Linux only supports kasan on rv64 right now so it's
technically undefined.  It's probably best to avoid picking an arbitrary number
for rv32, as we still have some open questions WRT the kernel memory map over
there.  I doubt that will get sorted out for a while, as the rv32 doesn't get a
lot of attention (though hopefully the glibc stuff will help out).

> On Wed, Aug 19, 2020 at 4:48 PM Andrew Waterman <andrew@sifive.com> wrote:
>>
>> I'm having trouble understanding why different ports chose their
>> various constants--e.g., SPARC uses 1<<29 for 32-bit and 1<<43 for
>> 64-bit, whereas x86 uses 1<<29 and 0x7fff8000, respectively.  So I
>> can't comment on the choice of the constant 1<<36 for RISC-V.  But
>> isn't it a problem that 1<<36 is not a valid Pmode value for ILP32?

This is for kasan (not regular asan), which requires some coordination between
the kernel's memory map and the compiler's inline address sanitizer (as you
can't just pick your own memory map).  Essentially what's going on is that
there's an array of valid tags associated with each address, which is checked
in-line by the compiler for performance reasons (IIRC it used to be library
routines).  The compiler needs to know how to map between addresses and tags,
which depends on the kernel's memory map -- essentially baking the kernel's
memory map into the compiler.  That's why the constants seem somewhat
arbitrary.

In order to save memory there's some lossyness in the address->tag mapping.
Most 32-bit ports pick a tag array that's 1/8th of the memory size, which is
where the 29 comes from.  I don't see any reason why that wouldn't be workable
on rv32, but it seems better to make sure that's the case rather than just
making up an ABI :)

>> On Wed, Aug 19, 2020 at 1:02 AM Joshua via Gcc-patches
>> <gcc-patches@gcc.gnu.org> wrote:
>> >
>> > From: cooper.joshua <cooper.joshua@linux.alibaba.com>
>> >
>> >     gcc/
>> >
>> >         * config/riscv/riscv.c (asan_shadow_offset): Implement the offset of asan shadow memory for risc-v.
>> >         (asan_shadow_offset): new macro definition.
>> > ---
>> >
>> >  gcc/config/riscv/riscv.c | 11 +++++++++++
>> >  1 file changed, 11 insertions(+)
>> >
>> > diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
>> > index 63b0c38..b85b459 100644
>> > --- a/gcc/config/riscv/riscv.c
>> > +++ b/gcc/config/riscv/riscv.c
>> > @@ -5292,6 +5292,14 @@ riscv_gpr_save_operation_p (rtx op)
>> >    return true;
>> >  }
>> >
>> > +/* Implement TARGET_ASAN_SHADOW_OFFSET.  */
>> > +
>> > +static unsigned HOST_WIDE_INT
>> > +riscv_asan_shadow_offset (void)
>> > +{
>> > +  return HOST_WIDE_INT_1U << 36;
>> > +}
>> > +
>> >  /* Initialize the GCC target structure.  */
>> >  #undef TARGET_ASM_ALIGNED_HI_OP
>> >  #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
>> > @@ -5475,6 +5483,9 @@ riscv_gpr_save_operation_p (rtx op)
>> >  #undef TARGET_NEW_ADDRESS_PROFITABLE_P
>> >  #define TARGET_NEW_ADDRESS_PROFITABLE_P riscv_new_address_profitable_p
>> >
>> > +#undef TARGET_ASAN_SHADOW_OFFSET
>> > +#define TARGET_ASAN_SHADOW_OFFSET riscv_asan_shadow_offset
>> > +
>> >  struct gcc_target targetm = TARGET_INITIALIZER;
>> >
>> >  #include "gt-riscv.h"
>> > --
>> > 2.7.4
>> >

Re: [RISC-V] Add support for AddressSanitizer on RISC-V GCC

[Kito Cheng]

On Fri, Aug 21, 2020 at 12:04 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Wed, 19 Aug 2020 02:25:37 PDT (-0700), gcc-patches@gcc.gnu.org wrote:
> > Hi Andrew:
> >
> > I am not sure the reason why some targets pick different numbers.
> > It seems it's not only target dependent but also OS dependent[1].
> >
> > For RV32, I think using 1<<29 like other 32 bit targets is fine.
> >
> > [1] https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/asan/asan_mapping.h#L159
> >
> > Hi Joshua:
> >
> > Could you update that for RV32, and this patch will be pending until
> > LLVM accepts the libsanitizer part.
>
> This is ABI, and Linux only supports kasan on rv64 right now so it's
> technically undefined.  It's probably best to avoid picking an arbitrary number
> for rv32, as we still have some open questions WRT the kernel memory map over
> there.  I doubt that will get sorted out for a while, as the rv32 doesn't get a
> lot of attention (though hopefully the glibc stuff will help out).

Yeah, I agree it's part of ABI, and I think this part could wait until
rv32 glibc upstream, the day seems not too far.

> > On Wed, Aug 19, 2020 at 4:48 PM Andrew Waterman <andrew@sifive.com> wrote:
> >>
> >> I'm having trouble understanding why different ports chose their
> >> various constants--e.g., SPARC uses 1<<29 for 32-bit and 1<<43 for
> >> 64-bit, whereas x86 uses 1<<29 and 0x7fff8000, respectively.  So I
> >> can't comment on the choice of the constant 1<<36 for RISC-V.  But
> >> isn't it a problem that 1<<36 is not a valid Pmode value for ILP32?
>
> This is for kasan (not regular asan), which requires some coordination between
> the kernel's memory map and the compiler's inline address sanitizer (as you
> can't just pick your own memory map).  Essentially what's going on is that
> there's an array of valid tags associated with each address, which is checked
> in-line by the compiler for performance reasons (IIRC it used to be library
> routines).  The compiler needs to know how to map between addresses and tags,
> which depends on the kernel's memory map -- essentially baking the kernel's
> memory map into the compiler.  That's why the constants seem somewhat
> arbitrary.

IIRC kasan will give the offset via -fasan-shadow-offset,
so TARGET_ASAN_SHADOW_OFFSET only meaningful for (user-space) asan.

>
> In order to save memory there's some lossyness in the address->tag mapping.
> Most 32-bit ports pick a tag array that's 1/8th of the memory size, which is
> where the 29 comes from.  I don't see any reason why that wouldn't be workable
> on rv32, but it seems better to make sure that's the case rather than just
> making up an ABI :)

I guess we could try it after rv32 glibc upstream.

>
> >> On Wed, Aug 19, 2020 at 1:02 AM Joshua via Gcc-patches
> >> <gcc-patches@gcc.gnu.org> wrote:
> >> >
> >> > From: cooper.joshua <cooper.joshua@linux.alibaba.com>
> >> >
> >> >     gcc/
> >> >
> >> >         * config/riscv/riscv.c (asan_shadow_offset): Implement the offset of asan shadow memory for risc-v.
> >> >         (asan_shadow_offset): new macro definition.
> >> > ---
> >> >
> >> >  gcc/config/riscv/riscv.c | 11 +++++++++++
> >> >  1 file changed, 11 insertions(+)
> >> >
> >> > diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
> >> > index 63b0c38..b85b459 100644
> >> > --- a/gcc/config/riscv/riscv.c
> >> > +++ b/gcc/config/riscv/riscv.c
> >> > @@ -5292,6 +5292,14 @@ riscv_gpr_save_operation_p (rtx op)
> >> >    return true;
> >> >  }
> >> >
> >> > +/* Implement TARGET_ASAN_SHADOW_OFFSET.  */
> >> > +
> >> > +static unsigned HOST_WIDE_INT
> >> > +riscv_asan_shadow_offset (void)
> >> > +{
> >> > +  return HOST_WIDE_INT_1U << 36;
> >> > +}
> >> > +
> >> >  /* Initialize the GCC target structure.  */
> >> >  #undef TARGET_ASM_ALIGNED_HI_OP
> >> >  #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
> >> > @@ -5475,6 +5483,9 @@ riscv_gpr_save_operation_p (rtx op)
> >> >  #undef TARGET_NEW_ADDRESS_PROFITABLE_P
> >> >  #define TARGET_NEW_ADDRESS_PROFITABLE_P riscv_new_address_profitable_p
> >> >
> >> > +#undef TARGET_ASAN_SHADOW_OFFSET
> >> > +#define TARGET_ASAN_SHADOW_OFFSET riscv_asan_shadow_offset
> >> > +
> >> >  struct gcc_target targetm = TARGET_INITIALIZER;
> >> >
> >> >  #include "gt-riscv.h"
> >> > --
> >> > 2.7.4
> >> >

Re: [RISC-V] Add support for AddressSanitizer on RISC-V GCC

[Jim Wilson]

On Wed, Aug 19, 2020 at 1:02 AM Joshua via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>         * config/riscv/riscv.c (asan_shadow_offset): Implement the offset of asan shadow memory for risc-v.
>         (asan_shadow_offset): new macro definition.

When I try the patch, I get asan errors complaining about memory mappings.

==9422==Shadow memory range interleaves with an existing memory
mapping. ASan cannot proceed correctly. ABORTING.
==9422==ASan shadow was supposed to be located in the
[0x00007fff7000-0x10007fff7fff] range.

I haven't gotten it working yet, but I haven't tried debugging it yet
either.  How exactly are you testing it?  It fails for me using a 4.15
kernel on hardware, and on a user mode qemu.  I haven't tried booting
a 5.x kernel in qemu, that would take some time to set up.  Likewise
using a 5.x kernel on hardware would take some time to set up.

Jim

Re: [RISC-V] Add support for AddressSanitizer on RISC-V GCC

[Jim Wilson]

On Tue, Aug 25, 2020 at 12:39 PM Jim Wilson <jimw@sifive.com> wrote:
> On Wed, Aug 19, 2020 at 1:02 AM Joshua via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >         * config/riscv/riscv.c (asan_shadow_offset): Implement the offset of asan shadow memory for risc-v.
> >         (asan_shadow_offset): new macro definition.
>
> When I try the patch, I get asan errors complaining about memory mappings.

I tried looking at this again today.  I spent a few hours debugging
sanitizer code to see what is going on, and managed to convince myself
that the patch can't possibly work for riscv64-linux.  There are at
least two changes missing.  It also can't possibly work for
riscv32-linux.  There is at least one change missing.
libsanitizer/configure.tgt only supports riscv64-linux, so nothing
gets built for riscv32-linux.  I have no idea how you got this
working.  Maybe you forgot to send the entire patch?  If there is a
way to get it working, it would be good if you could describe in
detail how you got it working.

For riscv64-linux with these additional patches

hifiveu017:1192$ more tmp.file
diff --git a/libsanitizer/asan/asan_mapping.h b/libsanitizer/asan/asan_mapping.h
index 09be904270c..906fb1eebc8 100644
--- a/libsanitizer/asan/asan_mapping.h
+++ b/libsanitizer/asan/asan_mapping.h
@@ -164,6 +164,7 @@ static const u64 kAArch64_ShadowOffset64 = 1ULL << 36;
 static const u64 kMIPS32_ShadowOffset32 = 0x0aaa0000;
 static const u64 kMIPS64_ShadowOffset64 = 1ULL << 37;
 static const u64 kPPC64_ShadowOffset64 = 1ULL << 41;
+static const u64 kRISCV64_ShadowOffset64 = 1ULL << 36;
 static const u64 kSystemZ_ShadowOffset64 = 1ULL << 52;
 static const u64 kSPARC64_ShadowOffset64 = 1ULL << 43;  // 0x80000000000
 static const u64 kFreeBSD_ShadowOffset32 = 1ULL << 30;  // 0x40000000
@@ -210,6 +211,8 @@ static const u64 kMyriadCacheBitMask32 = 0x40000000ULL;
 #    define SHADOW_OFFSET kAArch64_ShadowOffset64
 #  elif defined(__powerpc64__)
 #    define SHADOW_OFFSET kPPC64_ShadowOffset64
+#  elif defined(__riscv) && (__riscv_xlen == 64)
+#    define SHADOW_OFFSET kRISCV64_ShadowOffset64
 #  elif defined(__s390x__)
 #    define SHADOW_OFFSET kSystemZ_ShadowOffset64
 #  elif SANITIZER_FREEBSD
diff --git a/libsanitizer/sanitizer_common/sanitizer_linux.cpp b/libsanitizer/sa
nitizer_common/sanitizer_linux.cpp
index 11c03e286dc..962df07772e 100644
--- a/libsanitizer/sanitizer_common/sanitizer_linux.cpp
+++ b/libsanitizer/sanitizer_common/sanitizer_linux.cpp
@@ -1048,6 +1048,8 @@ uptr GetMaxVirtualAddress() {
   return (1ULL << (MostSignificantSetBitIndex(GET_CURRENT_FRAME()) + 1)) - 1;
 # elif defined(__mips64)
   return (1ULL << 40) - 1;  // 0x000000ffffffffffUL;
+# elif defined(__riscv) && (__riscv_xlen == 64)
+  return (1ULL << 39) - 1;  // 0x0000008000000000UL;
 # elif defined(__s390x__)
   return (1ULL << 53) - 1;  // 0x001fffffffffffffUL;
 #elif defined(__sparc__)
hifiveu017:1193$

I now get
==2936470==AddressSanitizer CHECK failed:
../../../../gcc-git/libsanitizer/sanitizer_common/sanitizer_allocator_primary64.h:76
"((kSpaceBeg)) == ((address_range.Init(TotalSpaceSize,
PrimaryAllocatorName, kSpaceBeg)))" (0x600000000000,
0xffffffffffffffff)
    <empty stack>
so there is still something missing but I think I am closer.

Jim

Re: [RISC-V] Add support for AddressSanitizer on RISC-V GCC

[Martin Liška]

On 8/19/20 11:25 AM, Kito Cheng via Gcc-patches wrote:
> Could you update that for RV32, and this patch will be pending until
> LLVM accepts the libsanitizer part.

Hello.

I've noticed that the libsanitizer part was accepted to LLVM. I'm planning to do
a merge upstream (for libsanitizer) and was planning to do so in 3 weeks.

I can do it earlier if it helps for the integration and testing purpose?

Thoughts?
Thanks,
Martin

Re: [RISC-V] Add support for AddressSanitizer on RISC-V GCC

[Kito Cheng]

Hi Martin:

> I can do it earlier if it helps for the integration and testing purpose?

I think it would be helpful! thanks!

On Fri, Oct 16, 2020 at 3:33 PM Martin Liška <mliska@suse.cz> wrote:
>
> On 8/19/20 11:25 AM, Kito Cheng via Gcc-patches wrote:
> > Could you update that for RV32, and this patch will be pending until
> > LLVM accepts the libsanitizer part.
>
> Hello.
>
> I've noticed that the libsanitizer part was accepted to LLVM. I'm planning to do
> a merge upstream (for libsanitizer) and was planning to do so in 3 weeks.
>
> I can do it earlier if it helps for the integration and testing purpose?
>
> Thoughts?
> Thanks,
> Martin

Re: [RISC-V] Add support for AddressSanitizer on RISC-V GCC

[Martin Liška]

On 10/16/20 9:45 AM, Kito Cheng wrote:
> I think it would be helpful! thanks!

Hello.

I've just installed the patches to master.

Martin

Re: [RISC-V] Add support for AddressSanitizer on RISC-V GCC

[Kito Cheng]

Hi Joshua, Jim:

> > +/* Implement TARGET_ASAN_SHADOW_OFFSET.  */
> > +
> > +static unsigned HOST_WIDE_INT
> > +riscv_asan_shadow_offset (void)
> > +{
> > +  return HOST_WIDE_INT_UC (0x10000000);
> > +}
>
> Is there a reason why you used 0x10000000?
>
> Looking at other targets, it appears the convention is 1<<29 for
> 32-bit targets, and a number larger than 1<<32 for 64-bit targets.  I
> think the RISC-V Linux port has a minimum of 39-bit virtual addresses
> (SV39) suggesting that this should be 1<<36 for 64-bit targets.  I can
> test the 32-bit support on qemu, and the 64-bit support on hardware,
> but my hardware is doing other stuff today.  I should be able to try
> testing this tomorrow.
>
> Otherwise the gcc stuff is pretty simple and looks OK.  We just need
> to double check these numbers.

Default offset is 1ULL << 44 for 64 bit target and 1ULL << 29 for 32
bit target in LLVM[1, 2],
I am not talking about we should use those values, just remind that we
should sync this offset value to LLVM :)

[1] https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp#L96
[2] https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/asan/asan_mapping.h#L159

Re: [RISC-V] Add support for AddressSanitizer on RISC-V GCC

[Jim Wilson]

On Thu, Jul 30, 2020 at 5:31 AM Joshua via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
> +/* Implement TARGET_ASAN_SHADOW_OFFSET.  */
> +
> +static unsigned HOST_WIDE_INT
> +riscv_asan_shadow_offset (void)
> +{
> +  return HOST_WIDE_INT_UC (0x10000000);
> +}

Is there a reason why you used 0x10000000?

Looking at other targets, it appears the convention is 1<<29 for
32-bit targets, and a number larger than 1<<32 for 64-bit targets.  I
think the RISC-V Linux port has a minimum of 39-bit virtual addresses
(SV39) suggesting that this should be 1<<36 for 64-bit targets.  I can
test the 32-bit support on qemu, and the 64-bit support on hardware,
but my hardware is doing other stuff today.  I should be able to try
testing this tomorrow.

Otherwise the gcc stuff is pretty simple and looks OK.  We just need
to double check these numbers.

> diff --git a/libsanitizer/sanitizer_common/sanitizer_common.h b/libsanitizer/sanitizer_common/sanitizer_common.h
> index ac16e0e..ea7dff7 100644
> --- a/libsanitizer/sanitizer_common/sanitizer_common.h
> +++ b/libsanitizer/sanitizer_common/sanitizer_common.h
> @@ -649,7 +649,8 @@ enum ModuleArch {
>    kModuleArchARMV7,
>    kModuleArchARMV7S,
>    kModuleArchARMV7K,
> -  kModuleArchARM64
> +  kModuleArchARM64,
> +  kModuleArchRISCV
>  };
>

Libsanitizer patches should go upstream and then be pulled down into
gcc.  I haven't done libsanitizer work before so I'm not sure of the
exact details.  I would expect to find the info here:
    https://gcc.gnu.org/codingconventions.html#upstream
but it doesn't mention libsanitizer.  I found the rules in the
libsanitizer/README.gcc and HOWTO_MERGE files.  As expected it says to
submit upstream first.

You are adding a SANITIZER_RISCV macro but not using it.  It isn't
clear why you need this, unless maybe it is just for completeness.  it
looks harmless though, and might be useful later.  This is something
for upstream reviewers to decide though.

In sanitizer_common.h I see a comment
// When adding a new architecture, don't forget to also update
// script/asan_symbolize.py and sanitizer_symbolizer_libcdep.cpp.
but I don't see any script/asan_symbolize.py file.  So maybe the
comment should be fixed.  Or if there is a file in the llvm tree that
we don't import into gcc, then you will need a patch for it.  if the
comment is wrong, then there is a similar comment in
sanitizer_symbolizer_libcdep.cpp that needs to be fixed too.  If the
file is gone, the comment fix can be a separate patch.

Otherwise this stuff looks pretty simple and obvious but it needs to
be submitted upstream.

Jim

Re: [RISC-V] Add support for AddressSanitizer on RISC-V GCC

[Jim Wilson]

On Thu, Jul 30, 2020 at 6:28 AM Martin Liška <mliska@suse.cz> wrote:
> What's the reason for sending the same patch multiple times
> from a different sender?

I see 3 in the gcc.gnu.org email archive, and I saw 3 on the NNTP feed
from gmane, but it seems only one of them ended up in my gmail inbox.
The other two appear to have problems with mail headers.  Maybe they
resent because of bounces.  Alibaba is fairly new to gcc development,
so I'd just chalk this up as newbies trying to get the procedure right
with tools that they aren't familiar with.  Few people still use email
the same way that we do for patches.

I am curious about the names though.  The first one was from "shaj
<shajun.sj@alibaba-inc.com>" and the last two are from "cooper.joshua
<cooper.joshua@linux.alibaba.com>".  If more than one person
contributed to the patch then it should include all names in the
changelog entry for correct attribution.

Jim

Re: [RISC-V] Add support for AddressSanitizer on RISC-V GCC

[Martin Liška]

Hello.

What's the reason for sending the same patch multiple times
from a different sender?

Thanks,
Martin

[RISC-V] Add support for AddressSanitizer on RISC-V GCC

[Joshua]

From: cooper.joshua <cooper.joshua@linux.alibaba.com>

    gcc/
    
        * config/riscv/riscv.c (asan_shadow_offset): Implement the offset of asan shadow memory for risc-v.
        (asan_shadow_offset): new macro definition.

    libsanitizer/
    
        * sanitizer_common/sanitizer_common.h (ModuleArch): New enumerator.
        (ModuleArchToString): New architecture option.
        * sanitizer_common/sanitizer_platform.h: New macro definition.
        * sanitizer_common/sanitizer_symbolizer_libcdep.cpp (GetArgV): New architecture option.
---
 gcc/config/riscv/riscv.c                                      | 11 +++++++++++
 libsanitizer/sanitizer_common/sanitizer_common.h              |  5 ++++-
 libsanitizer/sanitizer_common/sanitizer_platform.h            |  6 ++++++
 .../sanitizer_common/sanitizer_symbolizer_libcdep.cpp         |  2 ++
 4 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index bfb3885..05669c2 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -5245,6 +5245,14 @@ riscv_gpr_save_operation_p (rtx op)linux.alibaba
   return true;
 }
 
+/* Implement TARGET_ASAN_SHADOW_OFFSET.  */
+
+static unsigned HOST_WIDE_INT
+riscv_asan_shadow_offset (void)
+{
+  return HOST_WIDE_INT_UC (0x10000000);
+}
+
 /* Initialize the GCC target structure.  */
 #undef TARGET_ASM_ALIGNED_HI_OP
 #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
@@ -5428,6 +5436,9 @@ riscv_gpr_save_operation_p (rtx op)
 #undef TARGET_NEW_ADDRESS_PROFITABLE_P
 #define TARGET_NEW_ADDRESS_PROFITABLE_P riscv_new_address_profitable_p
 
+#undef TARGET_ASAN_SHADOW_OFFSET
+#define TARGET_ASAN_SHADOW_OFFSET riscv_asan_shadow_offset
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-riscv.h"
diff --git a/libsanitizer/sanitizer_common/sanitizer_common.h b/libsanitizer/sanitizer_common/sanitizer_common.h
index ac16e0e..ea7dff7 100644
--- a/libsanitizer/sanitizer_common/sanitizer_common.h
+++ b/libsanitizer/sanitizer_common/sanitizer_common.h
@@ -649,7 +649,8 @@ enum ModuleArch {
   kModuleArchARMV7,
   kModuleArchARMV7S,
   kModuleArchARMV7K,
-  kModuleArchARM64
+  kModuleArchARM64,
+  kModuleArchRISCV
 };
 
 // Opens the file 'file_name" and reads up to 'max_len' bytes.
@@ -693,6 +694,8 @@ inline const char *ModuleArchToString(ModuleArch arch) {
       return "armv7k";
     case kModuleArchARM64:
       return "arm64";
+    case kModuleArchRISCV:
+      return "riscv";
   }
   CHECK(0 && "Invalid module arch");
   return "";
diff --git a/libsanitizer/sanitizer_common/sanitizer_platform.h b/libsanitizer/sanitizer_common/sanitizer_platform.h
index c68bfa2..bf52490 100644
--- a/libsanitizer/sanitizer_common/sanitizer_platform.h
+++ b/libsanitizer/sanitizer_common/sanitizer_platform.h
@@ -126,6 +126,12 @@
 # define FIRST_32_SECOND_64(a, b) (a)
 #endif
 
+#if defined(__riscv__)
+# define SANITIZER_RISCV 1
+#else
+# define SANITIZER_RISCV 0
+#endif
+
 #if defined(__x86_64__) && !defined(_LP64)
 # define SANITIZER_X32 1
 #else
diff --git a/libsanitizer/sanitizer_common/sanitizer_symbolizer_libcdep.cpp b/libsanitizer/sanitizer_common/sanitizer_symbolizer_libcdep.cpp
index 490c6fe..408f57d 100644
--- a/libsanitizer/sanitizer_common/sanitizer_symbolizer_libcdep.cpp
+++ b/libsanitizer/sanitizer_common/sanitizer_symbolizer_libcdep.cpp
@@ -270,6 +270,8 @@ class LLVMSymbolizerProcess : public SymbolizerProcess {
     const char* const kSymbolizerArch = "--default-arch=s390x";
 #elif defined(__s390__)
     const char* const kSymbolizerArch = "--default-arch=s390";
+#elif defined(__riscv__)
+    const char* const kSymbolizerArch = "--default-arch=riscv";
 #else
     const char* const kSymbolizerArch = "--default-arch=unknown";
 #endif
-- 
2.7.4

[RISC-V] Add support for AddressSanitizer on RISC-V GCC

[D-C0211NB4-1017]

From: cooper.joshua <cooper.joshua@linux.alibaba.com>

    gcc/
    
        * config/riscv/riscv.c (asan_shadow_offset): Implement the offset of asan shadow memory for risc-v.
        (asan_shadow_offset): new macro definition.

    libsanitizer/
    
        * sanitizer_common/sanitizer_common.h (ModuleArch): New enumerator.
        (ModuleArchToString): New architecture option.
        * sanitizer_common/sanitizer_platform.h: New macro definition.
        * sanitizer_common/sanitizer_symbolizer_libcdep.cpp (GetArgV): New architecture option.
---
 gcc/config/riscv/riscv.c                                      | 11 +++++++++++
 libsanitizer/sanitizer_common/sanitizer_common.h              |  5 ++++-
 libsanitizer/sanitizer_common/sanitizer_platform.h            |  6 ++++++
 .../sanitizer_common/sanitizer_symbolizer_libcdep.cpp         |  2 ++
 4 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index bfb3885..05669c2 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -5245,6 +5245,14 @@ riscv_gpr_save_operation_p (rtx op)linux.alibaba
   return true;
 }
 
+/* Implement TARGET_ASAN_SHADOW_OFFSET.  */
+
+static unsigned HOST_WIDE_INT
+riscv_asan_shadow_offset (void)
+{
+  return HOST_WIDE_INT_UC (0x10000000);
+}
+
 /* Initialize the GCC target structure.  */
 #undef TARGET_ASM_ALIGNED_HI_OP
 #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
@@ -5428,6 +5436,9 @@ riscv_gpr_save_operation_p (rtx op)
 #undef TARGET_NEW_ADDRESS_PROFITABLE_P
 #define TARGET_NEW_ADDRESS_PROFITABLE_P riscv_new_address_profitable_p
 
+#undef TARGET_ASAN_SHADOW_OFFSET
+#define TARGET_ASAN_SHADOW_OFFSET riscv_asan_shadow_offset
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-riscv.h"
diff --git a/libsanitizer/sanitizer_common/sanitizer_common.h b/libsanitizer/sanitizer_common/sanitizer_common.h
index ac16e0e..ea7dff7 100644
--- a/libsanitizer/sanitizer_common/sanitizer_common.h
+++ b/libsanitizer/sanitizer_common/sanitizer_common.h
@@ -649,7 +649,8 @@ enum ModuleArch {
   kModuleArchARMV7,
   kModuleArchARMV7S,
   kModuleArchARMV7K,
-  kModuleArchARM64
+  kModuleArchARM64,
+  kModuleArchRISCV
 };
 
 // Opens the file 'file_name" and reads up to 'max_len' bytes.
@@ -693,6 +694,8 @@ inline const char *ModuleArchToString(ModuleArch arch) {
       return "armv7k";
     case kModuleArchARM64:
       return "arm64";
+    case kModuleArchRISCV:
+      return "riscv";
   }
   CHECK(0 && "Invalid module arch");
   return "";
diff --git a/libsanitizer/sanitizer_common/sanitizer_platform.h b/libsanitizer/sanitizer_common/sanitizer_platform.h
index c68bfa2..bf52490 100644
--- a/libsanitizer/sanitizer_common/sanitizer_platform.h
+++ b/libsanitizer/sanitizer_common/sanitizer_platform.h
@@ -126,6 +126,12 @@
 # define FIRST_32_SECOND_64(a, b) (a)
 #endif
 
+#if defined(__riscv__)
+# define SANITIZER_RISCV 1
+#else
+# define SANITIZER_RISCV 0
+#endif
+
 #if defined(__x86_64__) && !defined(_LP64)
 # define SANITIZER_X32 1
 #else
diff --git a/libsanitizer/sanitizer_common/sanitizer_symbolizer_libcdep.cpp b/libsanitizer/sanitizer_common/sanitizer_symbolizer_libcdep.cpp
index 490c6fe..408f57d 100644
--- a/libsanitizer/sanitizer_common/sanitizer_symbolizer_libcdep.cpp
+++ b/libsanitizer/sanitizer_common/sanitizer_symbolizer_libcdep.cpp
@@ -270,6 +270,8 @@ class LLVMSymbolizerProcess : public SymbolizerProcess {
     const char* const kSymbolizerArch = "--default-arch=s390x";
 #elif defined(__s390__)
     const char* const kSymbolizerArch = "--default-arch=s390";
+#elif defined(__riscv__)
+    const char* const kSymbolizerArch = "--default-arch=riscv";
 #else
     const char* const kSymbolizerArch = "--default-arch=unknown";
 #endif
-- 
2.7.4

[RISC-V] Add support for AddressSanitizer on RISC-V GCC

[D-C0211NB4-1017]

From: shaj <shajun.sj@alibaba-inc.com>

    gcc/
    
        * config/riscv/riscv.c (asan_shadow_offset): Implement the offset of asan shadow memory for risc-v.
        (asan_shadow_offset): new macro definition.

    libsanitizer/
    
        * sanitizer_common/sanitizer_common.h (ModuleArch): New enumerator.
        (ModuleArchToString): New architecture option.
        * sanitizer_common/sanitizer_platform.h: New macro definition.
        * sanitizer_common/sanitizer_symbolizer_libcdep.cpp (GetArgV): New architecture option.
---
 gcc/config/riscv/riscv.c                                      | 11 +++++++++++
 libsanitizer/sanitizer_common/sanitizer_common.h              |  5 ++++-
 libsanitizer/sanitizer_common/sanitizer_platform.h            |  6 ++++++
 .../sanitizer_common/sanitizer_symbolizer_libcdep.cpp         |  2 ++
 4 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index bfb3885..05669c2 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -5245,6 +5245,14 @@ riscv_gpr_save_operation_p (rtx op)
   return true;
 }
 
+/* Implement TARGET_ASAN_SHADOW_OFFSET.  */
+
+static unsigned HOST_WIDE_INT
+riscv_asan_shadow_offset (void)
+{
+  return HOST_WIDE_INT_UC (0x10000000);
+}
+
 /* Initialize the GCC target structure.  */
 #undef TARGET_ASM_ALIGNED_HI_OP
 #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
@@ -5428,6 +5436,9 @@ riscv_gpr_save_operation_p (rtx op)
 #undef TARGET_NEW_ADDRESS_PROFITABLE_P
 #define TARGET_NEW_ADDRESS_PROFITABLE_P riscv_new_address_profitable_p
 
+#undef TARGET_ASAN_SHADOW_OFFSET
+#define TARGET_ASAN_SHADOW_OFFSET riscv_asan_shadow_offset
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-riscv.h"
diff --git a/libsanitizer/sanitizer_common/sanitizer_common.h b/libsanitizer/sanitizer_common/sanitizer_common.h
index ac16e0e..ea7dff7 100644
--- a/libsanitizer/sanitizer_common/sanitizer_common.h
+++ b/libsanitizer/sanitizer_common/sanitizer_common.h
@@ -649,7 +649,8 @@ enum ModuleArch {
   kModuleArchARMV7,
   kModuleArchARMV7S,
   kModuleArchARMV7K,
-  kModuleArchARM64
+  kModuleArchARM64,
+  kModuleArchRISCV
 };
 
 // Opens the file 'file_name" and reads up to 'max_len' bytes.
@@ -693,6 +694,8 @@ inline const char *ModuleArchToString(ModuleArch arch) {
       return "armv7k";
     case kModuleArchARM64:
       return "arm64";
+    case kModuleArchRISCV:
+      return "riscv";
   }
   CHECK(0 && "Invalid module arch");
   return "";
diff --git a/libsanitizer/sanitizer_common/sanitizer_platform.h b/libsanitizer/sanitizer_common/sanitizer_platform.h
index c68bfa2..bf52490 100644
--- a/libsanitizer/sanitizer_common/sanitizer_platform.h
+++ b/libsanitizer/sanitizer_common/sanitizer_platform.h
@@ -126,6 +126,12 @@
 # define FIRST_32_SECOND_64(a, b) (a)
 #endif
 
+#if defined(__riscv__)
+# define SANITIZER_RISCV 1
+#else
+# define SANITIZER_RISCV 0
+#endif
+
 #if defined(__x86_64__) && !defined(_LP64)
 # define SANITIZER_X32 1
 #else
diff --git a/libsanitizer/sanitizer_common/sanitizer_symbolizer_libcdep.cpp b/libsanitizer/sanitizer_common/sanitizer_symbolizer_libcdep.cpp
index 490c6fe..408f57d 100644
--- a/libsanitizer/sanitizer_common/sanitizer_symbolizer_libcdep.cpp
+++ b/libsanitizer/sanitizer_common/sanitizer_symbolizer_libcdep.cpp
@@ -270,6 +270,8 @@ class LLVMSymbolizerProcess : public SymbolizerProcess {
     const char* const kSymbolizerArch = "--default-arch=s390x";
 #elif defined(__s390__)
     const char* const kSymbolizerArch = "--default-arch=s390";
+#elif defined(__riscv__)
+    const char* const kSymbolizerArch = "--default-arch=riscv";
 #else
     const char* const kSymbolizerArch = "--default-arch=unknown";
 #endif
-- 
2.7.4