Re: [PATCH 4/5] Disable -fcheck-pointer-bounds with sanitizers.

[Martin Liška]

On 03/07/2017 05:21 PM, Jakub Jelinek wrote:
> On Mon, Mar 06, 2017 at 02:07:37PM +0100, marxin wrote:
>>  PR target/65705
>>  PR target/69804
>>  * toplev.c (process_options): Disable -fcheck-pointer-bounds with
>>  sanitizers.
> 
> I can understand why it is disabled for -fsanitize=address or
> -fsanitize=bounds, perhaps -fsanitize=threads too,
> but don't understand why e.g. -fsanitize=shift or -fsanitize=unreachable
> or -fsanitize=signed-integer-overflow or -fsanitize=leak (which is purely
> a linking option) should affect it.
> 
>> +  const char *sanitizer_names[] = { "Address", "Undefined Behavior",
>> +"Leak", "Thread" };
>> +  const int sanitizer_flags[] = { SANITIZE_ADDRESS, SANITIZE_UNDEFINED,
>> +SANITIZE_LEAK, SANITIZE_THREAD };
> 
> Even if there is a reason for that, there is also
> SANITIZE_NONDEFAULT that is part of UB sanitizer, so if you can't
> -fcheck-pointer-bounds with any parts of -fsanitize=undefined, then
> likely it applies to others too.
> For -fsanitize=bounds-stricts it surely applies though, if -fsanitize=bounds
> can't be MPX instrumented.
> 
>> +  for (unsigned i = 0; i < sizeof (sanitizer_flags) / sizeof (int); i++)
>> +if (flag_sanitize & sanitizer_flags[i])
>> +  {
>> +error_at (UNKNOWN_LOCATION,
>> +  "-fcheck-pointer-bounds is not supported with "
>> +  "%s Sanitizer", sanitizer_names[i]);
> 
> This is not i18n friendly, I think you just want to unroll the loop
> by hand and use 4 (or just 3?) different error_at calls.
> 
>   Jakub
> 

Thanks for review, I fixed both in patch that I'm going to install.

Martin
>From a6c3066fb95b521d4b9ca396c115cbe32fab72ee Mon Sep 17 00:00:00 2001
From: marxin 
Date: Mon, 6 Mar 2017 14:07:37 +0100
Subject: [PATCH] Disable -fcheck-pointer-bounds with sanitizers.

gcc/ChangeLog:

2017-03-06  Martin Liska  

	PR target/65705
	PR target/69804
	* toplev.c (process_options): Disable -fcheck-pointer-bounds with
	sanitizers.

gcc/testsuite/ChangeLog:

2017-03-06  Martin Liska  

	PR target/65705
	PR target/69804
	* gcc.target/i386/pr71458.c: Update scanned pattern.
---
 gcc/testsuite/gcc.target/i386/pr71458.c |  2 +-
 gcc/toplev.c| 32 
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/gcc/testsuite/gcc.target/i386/pr71458.c b/gcc/testsuite/gcc.target/i386/pr71458.c
index 27e7764b5a0..2faf6bb9391 100644
--- a/gcc/testsuite/gcc.target/i386/pr71458.c
+++ b/gcc/testsuite/gcc.target/i386/pr71458.c
@@ -1,6 +1,6 @@
 /* { dg-do compile { target { ! x32 } } } */
 /* { dg-options "-fcheck-pointer-bounds -mmpx -fsanitize=bounds" } */
-/* { dg-error "-fcheck-pointer-bounds is not supported with -fsanitize=bounds" "" { target *-*-* } 0 } */
+/* { dg-error "-fcheck-pointer-bounds is not supported with Undefined Behavior Sanitizer" "" { target *-*-* } 0 } */
 
 enum {} a[0];
 void fn1(int);
diff --git a/gcc/toplev.c b/gcc/toplev.c
index beb581aba55..6a7e4fbdffb 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1274,22 +1274,30 @@ process_options (void)
 	  flag_check_pointer_bounds = 0;
 	}
 
-  if (flag_sanitize & SANITIZE_ADDRESS)
+  if (flag_sanitize)
 	{
-	  error_at (UNKNOWN_LOCATION,
-		"-fcheck-pointer-bounds is not supported with "
-		"Address Sanitizer");
-	  flag_check_pointer_bounds = 0;
-	}
+	  if (flag_sanitize & SANITIZE_ADDRESS)
+	error_at (UNKNOWN_LOCATION,
+		  "-fcheck-pointer-bounds is not supported with "
+		  "Address Sanitizer");
+
+	  if (flag_sanitize & (SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT))
+	error_at (UNKNOWN_LOCATION,
+		  "-fcheck-pointer-bounds is not supported with "
+		  "Undefined Behavior Sanitizer");
+
+	  if (flag_sanitize & SANITIZE_LEAK)
+	error_at (UNKNOWN_LOCATION,
+		  "-fcheck-pointer-bounds is not supported with "
+		  "Leak Sanitizer");
+
+	  if (flag_sanitize & SANITIZE_THREAD)
+	error_at (UNKNOWN_LOCATION,
+		  "-fcheck-pointer-bounds is not supported with "
+		  "Thread Sanitizer");
 
-  if (flag_sanitize & SANITIZE_BOUNDS)
-	{
-	  error_at (UNKNOWN_LOCATION,
-		"-fcheck-pointer-bounds is not supported with "
-		"-fsanitize=bounds");
 	  flag_check_pointer_bounds = 0;
 	}
