| Message ID | 20170123203629.GS1867@tucnak |
|---|---|
| State | New |
| Headers | show |
Hi! On Mon, Jan 23, 2017 at 09:36:29PM +0100, Jakub Jelinek wrote: > So, I've bootstrapped/regtested s390x-linux (64-bit only, don't have 32-bit > userland around anymore to test 31-bit) with the attached patch (and on top > of the PR79168 patch I'll post soon) and the > only regressions I got are: > FAIL: c-c++-common/asan/null-deref-1.c {-O2,-O2 -flto -fno-use-linker-plugin -flto-partition=none,-O2 -flto -fuse-linker-plugin -fno-fat-lto-objects,-O3 -g,-Os} output pattern test > FAIL: g++.dg/asan/deep-stack-uaf-1.C {-O0,-O1,-O2,-O3 -g,-Os} output pattern test > FAIL: c-c++-common/ubsan/overflow-vec-1.c {-O0,-O1,-O2,-O2 -flto -fno-use-linker-plugin -flto-partition=none,-O2 -flto -fuse-linker-plugin -fno-fat-lto-objects,-O3 -g,-Os} execution test > FAIL: c-c++-common/ubsan/overflow-vec-2.c {-O0,-O1,-O2,-O2 -flto -fno-use-linker-plugin -flto-partition=none,-O2 -flto -fuse-linker-plugin -fno-fat-lto-objects,-O3 -g,-Os} execution test > All but deep-stack-uaf-1.C in both check-gcc and check-g++. > > In null-deref-1.c it seems the problem is in the line for the deref, > the testcase is expecting runtime error on line 10, while > #0 0x80000a6d in NullDeref c-c++-common/asan/null-deref-1.c:11 > #1 0x800008f1 in main c-c++-common/asan/null-deref-1.c:15 > #2 0x3ff93022c5f in __libc_start_main (/lib64/libc.so.6+0x22c5f) > #3 0x8000096d (gcc-7.0.1-20170120/obj-s390x-redhat-linux/gcc/testsuite/g++/null-deref-1.exe+0x8000096d) > is reported. > The second test fails > ERROR: AddressSanitizer: heap-use-after-free on address 0x615000000205 at pc 0x000080000b12 bp 0x03fff8378928 sp 0x03fff8378918 > READ of size 1 at 0x615000000205 thread T0 > #0 0x80000b11 in main g++.dg/asan/deep-stack-uaf-1.C:33 > #1 0x3ffabe22c5f in __libc_start_main (/lib64/libc.so.6+0x22c5f) > #2 0x800009cd (gcc-7.0.1-20170120/obj-s390x-redhat-linux/gcc/testsuite/g++/deep-stack-uaf-1.exe+0x800009cd) > will need to debug if we don't need to add further options on s390x to > make sure it has all the frames it is expecting. > The last 2 tests aren't really asan related, will debug. > > Note apparently asan_test.C isn't enabled on non-i?86/x86_64, which > is a big mistake, will try to change it separately. I'd like to ping this patch, since then bootstrapped/regtested again several times on s390x-linux. asan_test.C is since then enabled on all architectures and passes with the patch (lots of small tests), the overflow-vec-*.c tests fail even on powerpc*-linux, so I think the port is in quite good shape and for feature parity it would be nice to have this feature on s390x-linux. > 2017-01-23 Jakub Jelinek <jakub@redhat.com> > > gcc/ > * config/s390/s390.c (s390_asan_shadow_offset): New function. > (TARGET_ASAN_SHADOW_OFFSET): Redefine. > libsanitizer/ > * configure.tgt: Enable asan and ubsan on 64-bit s390*-*-linux*. > > --- gcc/config/s390/s390.c.jj 2017-01-19 16:58:25.000000000 +0100 > +++ gcc/config/s390/s390.c 2017-01-23 16:32:28.220398187 +0100 > @@ -15435,6 +15435,14 @@ s390_excess_precision (enum excess_preci > return FLT_EVAL_METHOD_UNPREDICTABLE; > } > > +/* Implement the TARGET_ASAN_SHADOW_OFFSET hook. */ > + > +static unsigned HOST_WIDE_INT > +s390_asan_shadow_offset (void) > +{ > + return TARGET_64BIT ? HOST_WIDE_INT_1U << 52 : HOST_WIDE_INT_UC (0x20000000); > +} > + > /* Initialize GCC target structure. */ > > #undef TARGET_ASM_ALIGNED_HI_OP > @@ -15536,6 +15544,8 @@ s390_excess_precision (enum excess_preci > #define TARGET_BUILD_BUILTIN_VA_LIST s390_build_builtin_va_list > #undef TARGET_EXPAND_BUILTIN_VA_START > #define TARGET_EXPAND_BUILTIN_VA_START s390_va_start > +#undef TARGET_ASAN_SHADOW_OFFSET > +#define TARGET_ASAN_SHADOW_OFFSET s390_asan_shadow_offset > #undef TARGET_GIMPLIFY_VA_ARG_EXPR > #define TARGET_GIMPLIFY_VA_ARG_EXPR s390_gimplify_va_arg > > --- libsanitizer/configure.tgt.jj 2017-01-23 15:25:21.000000000 +0100 > +++ libsanitizer/configure.tgt 2017-01-23 15:36:40.787456320 +0100 > @@ -39,6 +39,11 @@ case "${target}" in > ;; > sparc*-*-linux*) > ;; > + s390*-*-linux*) > + if test x$ac_cv_sizeof_void_p = x4; then > + UNSUPPORTED=1 > + fi > + ;; > arm*-*-linux*) > ;; > aarch64*-*-linux*) > Jakub
On 01/30/2017 10:31 PM, Jakub Jelinek wrote: > Hi! > > On Mon, Jan 23, 2017 at 09:36:29PM +0100, Jakub Jelinek wrote: >> So, I've bootstrapped/regtested s390x-linux (64-bit only, don't have 32-bit >> userland around anymore to test 31-bit) with the attached patch (and on top >> of the PR79168 patch I'll post soon) and the >> only regressions I got are: >> FAIL: c-c++-common/asan/null-deref-1.c {-O2,-O2 -flto -fno-use-linker-plugin -flto-partition=none,-O2 -flto -fuse-linker-plugin -fno-fat-lto-objects,-O3 -g,-Os} output pattern test >> FAIL: g++.dg/asan/deep-stack-uaf-1.C {-O0,-O1,-O2,-O3 -g,-Os} output pattern test >> FAIL: c-c++-common/ubsan/overflow-vec-1.c {-O0,-O1,-O2,-O2 -flto -fno-use-linker-plugin -flto-partition=none,-O2 -flto -fuse-linker-plugin -fno-fat-lto-objects,-O3 -g,-Os} execution test >> FAIL: c-c++-common/ubsan/overflow-vec-2.c {-O0,-O1,-O2,-O2 -flto -fno-use-linker-plugin -flto-partition=none,-O2 -flto -fuse-linker-plugin -fno-fat-lto-objects,-O3 -g,-Os} execution test >> All but deep-stack-uaf-1.C in both check-gcc and check-g++. >> >> In null-deref-1.c it seems the problem is in the line for the deref, >> the testcase is expecting runtime error on line 10, while >> #0 0x80000a6d in NullDeref c-c++-common/asan/null-deref-1.c:11 >> #1 0x800008f1 in main c-c++-common/asan/null-deref-1.c:15 >> #2 0x3ff93022c5f in __libc_start_main (/lib64/libc.so.6+0x22c5f) >> #3 0x8000096d (gcc-7.0.1-20170120/obj-s390x-redhat-linux/gcc/testsuite/g++/null-deref-1.exe+0x8000096d) >> is reported. >> The second test fails >> ERROR: AddressSanitizer: heap-use-after-free on address 0x615000000205 at pc 0x000080000b12 bp 0x03fff8378928 sp 0x03fff8378918 >> READ of size 1 at 0x615000000205 thread T0 >> #0 0x80000b11 in main g++.dg/asan/deep-stack-uaf-1.C:33 >> #1 0x3ffabe22c5f in __libc_start_main (/lib64/libc.so.6+0x22c5f) >> #2 0x800009cd (gcc-7.0.1-20170120/obj-s390x-redhat-linux/gcc/testsuite/g++/deep-stack-uaf-1.exe+0x800009cd) >> will need to debug if we don't need to add further options on s390x to >> make sure it has all the frames it is expecting. >> The last 2 tests aren't really asan related, will debug. >> >> Note apparently asan_test.C isn't enabled on non-i?86/x86_64, which >> is a big mistake, will try to change it separately. > > I'd like to ping this patch, since then bootstrapped/regtested again > several times on s390x-linux. asan_test.C is since then enabled > on all architectures and passes with the patch (lots of small tests), > the overflow-vec-*.c tests fail even on powerpc*-linux, so I think the > port is in quite good shape and for feature parity it would be nice to > have this feature on s390x-linux. > >> 2017-01-23 Jakub Jelinek <jakub@redhat.com> >> >> gcc/ >> * config/s390/s390.c (s390_asan_shadow_offset): New function. >> (TARGET_ASAN_SHADOW_OFFSET): Redefine. >> libsanitizer/ >> * configure.tgt: Enable asan and ubsan on 64-bit s390*-*-linux*. Ok. Thanks! >> +/* Implement the TARGET_ASAN_SHADOW_OFFSET hook. */ >> + >> +static unsigned HOST_WIDE_INT >> +s390_asan_shadow_offset (void) >> +{ >> + return TARGET_64BIT ? HOST_WIDE_INT_1U << 52 : HOST_WIDE_INT_UC (0x20000000); >> +} These values probably come from the LLVM implementation?! The 64 bit offset appears to reside beyond the default task size which is 4TB (1<<42). So it will trigger the task to be upgraded to an additional level of page tables. Sounds reasonable in order to avoid collisions for most of the cases. The task size in that case will end up as 1<<53 so the shadow map should end up near to the top of it. The 32 bit value should not be used since 32 bit is disabled - right? This appears to be the default value for 32 bit targets. I'm not sure if we will have adjust it since we only have 31 bit address space. >> + >> /* Initialize GCC target structure. */ >> >> #undef TARGET_ASM_ALIGNED_HI_OP >> @@ -15536,6 +15544,8 @@ s390_excess_precision (enum excess_preci >> #define TARGET_BUILD_BUILTIN_VA_LIST s390_build_builtin_va_list >> #undef TARGET_EXPAND_BUILTIN_VA_START >> #define TARGET_EXPAND_BUILTIN_VA_START s390_va_start >> +#undef TARGET_ASAN_SHADOW_OFFSET >> +#define TARGET_ASAN_SHADOW_OFFSET s390_asan_shadow_offset >> #undef TARGET_GIMPLIFY_VA_ARG_EXPR >> #define TARGET_GIMPLIFY_VA_ARG_EXPR s390_gimplify_va_arg >> >> --- libsanitizer/configure.tgt.jj 2017-01-23 15:25:21.000000000 +0100 >> +++ libsanitizer/configure.tgt 2017-01-23 15:36:40.787456320 +0100 >> @@ -39,6 +39,11 @@ case "${target}" in >> ;; >> sparc*-*-linux*) >> ;; >> + s390*-*-linux*) >> + if test x$ac_cv_sizeof_void_p = x4; then >> + UNSUPPORTED=1 >> + fi >> + ;; >> arm*-*-linux*) >> ;; >> aarch64*-*-linux*) >> > > Jakub >
On Tue, Jan 31, 2017 at 01:15:20PM +0100, Andreas Krebbel wrote: > >> 2017-01-23 Jakub Jelinek <jakub@redhat.com> > >> > >> gcc/ > >> * config/s390/s390.c (s390_asan_shadow_offset): New function. > >> (TARGET_ASAN_SHADOW_OFFSET): Redefine. > >> libsanitizer/ > >> * configure.tgt: Enable asan and ubsan on 64-bit s390*-*-linux*. > > Ok. Thanks! > > >> +/* Implement the TARGET_ASAN_SHADOW_OFFSET hook. */ > >> + > >> +static unsigned HOST_WIDE_INT > >> +s390_asan_shadow_offset (void) > >> +{ > >> + return TARGET_64BIT ? HOST_WIDE_INT_1U << 52 : HOST_WIDE_INT_UC (0x20000000); > >> +} > > These values probably come from the LLVM implementation?! > The 64 bit offset appears to reside beyond the default task size which is 4TB (1<<42). So it will > trigger the task to be upgraded to an additional level of page tables. Sounds reasonable in order to > avoid collisions for most of the cases. The task size in that case will end up as 1<<53 so the > shadow map should end up near to the top of it. > The 32 bit value should not be used since 32 bit is disabled - right? This appears to be the default > value for 32 bit targets. I'm not sure if we will have adjust it since we only have 31 bit address > space. Both values reflect on what is right now in libsanitizer/asan/asan_mapping.h (the library has to agree with the compiler on the shift). That header has for s390{,x}: // Default Linux/S390 mapping: // || `[0x30000000, 0x7fffffff]` || HighMem || // || `[0x26000000, 0x2fffffff]` || HighShadow || // || `[0x24000000, 0x25ffffff]` || ShadowGap || // || `[0x20000000, 0x23ffffff]` || LowShadow || // || `[0x00000000, 0x1fffffff]` || LowMem || // // Default Linux/SystemZ mapping: // || `[0x14000000000000, 0x1fffffffffffff]` || HighMem || // || `[0x12800000000000, 0x13ffffffffffff]` || HighShadow || // || `[0x12000000000000, 0x127fffffffffff]` || ShadowGap || // || `[0x10000000000000, 0x11ffffffffffff]` || LowShadow || // || `[0x00000000000000, 0x0fffffffffffff]` || LowMem || ... static const u64 kDefaultShadowOffset32 = 1ULL << 29; // 0x20000000 ... static const u64 kSystemZ_ShadowOffset64 = 1ULL << 52; ... #if SANITIZER_WORDSIZE == 32 ... # else # define SHADOW_OFFSET kDefaultShadowOffset32 # endif #else ... # elif defined(__s390x__) # define SHADOW_OFFSET kSystemZ_ShadowOffset64 ... If you think some of those aren't appropriate for s390 64-bit or 32-bit, then it would need to be changed in upstream too. The if: > >> --- libsanitizer/configure.tgt.jj 2017-01-23 15:25:21.000000000 +0100 > >> +++ libsanitizer/configure.tgt 2017-01-23 15:36:40.787456320 +0100 > >> @@ -39,6 +39,11 @@ case "${target}" in > >> ;; > >> sparc*-*-linux*) > >> ;; > >> + s390*-*-linux*) > >> + if test x$ac_cv_sizeof_void_p = x4; then > >> + UNSUPPORTED=1 > >> + fi > >> + ;; part is purely because in Fedora s390 31-bit is not available anymore (only 64-bit s390x), so I can't test that. If you could test it somewhere and verify even 31-bit works fine, then the if test x$ac_cv_sizeof_void_p = x4; then UNSUPPORTED=1 fi lines could be removed. Jakub
--- gcc/config/s390/s390.c.jj 2017-01-19 16:58:25.000000000 +0100 +++ gcc/config/s390/s390.c 2017-01-23 16:32:28.220398187 +0100 @@ -15435,6 +15435,14 @@ s390_excess_precision (enum excess_preci return FLT_EVAL_METHOD_UNPREDICTABLE; } +/* Implement the TARGET_ASAN_SHADOW_OFFSET hook. */ + +static unsigned HOST_WIDE_INT +s390_asan_shadow_offset (void) +{ + return TARGET_64BIT ? HOST_WIDE_INT_1U << 52 : HOST_WIDE_INT_UC (0x20000000); +} + /* Initialize GCC target structure. */ #undef TARGET_ASM_ALIGNED_HI_OP @@ -15536,6 +15544,8 @@ s390_excess_precision (enum excess_preci #define TARGET_BUILD_BUILTIN_VA_LIST s390_build_builtin_va_list #undef TARGET_EXPAND_BUILTIN_VA_START #define TARGET_EXPAND_BUILTIN_VA_START s390_va_start +#undef TARGET_ASAN_SHADOW_OFFSET +#define TARGET_ASAN_SHADOW_OFFSET s390_asan_shadow_offset #undef TARGET_GIMPLIFY_VA_ARG_EXPR #define TARGET_GIMPLIFY_VA_ARG_EXPR s390_gimplify_va_arg --- libsanitizer/configure.tgt.jj 2017-01-23 15:25:21.000000000 +0100 +++ libsanitizer/configure.tgt 2017-01-23 15:36:40.787456320 +0100 @@ -39,6 +39,11 @@ case "${target}" in ;; sparc*-*-linux*) ;; + s390*-*-linux*) + if test x$ac_cv_sizeof_void_p = x4; then + UNSUPPORTED=1 + fi + ;; arm*-*-linux*) ;; aarch64*-*-linux*)