| Message ID | 201211130138.qAD1cMWL013966@ignucius.se.axis.com |
|---|---|
| State | New |
| Headers | show |
Hans-Peter Nilsson <hans-peter.nilsson@axis.com> a écrit: > While the fallout(*) from the libsanitizer commit is handled, > it's obvious it should have a noconfigdirs= section in > toplevel/configure.ac like the other target libs. Here's what I > committed after observing that a cris-elf build passed, where it > previously failed building libsanitizer which wrongly tries to > compile using -fPIC despite checking in its configure.ac IIUC. Thank you for doing this. > But, I'd like to update the target contents there to something a > bit more generic. > > Maybe disable libsanitizer everywhere except for (referring to > the three common target-related file-name parts in libsanitizer) > "mac", "win" and "linux"? Or disable everywhere except x86_64 / > i386? That's the only port that defines the required > TARGET_ASAN_SHADOW_OFFSET. FWIW, I'd think we should just disable it everywhere except on x86_64 / i*86 for now, as these are the only targets that define TARGET_ASAN_SHADOW_OFFSET. What do the maintainers think? [...] > Index: configure.ac > =================================================================== > --- configure.ac (revision 193455) > +++ configure.ac (working copy) > @@ -547,6 +547,13 @@ case "${target}" in > ;; > esac > > +# Disable libsanitizer for some systems. > +case "${target}" in > + cris-*-* | crisv32-*-* | mmix-*-*) > + noconfigdirs="$noconfigdirs target-libsanitizer" > + ;; > +esac > + [...]
On Tue, Nov 13, 2012 at 02:17:55PM +0100, Dodji Seketeli wrote: > What do the maintainers think? Yes. And it shouldn't be just based on target CPU, but also based on target OS, I don't think libsanitizer supports anything but linux (glibc + maybe android) right now, with some smaller or bigger tweaks it could support darwin (but see the reports that it doesn't build there right now) or mingw/cygwin? (but there is a PR that it doesn't build there). So IMHO it should be a whitelist of supported targets with *) case adding noconfigdirs="$noconfigdirs target-libsanitizer", rather than blacklist of few unsupported ones. Can you please prepare a patch? > > --- configure.ac (revision 193455) > > +++ configure.ac (working copy) > > @@ -547,6 +547,13 @@ case "${target}" in > > ;; > > esac > > > > +# Disable libsanitizer for some systems. > > +case "${target}" in > > + cris-*-* | crisv32-*-* | mmix-*-*) > > + noconfigdirs="$noconfigdirs target-libsanitizer" > > + ;; > > +esac > > + Jakub
Jakub Jelinek <jakub@redhat.com> writes: > On Tue, Nov 13, 2012 at 02:17:55PM +0100, Dodji Seketeli wrote: >> What do the maintainers think? > > Yes. And it shouldn't be just based on target CPU, but also based > on target OS, I don't think libsanitizer supports anything but linux (glibc > + maybe android) right now, with some smaller or bigger tweaks it could > support darwin (but see the reports that it doesn't build there right now) > or mingw/cygwin? (but there is a PR that it doesn't build there). > So IMHO it should be a whitelist of supported targets with *) case > adding noconfigdirs="$noconfigdirs target-libsanitizer", rather than > blacklist of few unsupported ones. Can you please prepare a patch? Sure, will do.
On Tue, Nov 13, 2012 at 03:06:56PM +0100, Dodji Seketeli wrote: > Jakub Jelinek <jakub@redhat.com> writes: > > > On Tue, Nov 13, 2012 at 02:17:55PM +0100, Dodji Seketeli wrote: > >> What do the maintainers think? > > > > Yes. And it shouldn't be just based on target CPU, but also based > > on target OS, I don't think libsanitizer supports anything but linux (glibc > > + maybe android) right now, with some smaller or bigger tweaks it could > > support darwin (but see the reports that it doesn't build there right now) > > or mingw/cygwin? (but there is a PR that it doesn't build there). > > So IMHO it should be a whitelist of supported targets with *) case > > adding noconfigdirs="$noconfigdirs target-libsanitizer", rather than > > blacklist of few unsupported ones. Can you please prepare a patch? > > Sure, will do. > > -- > Dodji Dodji, I don't think darwin is very far from having usable asan support. Basically we need the following changes... 1) The missing libsanitizer/interception/mach_override subdirectory with the files... LICENSE.TXT Makefile.mk README.txt mach_override.c mach_override.h needs to be imported from compiler-rt's git. 2) In libsanitizer/interception, mach_override/mach_override.c needs to be added to interception_files and in libsanitizer/asan, the resulting object code in libsanitizer/interception/mach_override/mach_override.o needs to be linked into libasan.a and libasan.dylib. 3) In gcc/config/darwin.h, we need to add an entry to LINK_COMMAND_SPEC_A for faddress-sanitizer to automatically pass -framework CoreFoundation -lasan to the linker. Jack
On 11/13/2012 05:24 AM, Jakub Jelinek wrote: > Yes. And it shouldn't be just based on target CPU, but also based > on target OS, I don't think libsanitizer supports anything but linux (glibc > + maybe android) right now, with some smaller or bigger tweaks it could > support darwin (but see the reports that it doesn't build there right now) > or mingw/cygwin? (but there is a PR that it doesn't build there). > So IMHO it should be a whitelist of supported targets with *) case > adding noconfigdirs="$noconfigdirs target-libsanitizer", rather than > blacklist of few unsupported ones. Can you please prepare a patch? See how libatomic and libitm are structured. The logic for what targets are supported belongs inside the library directory, and not at top-level. Add a configure.tgt script with that knowledge. r~
Index: configure.ac =================================================================== --- configure.ac (revision 193455) +++ configure.ac (working copy) @@ -547,6 +547,13 @@ case "${target}" in ;; esac +# Disable libsanitizer for some systems. +case "${target}" in + cris-*-* | crisv32-*-* | mmix-*-*) + noconfigdirs="$noconfigdirs target-libsanitizer" + ;; +esac + # Disable libssp for some systems. case "${target}" in avr-*-*) Index: configure =================================================================== --- configure (revision 193455) +++ configure (working copy) @@ -3205,6 +3205,13 @@ case "${target}" in ;; esac +# Disable libsanitizer for some systems. +case "${target}" in + cris-*-* | crisv32-*-* | mmix-*-*) + noconfigdirs="$noconfigdirs target-libsanitizer" + ;; +esac + # Disable libssp for some systems. case "${target}" in avr-*-*)