| Message ID | CAKdteObkDtD99dihWAwZiAszRW2Qxpr=mA+bkc79Wp8V2Kmgyg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
+eugenis@google.com Hi Christophe, On Thu, Mar 28, 2013 at 2:09 AM, Christophe Lyon <christophe.lyon@linaro.org> wrote: > Hi, > This small patch enables libsanitizer on ARM. > It has been tested successfully on cortex-a9 hardware (via the GCC testsuite). > > I have chosen to bundle -funwind-table with -fsanitize=* so that a > useful backtrace can be printed to the user in case of error, > otherwise the reporting is limited to one line belonging to > libsanitizer.so. > > Note that the testsuite currently fails when executing under qemu: > - support of /proc/self/maps does not conform to the kernel format. > One extra space is missing from some lines, which confuses libsanitizer. > Patch proposed to upstream qemu: > http://lists.gnu.org/archive/html/qemu-devel/2013-02/msg03051.html As we discussed in https://code.google.com/p/address-sanitizer/issues/detail?id=160 this may be fixed in libsanitizer, although i'd still prefer the qemu fix. > > - qemu reserves some memory space by default, conflicting with > libsanitizer needs. > Workaround: invoke qemu with -R 0 Good to know! > > - libsanitizer detects if its output is a tty, and when GCC testsuite > is executed under qemu, libsanitizer concludes that it is actually > running under a tty, and adds beautyfying characters which confuse > dejanu. Is this again a quemu problem? Or should we do some more checks before emitting color codes? A comment about this patch and a question to Evgeniy: on Android/ARM we use zero shadow offset. (code.google.com/p/address-sanitizer/wiki/ZeroBasedShadow) Can we do it on other ARM targets too? --kcc > > > OK? > > Christophe. > > 2013-03-27 Christophe Lyon <christophe.lyon@linaro.org> > > gcc/ > * config/arm/arm.c (arm_asan_shadow_offset): New function. > (TARGET_ASAN_SHADOW_OFFSET): Define. > * config/arm/linux-eabi.h (ASAN_CC1_SPEC): Define. > (LINUX_OR_ANDROID_CC): Add ASAN_CC1_SPEC. > > libsanitizer/ > * configure.tgt: Add ARM pattern.
On Thu, Mar 28, 2013 at 11:36 AM, Konstantin Serebryany <konstantin.s.serebryany@gmail.com> wrote: > +eugenis@google.com > > Hi Christophe, > > On Thu, Mar 28, 2013 at 2:09 AM, Christophe Lyon > <christophe.lyon@linaro.org> wrote: >> Hi, >> This small patch enables libsanitizer on ARM. >> It has been tested successfully on cortex-a9 hardware (via the GCC testsuite). >> >> I have chosen to bundle -funwind-table with -fsanitize=* so that a >> useful backtrace can be printed to the user in case of error, >> otherwise the reporting is limited to one line belonging to >> libsanitizer.so. >> >> Note that the testsuite currently fails when executing under qemu: >> - support of /proc/self/maps does not conform to the kernel format. >> One extra space is missing from some lines, which confuses libsanitizer. >> Patch proposed to upstream qemu: >> http://lists.gnu.org/archive/html/qemu-devel/2013-02/msg03051.html > > As we discussed in > https://code.google.com/p/address-sanitizer/issues/detail?id=160 > this may be fixed in libsanitizer, although i'd still prefer the qemu fix. > >> >> - qemu reserves some memory space by default, conflicting with >> libsanitizer needs. >> Workaround: invoke qemu with -R 0 > > Good to know! > >> >> - libsanitizer detects if its output is a tty, and when GCC testsuite >> is executed under qemu, libsanitizer concludes that it is actually >> running under a tty, and adds beautyfying characters which confuse >> dejanu. > > Is this again a quemu problem? > Or should we do some more checks before emitting color codes? > > A comment about this patch and a question to Evgeniy: > on Android/ARM we use zero shadow offset. > (code.google.com/p/address-sanitizer/wiki/ZeroBasedShadow) > Can we do it on other ARM targets too? We do it because newer versions of Android use PIE binaries, and, combined with other specifics of address space on Linux/ARM, there is no space for ASan shadow anywhere else. And it's faster. Zero-based shadow requires PIE. Non-zero-based requires non-PIE on Android. Is it the same with QEMU? If so, we should switch to zero-based for uniformity and performance. > > --kcc > >> >> >> OK? >> >> Christophe. >> >> 2013-03-27 Christophe Lyon <christophe.lyon@linaro.org> >> >> gcc/ >> * config/arm/arm.c (arm_asan_shadow_offset): New function. >> (TARGET_ASAN_SHADOW_OFFSET): Define. >> * config/arm/linux-eabi.h (ASAN_CC1_SPEC): Define. >> (LINUX_OR_ANDROID_CC): Add ASAN_CC1_SPEC. >> >> libsanitizer/ >> * configure.tgt: Add ARM pattern.
On Thu, Mar 28, 2013 at 12:00:23PM +0400, Evgeniy Stepanov wrote: > We do it because newer versions of Android use PIE binaries, and, > combined with other specifics of address space on Linux/ARM, there is > no space for ASan shadow anywhere else. And it's faster. > > Zero-based shadow requires PIE. Non-zero-based requires non-PIE on > Android. Is it the same with QEMU? If so, we should switch to > zero-based for uniformity and performance. I don't think most of the arm-linux-gnueabi binaries are PIEs, so using zero shadow offset would be wrong on Linux. If 1 << 29 works (e.g. prelink library area on linux-arm is 0x41000000 .. 0x50000000, so shadow of 0x20000000 .. 0x3fffffff is fine for that), IMHO we should use it. Jakub
On Thu, Mar 28, 2013 at 12:07 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Thu, Mar 28, 2013 at 12:00:23PM +0400, Evgeniy Stepanov wrote: >> We do it because newer versions of Android use PIE binaries, and, >> combined with other specifics of address space on Linux/ARM, there is >> no space for ASan shadow anywhere else. And it's faster. >> >> Zero-based shadow requires PIE. Non-zero-based requires non-PIE on >> Android. Is it the same with QEMU? If so, we should switch to >> zero-based for uniformity and performance. > > I don't think most of the arm-linux-gnueabi binaries are PIEs, so using > zero shadow offset would be wrong on Linux. If 1 << 29 works (e.g. prelink > library area on linux-arm is 0x41000000 .. 0x50000000, so > shadow of 0x20000000 .. 0x3fffffff is fine for that), IMHO we should use it. Do we need two separate offsets for Linux/ARM and Android/ARM? That's what we have in clang today. --kcc > > Jakub
>> Note that the testsuite currently fails when executing under qemu: >> - support of /proc/self/maps does not conform to the kernel format. >> One extra space is missing from some lines, which confuses libsanitizer. >> Patch proposed to upstream qemu: >> http://lists.gnu.org/archive/html/qemu-devel/2013-02/msg03051.html > > As we discussed in > https://code.google.com/p/address-sanitizer/issues/detail?id=160 > this may be fixed in libsanitizer, although i'd still prefer the qemu fix. > I'll ping the qemu list. >> >> - libsanitizer detects if its output is a tty, and when GCC testsuite >> is executed under qemu, libsanitizer concludes that it is actually >> running under a tty, and adds beautyfying characters which confuse >> dejanu. > > Is this again a quemu problem? I still don't know. I tried to investigate some time ago; I thought it could be a problem when qemu interprets a ~isatty syscall, but IIRC this syscall isn't used. So I don't know who finally asnwers to the isatty() query. Christophe.
~/src/qemu/qemu-git/arm-linux-user/qemu-arm -cpu cortex-a9 -R 0 -L /home/lyon/src/GCC/builds/gcc-fsf-asan-arm-none-linux-gnueabihf/sysroot ./heap-overflow-1.arm On 28 March 2013 15:33, Christophe Lyon <christophe.lyon@linaro.org> wrote: >>> - libsanitizer detects if its output is a tty, and when GCC testsuite >>> is executed under qemu, libsanitizer concludes that it is actually >>> running under a tty, and adds beautyfying characters which confuse >>> dejanu. >> >> Is this again a quemu problem? > I still don't know. I tried to investigate some time ago; I thought it > could be a problem when qemu interprets a ~isatty syscall, but IIRC > this syscall isn't used. So I don't know who finally asnwers to the > isatty() query. > After a bit more debugging, the libsanitizer query isatty(2) is handled by glibc as a call to __tcgetattr(2,...), which is turned into ioctl(2, TCGETS,...). Qemu issues the same query to the host, and I have observed that when called by expect, qemu's fd 2 points to /dev/pts/XXX which explains why it thinks it is a tty. So far I have haven't look at what actually happens when executed on a board, but why would expect behave differently? Christophe.
On 4 April 2013 14:19, Christophe Lyon <christophe.lyon@linaro.org> wrote: > ~/src/qemu/qemu-git/arm-linux-user/qemu-arm -cpu cortex-a9 -R 0 -L > /home/lyon/src/GCC/builds/gcc-fsf-asan-arm-none-linux-gnueabihf/sysroot > ./heap-overflow-1.arm > > On 28 March 2013 15:33, Christophe Lyon <christophe.lyon@linaro.org> wrote: >>>> - libsanitizer detects if its output is a tty, and when GCC testsuite >>>> is executed under qemu, libsanitizer concludes that it is actually >>>> running under a tty, and adds beautyfying characters which confuse >>>> dejanu. >>> >>> Is this again a quemu problem? >> I still don't know. I tried to investigate some time ago; I thought it >> could be a problem when qemu interprets a ~isatty syscall, but IIRC >> this syscall isn't used. So I don't know who finally asnwers to the >> isatty() query. >> > > After a bit more debugging, the libsanitizer query isatty(2) is > handled by glibc as a call to __tcgetattr(2,...), which is turned into > ioctl(2, TCGETS,...). > > Qemu issues the same query to the host, and I have observed that when > called by expect, qemu's fd 2 points to /dev/pts/XXX which explains > why it thinks it is a tty. > > So far I have haven't look at what actually happens when executed on a > board, but why would expect behave differently? > After debugging on ARM hardware, I noticed that in this latter case, runtest uses "unix" as target, and uses pipes to communicate with the test program, while it creates ptys when using a simulator. By adding the "readonly" flag in sim_spawn (/usr/share/dejagnu/config/sim.exp) I managed to have these tests pass under qemu, but I don't know if this is safe or if there is a better way of achieving the same effect. However, doing this makes other tests fail "harder": the tests involving clone() fail when run under qemu, but when the "readonly" flag is set, qemu sometimes fail in timeout rather than exiting with an error code. Threads in general are not well supported by qemu (there are other random failures in GCC testsuite related to this). So.... maybe it would be better to skip asan tests when running under qemu: is the GCC testsuite aware of being run under qemu? Thanks for any suggestion. Christophe.
On 18 April 2013 11:30, Christophe Lyon <christophe.lyon@linaro.org> wrote: > On 4 April 2013 14:19, Christophe Lyon <christophe.lyon@linaro.org> wrote: >> ~/src/qemu/qemu-git/arm-linux-user/qemu-arm -cpu cortex-a9 -R 0 -L >> /home/lyon/src/GCC/builds/gcc-fsf-asan-arm-none-linux-gnueabihf/sysroot >> ./heap-overflow-1.arm >> >> On 28 March 2013 15:33, Christophe Lyon <christophe.lyon@linaro.org> wrote: >>>>> - libsanitizer detects if its output is a tty, and when GCC testsuite >>>>> is executed under qemu, libsanitizer concludes that it is actually >>>>> running under a tty, and adds beautyfying characters which confuse >>>>> dejanu. >>>> >>>> Is this again a quemu problem? >>> I still don't know. I tried to investigate some time ago; I thought it >>> could be a problem when qemu interprets a ~isatty syscall, but IIRC >>> this syscall isn't used. So I don't know who finally asnwers to the >>> isatty() query. >>> >> >> After a bit more debugging, the libsanitizer query isatty(2) is >> handled by glibc as a call to __tcgetattr(2,...), which is turned into >> ioctl(2, TCGETS,...). >> >> Qemu issues the same query to the host, and I have observed that when >> called by expect, qemu's fd 2 points to /dev/pts/XXX which explains >> why it thinks it is a tty. >> >> So far I have haven't look at what actually happens when executed on a >> board, but why would expect behave differently? >> > > After debugging on ARM hardware, I noticed that in this latter case, > runtest uses "unix" as target, and uses pipes to communicate with the > test program, while it creates ptys when using a simulator. > By adding the "readonly" flag in sim_spawn > (/usr/share/dejagnu/config/sim.exp) I managed to have these tests pass > under qemu, but I don't know if this is safe or if there is a better > way of achieving the same effect. > > However, doing this makes other tests fail "harder": the tests > involving clone() fail when run under qemu, but when the "readonly" > flag is set, qemu sometimes fail in timeout rather than exiting with > an error code. Threads in general are not well supported by qemu > (there are other random failures in GCC testsuite related to this). > > So.... maybe it would be better to skip asan tests when running under > qemu: is the GCC testsuite aware of being run under qemu? > > Thanks for any suggestion. > Replying to myself: I have added a new check_effective_target_hw function in the testsuite, to enable to skip tests when running under a simulator. My concern is that in this particular case, it's a problem with a particular simulator (qemu-user), not simulators in general (ie it's not a speed problem). So, should/could I add another variable that people would set when using qemu? For the record, we have to skip clone-test-1.c and rlimit-mmap-test-1.c because qemu isn't currently able to run them (whatever the target). Regarding the isatty(2) problem discussed earlier, I can workaround it by modifying the regexps in dg-output clauses such as: -/* { dg-output "AddressSanitizer can not provide additional info.*(\n|\r\n|\r)" } */ +/* { dg-output ".*AddressSanitizer can not provide additional info.*(\n|\r\n|\r)" } */ What do people think about this proposal? Christophe.
On Fri, Apr 19, 2013 at 7:59 AM, Christophe Lyon <christophe.lyon@linaro.org> wrote: > On 18 April 2013 11:30, Christophe Lyon <christophe.lyon@linaro.org> wrote: >> On 4 April 2013 14:19, Christophe Lyon <christophe.lyon@linaro.org> wrote: >>> ~/src/qemu/qemu-git/arm-linux-user/qemu-arm -cpu cortex-a9 -R 0 -L >>> /home/lyon/src/GCC/builds/gcc-fsf-asan-arm-none-linux-gnueabihf/sysroot >>> ./heap-overflow-1.arm >>> >>> On 28 March 2013 15:33, Christophe Lyon <christophe.lyon@linaro.org> wrote: >>>>>> - libsanitizer detects if its output is a tty, and when GCC testsuite >>>>>> is executed under qemu, libsanitizer concludes that it is actually >>>>>> running under a tty, and adds beautyfying characters which confuse >>>>>> dejanu. >>>>> >>>>> Is this again a quemu problem? >>>> I still don't know. I tried to investigate some time ago; I thought it >>>> could be a problem when qemu interprets a ~isatty syscall, but IIRC >>>> this syscall isn't used. So I don't know who finally asnwers to the >>>> isatty() query. >>>> >>> >>> After a bit more debugging, the libsanitizer query isatty(2) is >>> handled by glibc as a call to __tcgetattr(2,...), which is turned into >>> ioctl(2, TCGETS,...). >>> >>> Qemu issues the same query to the host, and I have observed that when >>> called by expect, qemu's fd 2 points to /dev/pts/XXX which explains >>> why it thinks it is a tty. >>> >>> So far I have haven't look at what actually happens when executed on a >>> board, but why would expect behave differently? >>> >> >> After debugging on ARM hardware, I noticed that in this latter case, >> runtest uses "unix" as target, and uses pipes to communicate with the >> test program, while it creates ptys when using a simulator. >> By adding the "readonly" flag in sim_spawn >> (/usr/share/dejagnu/config/sim.exp) I managed to have these tests pass >> under qemu, but I don't know if this is safe or if there is a better >> way of achieving the same effect. >> >> However, doing this makes other tests fail "harder": the tests >> involving clone() fail when run under qemu, but when the "readonly" >> flag is set, qemu sometimes fail in timeout rather than exiting with >> an error code. Threads in general are not well supported by qemu >> (there are other random failures in GCC testsuite related to this). >> >> So.... maybe it would be better to skip asan tests when running under >> qemu: is the GCC testsuite aware of being run under qemu? >> >> Thanks for any suggestion. >> > > Replying to myself: > I have added a new check_effective_target_hw function in the > testsuite, to enable to skip tests when running under a simulator. > My concern is that in this particular case, it's a problem with a > particular simulator (qemu-user), not simulators in general (ie it's > not a speed problem). > > So, should/could I add another variable that people would set when using qemu? > > For the record, we have to skip clone-test-1.c and > rlimit-mmap-test-1.c because qemu isn't currently able to run them > (whatever the target). > > Regarding the isatty(2) problem discussed earlier, I can workaround it For the issue with colors: Evgeniy, you probably want to implement something like ASAN_OPTIONS=color=0 to fight https://code.google.com/p/address-sanitizer/issues/detail?id=174 This will also help with quemu. > by modifying the regexps in dg-output clauses such as: > -/* { dg-output "AddressSanitizer can not provide additional > info.*(\n|\r\n|\r)" } */ > +/* { dg-output ".*AddressSanitizer can not provide additional > info.*(\n|\r\n|\r)" } */ > > What do people think about this proposal? > > Christophe.
=== modified file 'gcc/config/arm/arm.c' --- gcc/config/arm/arm.c 2013-02-28 10:26:41 +0000 +++ gcc/config/arm/arm.c 2013-03-04 08:39:02 +0000 @@ -280,6 +280,8 @@ static void arm_canonicalize_comparison (int *code, rtx *op0, rtx *op1, bool op0_preserve_value); +static unsigned HOST_WIDE_INT arm_asan_shadow_offset (void); + /* Table of machine attributes. */ static const struct attribute_spec arm_attribute_table[] = @@ -649,6 +651,9 @@ #define TARGET_CANONICALIZE_COMPARISON \ arm_canonicalize_comparison +#undef TARGET_ASAN_SHADOW_OFFSET +#define TARGET_ASAN_SHADOW_OFFSET arm_asan_shadow_offset + struct gcc_target targetm = TARGET_INITIALIZER; /* Obstack for minipool constant handling. */ @@ -27393,4 +27398,12 @@ } +/* Implement the TARGET_ASAN_SHADOW_OFFSET hook. */ + +static unsigned HOST_WIDE_INT +arm_asan_shadow_offset (void) +{ + return (unsigned HOST_WIDE_INT) 1 << 29; +} + #include "gt-arm.h" === modified file 'gcc/config/arm/linux-eabi.h' --- gcc/config/arm/linux-eabi.h 2013-01-10 20:38:27 +0000 +++ gcc/config/arm/linux-eabi.h 2013-03-26 09:59:11 +0000 @@ -84,10 +84,14 @@ LINUX_OR_ANDROID_LD (LINUX_TARGET_LINK_SPEC, \ LINUX_TARGET_LINK_SPEC " " ANDROID_LINK_SPEC) +#undef ASAN_CC1_SPEC +#define ASAN_CC1_SPEC "%{fsanitize=*:-funwind-tables}" + #undef CC1_SPEC #define CC1_SPEC \ - LINUX_OR_ANDROID_CC (GNU_USER_TARGET_CC1_SPEC, \ - GNU_USER_TARGET_CC1_SPEC " " ANDROID_CC1_SPEC) + LINUX_OR_ANDROID_CC (GNU_USER_TARGET_CC1_SPEC " " ASAN_CC1_SPEC, \ + GNU_USER_TARGET_CC1_SPEC " " ASAN_CC1_SPEC " " \ + ANDROID_CC1_SPEC) #define CC1PLUS_SPEC \ LINUX_OR_ANDROID_CC ("", ANDROID_CC1PLUS_SPEC) === modified file 'libsanitizer/configure.tgt' --- libsanitizer/configure.tgt 2013-02-11 23:13:37 +0000 +++ libsanitizer/configure.tgt 2013-03-04 08:39:02 +0000 @@ -29,6 +29,8 @@ ;; sparc*-*-linux*) ;; + arm*-*-linux*) + ;; x86_64-*-darwin[1]* | i?86-*-darwin[1]*) TSAN_SUPPORTED=no ;;
Hi, This small patch enables libsanitizer on ARM. It has been tested successfully on cortex-a9 hardware (via the GCC testsuite). I have chosen to bundle -funwind-table with -fsanitize=* so that a useful backtrace can be printed to the user in case of error, otherwise the reporting is limited to one line belonging to libsanitizer.so. Note that the testsuite currently fails when executing under qemu: - support of /proc/self/maps does not conform to the kernel format. One extra space is missing from some lines, which confuses libsanitizer. Patch proposed to upstream qemu: http://lists.gnu.org/archive/html/qemu-devel/2013-02/msg03051.html - qemu reserves some memory space by default, conflicting with libsanitizer needs. Workaround: invoke qemu with -R 0 - libsanitizer detects if its output is a tty, and when GCC testsuite is executed under qemu, libsanitizer concludes that it is actually running under a tty, and adds beautyfying characters which confuse dejanu. OK? Christophe. 2013-03-27 Christophe Lyon <christophe.lyon@linaro.org> gcc/ * config/arm/arm.c (arm_asan_shadow_offset): New function. (TARGET_ASAN_SHADOW_OFFSET): Define. * config/arm/linux-eabi.h (ASAN_CC1_SPEC): Define. (LINUX_OR_ANDROID_CC): Add ASAN_CC1_SPEC. libsanitizer/ * configure.tgt: Add ARM pattern.