Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
> From: Richard Henderson > Date: Wed, 14 Nov 2012 02:34:32 +0100 > On 11/13/2012 05:20 PM, Hans-Peter Nilsson wrote: > > Right. And, I think it's worth to repeat ;) that IMHO best to > > there simply check that -faddress-sanitizer can compile > > error-free (i.e. that TARGET_ASAN_SHADOW_OFFSET is defined on > > the target). No target lists needed. > > We can't do that, since the compiler hasn't been built > at top-level configure time. Obviously. Yah, I was referring to configure-time in libsanitizer ...but that's obviously not what you meant; I now see you referred to toplevel configure.ac including various subdir/configure.tgt at (toplevel) configure-time. Delaying bail-out to target-library configure-time would probably require things like a dummy buildsubdir/Makefile. I don't see at a glance other libraries doing that so might not be worth pursuing. Bah. brgds, H-P
Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
On 11/13/2012 05:20 PM, Hans-Peter Nilsson wrote: > Right. And, I think it's worth to repeat ;) that IMHO best to > there simply check that -faddress-sanitizer can compile > error-free (i.e. that TARGET_ASAN_SHADOW_OFFSET is defined on > the target). No target lists needed. We can't do that, since the compiler hasn't been built at top-level configure time. Obviously. r~
Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
> From: Richard Henderson > Date: Tue, 13 Nov 2012 21:38:40 +0100 > 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. Right. And, I think it's worth to repeat ;) that IMHO best to there simply check that -faddress-sanitizer can compile error-free (i.e. that TARGET_ASAN_SHADOW_OFFSET is defined on the target). No target lists needed. brgds, H-P
Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
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~
Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
> From: Dodji Seketeli > Date: Tue, 13 Nov 2012 16:04:12 +0100 > I guess when the issue of the missing files is resolved, we can enable > building libsanitizer on Darwin proper. Here is the patchlet I am > proposing so far http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00993.html. That's the direction I suggested, thanks for resolving this. brgds, H-P
Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
domi...@lps.ens.fr (Dominique Dhumieres) writes: >> 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) >> ... > > Importing the missing files allows the bootstrap to complete. > However I did not manage so far to make it working on darwin10: > see http://gcc.gnu.org/ml/gcc-bugs/2012-11/msg01145.html . > > I don't think disabling libsanitizer is an emergency for darwin. I guess when the issue of the missing files is resolved, we can enable building libsanitizer on Darwin proper. Here is the patchlet I am proposing so far http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00993.html. -- Dodji
Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
On Tue, Nov 13, 2012 at 03:06:56PM +0100, Dodji Seketeli wrote: > Jakub Jelinek 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
Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
Jakub Jelinek 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
Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
> 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) > ... Importing the missing files allows the bootstrap to complete. However I did not manage so far to make it working on darwin10: see http://gcc.gnu.org/ml/gcc-bugs/2012-11/msg01145.html . I don't think disabling libsanitizer is an emergency for darwin. Cheers, Dominique
Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
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
Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
Hans-Peter Nilsson 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
> +
[...]
--
Dodji
Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
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.
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. Maybe the library configure.ac
should check whether using -fsanitizer is error-free and disable
the libsanitizer build automatically?
toplevel:
* configure.ac: Add section for noconfigdirs for libsanitizer.
Disable for cris-*-* and mmix-*-*.
* configure: Regenerate.
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-*-*)
*) -fPIC passed when building libsanitizer for targets that
don't support it, lack of multilib setup for libsanitizer,
libsanitizer not installed in version-specific subdir...
Basically, IMHO it should just copy the generic
libgfortran/configure.ac and be done with it. Right now it has
the smallest configure.ac of the target-libraries.
brgds, H-P
