| Message ID | 20150216171531.GC51560@msticlxl57.ims.intel.com |
|---|---|
| State | New |
| Headers | show |
Ping 2015-02-16 20:15 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>: > On 16 Feb 17:01, Jakub Jelinek wrote: >> On Mon, Feb 16, 2015 at 06:56:45PM +0300, Ilya Enkovich wrote: >> > On 16 Feb 16:31, Jakub Jelinek wrote: >> > > On Mon, Feb 16, 2015 at 06:20:59PM +0300, Ilya Enkovich wrote: >> > > > This patch restricts usage of Pointer Bounds Checker with Sanitizer. OK for trunk? >> > > >> > > There are many sanitizers, and for most of them I don't see why they would >> > > be in any conflict with -mmpx, it is just -fsanitize=address and >> > > -fsanitize=kernel-address. >> > > So perhaps test instead if (flag_sanitize & SANITIZE_ADDRESS) != 0, and >> > > better clear the flag_pointer_bounds after issuing the error, error () is >> > > not a fatal function, so you need something sensible for error-recovery. >> > > >> > > Jakub >> > >> > I don't know all sanitizers in details. Code generated by some of them may be incorrect from checker point of view. Thus I just wanted to disable unexplored and untested combinations. >> >> Shouldn't be that hard to write a testcase and test it. >> >> Most of the sanitizers just add code like >> if (some_condition) >> __ubsan_handle_... (); >> where from the POV of the program the __ubsan_* function reports or might >> report some problem, and optionally abort the program. >> That some_condition can be a check of the pointer value, shift count, >> divisor check, etc. >> >> Jakub > > OK. With no tricky memory references this should be safe. Here is a patch to filter off Adress Sanitizer only. > > Thanks for review! > > Ilya > -- > gcc/ > > 2015-02-16 Ilya Enkovich <ilya.enkovich@intel.com> > > PR target/65044 > * toplev.c (process_options): Restrict Pointer Bounds Checker > usage with Address Sanitizer. > > gcc/testsuite/ > > 2015-02-16 Ilya Enkovich <ilya.enkovich@intel.com> > > PR target/65044 > * gcc.target/i386/pr65044.c: New. > > > diff --git a/gcc/testsuite/gcc.target/i386/pr65044.c b/gcc/testsuite/gcc.target/i386/pr65044.c > new file mode 100644 > index 0000000..4f318d6 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr65044.c > @@ -0,0 +1,12 @@ > +/* { dg-error "-fcheck-pointer-bounds is not supported with Address Sanitizer" } */ > +/* { dg-do compile } */ > +/* { dg-require-effective-target mpx } */ > +/* { dg-options "-fcheck-pointer-bounds -mmpx -fsanitize=address" } */ > + > +extern int x[]; > + > +void > +foo () > +{ > + x[0] = 0; > +} > diff --git a/gcc/toplev.c b/gcc/toplev.c > index 99cf180..70eb6b6 100644 > --- a/gcc/toplev.c > +++ b/gcc/toplev.c > @@ -1376,6 +1376,11 @@ process_options (void) > { > if (targetm.chkp_bound_mode () == VOIDmode) > error ("-fcheck-pointer-bounds is not supported for this target"); > + > + if (flag_sanitize & SANITIZE_ADDRESS) > + error ("-fcheck-pointer-bounds is not supported with Address Sanitizer"); > + > + flag_check_pointer_bounds = 0; > } > > /* One region RA really helps to decrease the code size. */
On Mon, Mar 02, 2015 at 01:25:43PM +0300, Ilya Enkovich wrote: > > --- a/gcc/toplev.c > > +++ b/gcc/toplev.c > > @@ -1376,6 +1376,11 @@ process_options (void) > > { > > if (targetm.chkp_bound_mode () == VOIDmode) > > error ("-fcheck-pointer-bounds is not supported for this target"); > > + > > + if (flag_sanitize & SANITIZE_ADDRESS) > > + error ("-fcheck-pointer-bounds is not supported with Address Sanitizer"); > > + > > + flag_check_pointer_bounds = 0; > > } Doesn't this disable -fcheck-pointer-bounds always? I'd expect you want to clear flag_check_pointer_bounds only if you issued one of the two errors... Jakub
2015-03-12 12:02 GMT+03:00 Jakub Jelinek <jakub@redhat.com>: > On Thu, Mar 12, 2015 at 11:51:51AM +0300, Ilya Enkovich wrote: >> On 09 Mar 15:51, Jakub Jelinek wrote: >> > On Mon, Mar 02, 2015 at 01:25:43PM +0300, Ilya Enkovich wrote: >> > > > --- a/gcc/toplev.c >> > > > +++ b/gcc/toplev.c >> > > > @@ -1376,6 +1376,11 @@ process_options (void) >> > > > { >> > > > if (targetm.chkp_bound_mode () == VOIDmode) >> > > > error ("-fcheck-pointer-bounds is not supported for this target"); >> > > > + >> > > > + if (flag_sanitize & SANITIZE_ADDRESS) >> > > > + error ("-fcheck-pointer-bounds is not supported with Address Sanitizer"); >> > > > + >> > > > + flag_check_pointer_bounds = 0; >> > > > } >> > >> > Doesn't this disable -fcheck-pointer-bounds always? >> > I'd expect you want to clear flag_check_pointer_bounds only if you issued >> > one of the two errors... >> > >> > Jakub >> >> Whoops! Here is a less destructive version. > > Ok for trunk. Did the old version pass make check? If so, perhaps you want to add > (incrementally) some test that would actually verify that > -fcheck-pointer-bounds does what it should do (e.g. by scanning tree dumps > etc.). Thanks! I sent previous version before make check. There are several chkp tests which would fail. Ilya > > Jakub
diff --git a/gcc/testsuite/gcc.target/i386/pr65044.c b/gcc/testsuite/gcc.target/i386/pr65044.c new file mode 100644 index 0000000..4f318d6 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr65044.c @@ -0,0 +1,12 @@ +/* { dg-error "-fcheck-pointer-bounds is not supported with Address Sanitizer" } */ +/* { dg-do compile } */ +/* { dg-require-effective-target mpx } */ +/* { dg-options "-fcheck-pointer-bounds -mmpx -fsanitize=address" } */ + +extern int x[]; + +void +foo () +{ + x[0] = 0; +} diff --git a/gcc/toplev.c b/gcc/toplev.c index 99cf180..70eb6b6 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -1376,6 +1376,11 @@ process_options (void) { if (targetm.chkp_bound_mode () == VOIDmode) error ("-fcheck-pointer-bounds is not supported for this target"); + + if (flag_sanitize & SANITIZE_ADDRESS) + error ("-fcheck-pointer-bounds is not supported with Address Sanitizer"); + + flag_check_pointer_bounds = 0; } /* One region RA really helps to decrease the code size. */