| Message ID | 1354746852.4743.16.camel@otta |
|---|---|
| State | New |
| Headers | show |
On Wed, Dec 5, 2012 at 5:34 PM, Peter Bergner <bergner@vnet.ibm.com> wrote: > Below are the port changes to enable building ASAN on powerpc*-linux. > The libsanitizer changes required for powerpc*-linux have already been > committed. This passes bootstrap and regtesting with no regressions > and we also pass the two ASAN test suite test cases. > > Ok for mainline? > > Peter > > > libsanitizer/ > * configure.tgt: Enable build on powerpc*-linux. > > gcc/ > * config/rs6000/rs6000.c (TARGET_ASAN_SHADOW_OFFSET): Define. > (rs6000_asan_shadow_offset): New function. > * config/rs6000/rs6000.h (FRAME_GROWS_DOWNWARD): Define to > flag_stack_protect != 0 || flag_asan != 0. The definition of TARGET_ASAN_SHADOW_OFFSET and rs6000_asan_shadow_offset need to be limited to TARGET_ELF or ! TARGET_XCOFF (Does it work on PPC BSD or PPC Darwin?). That macro determines if GCC thinks -fsanitze=address is supported, which it is not on AIX. The macro probably should be defined in sysv4.h, not rs6000.c, and the function should not be defined appropriately. Thanks, David
On Thu, 2012-12-06 at 09:39 -0500, David Edelsohn wrote: > On Wed, Dec 5, 2012 at 5:34 PM, Peter Bergner <bergner@vnet.ibm.com> wrote: > > * config/rs6000/rs6000.c (TARGET_ASAN_SHADOW_OFFSET): Define. > > (rs6000_asan_shadow_offset): New function. > > * config/rs6000/rs6000.h (FRAME_GROWS_DOWNWARD): Define to > > flag_stack_protect != 0 || flag_asan != 0. > > The definition of TARGET_ASAN_SHADOW_OFFSET and > rs6000_asan_shadow_offset need to be limited to TARGET_ELF or ! > TARGET_XCOFF (Does it work on PPC BSD or PPC Darwin?). That's a good question. Having no way to test that myself, I think limiting it to TARGET_ELF for now is the right solution and if someone wants it to work on BSD or Darwin, they can make the requisite changes and test it. > That macro determines if GCC thinks -fsanitze=address is supported, > which it is not on AIX. The macro probably should be defined in sysv4.h, > not rs6000.c, and the function should not be defined appropriately. Ok, I can move the macro there. And by the function being defined appropriately, do you mean wrapping it in a: #ifdef TARGET_ASAN_SHADOW_OFFSET or ??? Peter
On Thu, Dec 06, 2012 at 08:46:20AM -0600, Peter Bergner wrote: > On Thu, 2012-12-06 at 09:39 -0500, David Edelsohn wrote: > > On Wed, Dec 5, 2012 at 5:34 PM, Peter Bergner <bergner@vnet.ibm.com> wrote: > > > * config/rs6000/rs6000.c (TARGET_ASAN_SHADOW_OFFSET): Define. > > > (rs6000_asan_shadow_offset): New function. > > > * config/rs6000/rs6000.h (FRAME_GROWS_DOWNWARD): Define to > > > flag_stack_protect != 0 || flag_asan != 0. > > > > The definition of TARGET_ASAN_SHADOW_OFFSET and > > rs6000_asan_shadow_offset need to be limited to TARGET_ELF or ! > > TARGET_XCOFF (Does it work on PPC BSD or PPC Darwin?). > > That's a good question. Having no way to test that myself, I > think limiting it to TARGET_ELF for now is the right solution > and if someone wants it to work on BSD or Darwin, they can > make the requisite changes and test it. On i?86/x86_64 it is provided for all targets, but only some of the targets build libsanitizer then. Either users get errors at compile time (missing compiler support), or at link time (missing library support). I think the compiler side should error out if the compiler can't support it for some reason, if the only reason is missing library support, you should get a link error. > > That macro determines if GCC thinks -fsanitze=address is supported, > > which it is not on AIX. The macro probably should be defined in sysv4.h, > > not rs6000.c, and the function should not be defined appropriately. > > Ok, I can move the macro there. And by the function being defined > appropriately, do you mean wrapping it in a: > > #ifdef TARGET_ASAN_SHADOW_OFFSET > > or ??? You can also provide the function always, and set targetm.asan_shadow_offset = NULL; for AIX/etc. in some option post-processing target function (somewhere before process_options will try to error out on missing asan support). Jakub
On Thu, Dec 6, 2012 at 9:46 AM, Peter Bergner <bergner@vnet.ibm.com> wrote: >> That macro determines if GCC thinks -fsanitze=address is supported, >> which it is not on AIX. The macro probably should be defined in sysv4.h, >> not rs6000.c, and the function should not be defined appropriately. > > Ok, I can move the macro there. And by the function being defined > appropriately, do you mean wrapping it in a: > > #ifdef TARGET_ASAN_SHADOW_OFFSET I meant wrapping the code in #if TARGET_ELF or #if !TARGET_XCOFF or whatever is appropriate for where it is supported. Thanks, David
On Thu, Dec 6, 2012 at 9:53 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On i?86/x86_64 it is provided for all targets, but only some of the targets > build libsanitizer then. Either users get errors at compile time (missing > compiler support), or at link time (missing library support). > I think the compiler side should error out if the compiler can't support it > for some reason, if the only reason is missing library support, you should > get a link error. This is my concern. If the library support is not present, I think it is a worse user experience to fail with a link error because that does not communicate to the user if the feature is supported or not. The library could be missing because it was not installed correctly or some user error. And I think that it annoys the user to progress most of the way through the build and fail later. If the compiler can know that the feature is not supported, it should inform the user early and with a message directly from the compiler. The details of the reason for lack of support are of interest to the GCC developer, not the end user. Thanks, David
Index: libsanitizer/configure.tgt =================================================================== --- libsanitizer/configure.tgt (revision 194226) +++ libsanitizer/configure.tgt (working copy) @@ -25,6 +25,8 @@ case "${target}" in TSAN_SUPPORTED=yes fi ;; + powerpc*-*-linux*) + ;; sparc*-*-linux*) ;; x86_64-*-darwin* | i?86-*-darwin*) Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 194226) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -1187,6 +1187,9 @@ static const struct attribute_spec rs600 #undef TARGET_DELEGITIMIZE_ADDRESS #define TARGET_DELEGITIMIZE_ADDRESS rs6000_delegitimize_address +#undef TARGET_ASAN_SHADOW_OFFSET +#define TARGET_ASAN_SHADOW_OFFSET rs6000_asan_shadow_offset + #undef TARGET_CONST_NOT_OK_FOR_DEBUG_P #define TARGET_CONST_NOT_OK_FOR_DEBUG_P rs6000_const_not_ok_for_debug_p @@ -27521,6 +27524,13 @@ rs6000_final_prescan_insn (rtx insn, rtx } } +/* Implement the TARGET_ASAN_SHADOW_OFFSET hook. */ + +static unsigned HOST_WIDE_INT +rs6000_asan_shadow_offset (void) +{ + return (unsigned HOST_WIDE_INT) 1 << (TARGET_64BIT ? 41 : 29); +} /* Mask options that we want to support inside of attribute((target)) and #pragma GCC target operations. Note, we do not include things like Index: gcc/config/rs6000/rs6000.h =================================================================== --- gcc/config/rs6000/rs6000.h (revision 194226) +++ gcc/config/rs6000/rs6000.h (working copy) @@ -1406,7 +1406,7 @@ extern enum reg_class rs6000_constraints On the RS/6000, we grow upwards, from the area after the outgoing arguments. */ -#define FRAME_GROWS_DOWNWARD (flag_stack_protect != 0) +#define FRAME_GROWS_DOWNWARD (flag_stack_protect != 0 || flag_asan != 0) /* Size of the outgoing register save area */ #define RS6000_REG_SAVE ((DEFAULT_ABI == ABI_AIX \