public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
@ 2012-11-13 13:41 Dominique Dhumieres
  2012-11-13 15:04 ` Dodji Seketeli
  0 siblings, 1 reply; 12+ messages in thread
From: Dominique Dhumieres @ 2012-11-13 13:41 UTC (permalink / raw)
  To: gcc-patches; +Cc: wmi, davidxl, dnovillo, hans-peter.nilsson, jakub, dodji

> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
  2012-11-13 13:41 Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it Dominique Dhumieres
@ 2012-11-13 15:04 ` Dodji Seketeli
  2012-11-13 15:09   ` Hans-Peter Nilsson
  0 siblings, 1 reply; 12+ messages in thread
From: Dodji Seketeli @ 2012-11-13 15:04 UTC (permalink / raw)
  To: Dominique Dhumieres
  Cc: gcc-patches, wmi, davidxl, dnovillo, hans-peter.nilsson, jakub

dominiq@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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
  2012-11-13 15:04 ` Dodji Seketeli
@ 2012-11-13 15:09   ` Hans-Peter Nilsson
  0 siblings, 0 replies; 12+ messages in thread
From: Hans-Peter Nilsson @ 2012-11-13 15:09 UTC (permalink / raw)
  To: dodji; +Cc: dominiq, gcc-patches, wmi, davidxl, dnovillo, jakub

> From: Dodji Seketeli <dodji@redhat.com>
> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Committed: framework bits for disabling libsanitizer.  RFC on which targets for which to disable it.
  2012-11-14  1:34         ` Richard Henderson
@ 2012-11-14  2:47           ` Hans-Peter Nilsson
  0 siblings, 0 replies; 12+ messages in thread
From: Hans-Peter Nilsson @ 2012-11-14  2:47 UTC (permalink / raw)
  To: rth; +Cc: gcc-patches, jakub, dodji, dnovillo, davidxl, wmi

> From: Richard Henderson <rth@redhat.com>
> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Committed: framework bits for disabling libsanitizer.  RFC on which targets for which to disable it.
  2012-11-14  1:20       ` Hans-Peter Nilsson
@ 2012-11-14  1:34         ` Richard Henderson
  2012-11-14  2:47           ` Hans-Peter Nilsson
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Henderson @ 2012-11-14  1:34 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: gcc-patches, jakub, dodji, dnovillo, davidxl, wmi

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~

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Committed: framework bits for disabling libsanitizer.  RFC on which targets for which to disable it.
  2012-11-13 20:38     ` Richard Henderson
@ 2012-11-14  1:20       ` Hans-Peter Nilsson
  2012-11-14  1:34         ` Richard Henderson
  0 siblings, 1 reply; 12+ messages in thread
From: Hans-Peter Nilsson @ 2012-11-14  1:20 UTC (permalink / raw)
  To: gcc-patches; +Cc: jakub, dodji, dnovillo, davidxl, wmi, rth

> From: Richard Henderson <rth@redhat.com>
> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Committed: framework bits for disabling libsanitizer.  RFC on which targets for which to disable it.
  2012-11-13 13:24   ` Jakub Jelinek
  2012-11-13 14:07     ` Dodji Seketeli
@ 2012-11-13 20:38     ` Richard Henderson
  2012-11-14  1:20       ` Hans-Peter Nilsson
  1 sibling, 1 reply; 12+ messages in thread
From: Richard Henderson @ 2012-11-13 20:38 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Dodji Seketeli, Hans-Peter Nilsson, gcc-patches, Diego Novillo,
	Xinliang David Li, Wei Mi

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~

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Committed: framework bits for disabling libsanitizer.  RFC on which targets for which to disable it.
  2012-11-13 14:07     ` Dodji Seketeli
@ 2012-11-13 14:53       ` Jack Howarth
  0 siblings, 0 replies; 12+ messages in thread
From: Jack Howarth @ 2012-11-13 14:53 UTC (permalink / raw)
  To: Dodji Seketeli
  Cc: Jakub Jelinek, Hans-Peter Nilsson, gcc-patches, Diego Novillo,
	Xinliang David Li, Wei Mi

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Committed: framework bits for disabling libsanitizer.  RFC on which targets for which to disable it.
  2012-11-13 13:24   ` Jakub Jelinek
@ 2012-11-13 14:07     ` Dodji Seketeli
  2012-11-13 14:53       ` Jack Howarth
  2012-11-13 20:38     ` Richard Henderson
  1 sibling, 1 reply; 12+ messages in thread
From: Dodji Seketeli @ 2012-11-13 14:07 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Hans-Peter Nilsson, gcc-patches, Diego Novillo,
	Xinliang David Li, Wei Mi

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Committed: framework bits for disabling libsanitizer.  RFC on which targets for which to disable it.
  2012-11-13 13:18 ` Dodji Seketeli
@ 2012-11-13 13:24   ` Jakub Jelinek
  2012-11-13 14:07     ` Dodji Seketeli
  2012-11-13 20:38     ` Richard Henderson
  0 siblings, 2 replies; 12+ messages in thread
From: Jakub Jelinek @ 2012-11-13 13:24 UTC (permalink / raw)
  To: Dodji Seketeli
  Cc: Hans-Peter Nilsson, gcc-patches, Diego Novillo,
	Xinliang David Li, Wei Mi

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Committed: framework bits for disabling libsanitizer.  RFC on which targets for which to disable it.
  2012-11-13  1:38 Hans-Peter Nilsson
@ 2012-11-13 13:18 ` Dodji Seketeli
  2012-11-13 13:24   ` Jakub Jelinek
  0 siblings, 1 reply; 12+ messages in thread
From: Dodji Seketeli @ 2012-11-13 13:18 UTC (permalink / raw)
  To: Hans-Peter Nilsson
  Cc: gcc-patches, Jakub Jelinek, Diego Novillo, Xinliang David Li, Wei Mi

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
> +

[...]

-- 
		Dodji

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Committed: framework bits for disabling libsanitizer.  RFC on which targets for which to disable it.
@ 2012-11-13  1:38 Hans-Peter Nilsson
  2012-11-13 13:18 ` Dodji Seketeli
  0 siblings, 1 reply; 12+ messages in thread
From: Hans-Peter Nilsson @ 2012-11-13  1:38 UTC (permalink / raw)
  To: gcc-patches

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-11-14  2:47 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-13 13:41 Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it Dominique Dhumieres
2012-11-13 15:04 ` Dodji Seketeli
2012-11-13 15:09   ` Hans-Peter Nilsson
  -- strict thread matches above, loose matches on Subject: below --
2012-11-13  1:38 Hans-Peter Nilsson
2012-11-13 13:18 ` Dodji Seketeli
2012-11-13 13:24   ` Jakub Jelinek
2012-11-13 14:07     ` Dodji Seketeli
2012-11-13 14:53       ` Jack Howarth
2012-11-13 20:38     ` Richard Henderson
2012-11-14  1:20       ` Hans-Peter Nilsson
2012-11-14  1:34         ` Richard Henderson
2012-11-14  2:47           ` Hans-Peter Nilsson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).