-
 }
 
   /* One region RA really helps to decrease the code size.  */
-- 
2.11.1

Re: [PATCH 4/5] Disable -fcheck-pointer-bounds with sanitizers.

[Jakub Jelinek]

On Mon, Mar 06, 2017 at 02:07:37PM +0100, marxin wrote:
>   PR target/65705
>   PR target/69804
>   * toplev.c (process_options): Disable -fcheck-pointer-bounds with
>   sanitizers.

I can understand why it is disabled for -fsanitize=address or
-fsanitize=bounds, perhaps -fsanitize=threads too,
but don't understand why e.g. -fsanitize=shift or -fsanitize=unreachable
or -fsanitize=signed-integer-overflow or -fsanitize=leak (which is purely
a linking option) should affect it.

> +  const char *sanitizer_names[] = { "Address", "Undefined Behavior",
> + "Leak", "Thread" };
> +  const int sanitizer_flags[] = { SANITIZE_ADDRESS, SANITIZE_UNDEFINED,
> + SANITIZE_LEAK, SANITIZE_THREAD };

Even if there is a reason for that, there is also
SANITIZE_NONDEFAULT that is part of UB sanitizer, so if you can't
-fcheck-pointer-bounds with any parts of -fsanitize=undefined, then
likely it applies to others too.
For -fsanitize=bounds-stricts it surely applies though, if -fsanitize=bounds
can't be MPX instrumented.

> +  for (unsigned i = 0; i < sizeof (sanitizer_flags) / sizeof (int); i++)
> + if (flag_sanitize & sanitizer_flags[i])
> +   {
> + error_at (UNKNOWN_LOCATION,
> +   "-fcheck-pointer-bounds is not supported with "
> +   "%s Sanitizer", sanitizer_names[i]);

This is not i18n friendly, I think you just want to unroll the loop
by hand and use 4 (or just 3?) different error_at calls.

Jakub

Re: [PATCH 4/5] Disable -fcheck-pointer-bounds with sanitizers.

[Jeff Law]

On 03/06/2017 06:07 AM, marxin wrote:

gcc/ChangeLog:

2017-03-06  Martin Liska  

PR target/65705
PR target/69804
* toplev.c (process_options): Disable -fcheck-pointer-bounds with
sanitizers.

gcc/testsuite/ChangeLog:

2017-03-06  Martin Liska  

PR target/65705
PR target/69804
* gcc.target/i386/pr71458.c: Update scanned pattern.

OK.
jeff

[PATCH 4/5] Disable -fcheck-pointer-bounds with sanitizers.

[marxin]

gcc/ChangeLog:

2017-03-06  Martin Liska  

PR target/65705
PR target/69804
* toplev.c (process_options): Disable -fcheck-pointer-bounds with
sanitizers.

gcc/testsuite/ChangeLog:

2017-03-06  Martin Liska  

PR target/65705
PR target/69804
* gcc.target/i386/pr71458.c: Update scanned pattern.
---
 gcc/testsuite/gcc.target/i386/pr71458.c |  2 +-
 gcc/toplev.c| 29 +
 2 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/gcc/testsuite/gcc.target/i386/pr71458.c 
b/gcc/testsuite/gcc.target/i386/pr71458.c
index 27e7764b5a0..2faf6bb9391 100644
--- a/gcc/testsuite/gcc.target/i386/pr71458.c
+++ b/gcc/testsuite/gcc.target/i386/pr71458.c
@@ -1,6 +1,6 @@
 /* { dg-do compile { target { ! x32 } } } */
 /* { dg-options "-fcheck-pointer-bounds -mmpx -fsanitize=bounds" } */
-/* { dg-error "-fcheck-pointer-bounds is not supported with -fsanitize=bounds" 
"" { target *-*-* } 0 } */
+/* { dg-error "-fcheck-pointer-bounds is not supported with Undefined Behavior 
Sanitizer" "" { target *-*-* } 0 } */
 
 enum {} a[0];
 void fn1(int);
diff --git a/gcc/toplev.c b/gcc/toplev.c
index beb581aba55..b8f87b878da 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1274,22 +1274,19 @@ process_options (void)
  flag_check_pointer_bounds = 0;
}
 
-  if (flag_sanitize & SANITIZE_ADDRESS)
-   {
- error_at (UNKNOWN_LOCATION,
-   "-fcheck-pointer-bounds is not supported with "
-   "Address Sanitizer");
- flag_check_pointer_bounds = 0;
-   }
-
-  if (flag_sanitize & SANITIZE_BOUNDS)
-   {
- error_at (UNKNOWN_LOCATION,
-   "-fcheck-pointer-bounds is not supported with "
-   "-fsanitize=bounds");
- flag_check_pointer_bounds = 0;
-   }
-
+  const char *sanitizer_names[] = { "Address", "Undefined Behavior",
+   "Leak", "Thread" };
+  const int sanitizer_flags[] = { SANITIZE_ADDRESS, SANITIZE_UNDEFINED,
+   SANITIZE_LEAK, SANITIZE_THREAD };
+
+  for (unsigned i = 0; i < sizeof (sanitizer_flags) / sizeof (int); i++)
+   if (flag_sanitize & sanitizer_flags[i])
+ {
+   error_at (UNKNOWN_LOCATION,
+ "-fcheck-pointer-bounds is not supported with "
+ "%s Sanitizer", sanitizer_names[i]);
+   flag_check_pointer_bounds = 0;
+ }
 }
 
   /* One region RA really helps to decrease the code size.  */
-- 
2.11.1