Re: [PATCH][PR sanitizer/78887] Don't emit ODR indicators if -fsanitize=kernel-address is present.

2017-01-13 Thread Jakub Jelinek
On Fri, Jan 13, 2017 at 12:01:54PM +0300, Maxim Ostapenko wrote:
> +of symbols they are emmitted for, these assumptions would be broken 
> for

emitted rather than emmitted.

> +ODR indicator symbols.  */
> +  return (!(flag_sanitize & SANITIZE_KERNEL_ADDRESS)
> +   && !DECL_ARTIFICIAL (decl)
> +   && !DECL_WEAK (decl)
> +   && TREE_PUBLIC (decl));
>  }
>  
>  /* Append description of a single global DECL into vector V.


Jakub


Re: [PATCH][PR sanitizer/78887] Don't emit ODR indicators if -fsanitize=kernel-address is present.

2017-01-13 Thread Maxim Ostapenko

On 13/01/17 11:24, Jakub Jelinek wrote:

On Fri, Jan 13, 2017 at 11:19:19AM +0300, Maxim Ostapenko wrote:

as mentioned in PR, Linux kernel 4.9 fails to build with ASan due to wrong
handling of emitted ODR indicator symbols. Although this might be a kernel
bug (relying on specific pattern in symbol name sounds questionable), kernel
doesn't need ODR indicators at all thus we can just disable them if
-fsanitize=kernel-address is present.
Tested on x86_64-unknown-linux-gnu, OK for trunk?
gcc/ChangeLog:

2017-01-13  Maxim Ostapenko  

PR sanitizer/78887
* asan.c (asan_needs_odr_indicator_p): Don't emit ODR indicators
if -fsanitize=kernel-address is present.

diff --git a/gcc/asan.c b/gcc/asan.c
index bc7ebc8..157d468 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -2360,7 +2360,8 @@ create_odr_indicator (tree decl, tree type)
  static bool
  asan_needs_odr_indicator_p (tree decl)
  {
-  return !DECL_ARTIFICIAL (decl) && !DECL_WEAK (decl) && TREE_PUBLIC (decl);
+  return !(flag_sanitize & SANITIZE_KERNEL_ADDRESS) && !DECL_ARTIFICIAL (decl)
+&& !DECL_WEAK (decl) && TREE_PUBLIC (decl);

As the condition is longer than a line, please use
   return (!(flag_sanitize & SANITIZE_KERNEL_ADDRESS)
  && !DECL_ARTIFICIAL (decl)
  && !DECL_WEAK (decl)
  && TREE_PUBLIC (decl));
instead (i.e. one sub-condition per line, and ()s around the whole
condition.  Perhaps a short comment why we don't emit those for
-fsanitize=kernel-address would be useful too.

Ok for trunk with those changes.


OK, thanks, I'm going to apply following patch.

-Maxim



Jakub




gcc/ChangeLog:

2017-01-13  Maxim Ostapenko  

	PR sanitizer/78887
	* asan.c (asan_needs_odr_indicator_p): Don't emit ODR indicators
	if -fsanitize=kernel-address is present.

diff --git a/gcc/asan.c b/gcc/asan.c
index bc7ebc8..e3bf16d 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -2360,7 +2360,16 @@ create_odr_indicator (tree decl, tree type)
 static bool
 asan_needs_odr_indicator_p (tree decl)
 {
-  return !DECL_ARTIFICIAL (decl) && !DECL_WEAK (decl) && TREE_PUBLIC (decl);
+  /* Don't emit ODR indicators for kernel because:
+ a) Kernel is written in C thus doesn't need ODR indicators.
+ b) Some kernel code may have assumptions about symbols containing specific
+patterns in their names.  Since ODR indicators contain original names
+of symbols they are emmitted for, these assumptions would be broken for
+ODR indicator symbols.  */
+  return (!(flag_sanitize & SANITIZE_KERNEL_ADDRESS)
+	  && !DECL_ARTIFICIAL (decl)
+	  && !DECL_WEAK (decl)
+	  && TREE_PUBLIC (decl));
 }
 
 /* Append description of a single global DECL into vector V.


Re: [PATCH][PR sanitizer/78887] Don't emit ODR indicators if -fsanitize=kernel-address is present.

2017-01-13 Thread Jakub Jelinek
On Fri, Jan 13, 2017 at 11:19:19AM +0300, Maxim Ostapenko wrote:
> as mentioned in PR, Linux kernel 4.9 fails to build with ASan due to wrong
> handling of emitted ODR indicator symbols. Although this might be a kernel
> bug (relying on specific pattern in symbol name sounds questionable), kernel
> doesn't need ODR indicators at all thus we can just disable them if
> -fsanitize=kernel-address is present.
> Tested on x86_64-unknown-linux-gnu, OK for trunk?

> gcc/ChangeLog:
> 
> 2017-01-13  Maxim Ostapenko  
> 
>   PR sanitizer/78887
>   * asan.c (asan_needs_odr_indicator_p): Don't emit ODR indicators
>   if -fsanitize=kernel-address is present.
> 
> diff --git a/gcc/asan.c b/gcc/asan.c
> index bc7ebc8..157d468 100644
> --- a/gcc/asan.c
> +++ b/gcc/asan.c
> @@ -2360,7 +2360,8 @@ create_odr_indicator (tree decl, tree type)
>  static bool
>  asan_needs_odr_indicator_p (tree decl)
>  {
> -  return !DECL_ARTIFICIAL (decl) && !DECL_WEAK (decl) && TREE_PUBLIC (decl);
> +  return !(flag_sanitize & SANITIZE_KERNEL_ADDRESS) && !DECL_ARTIFICIAL 
> (decl)
> +  && !DECL_WEAK (decl) && TREE_PUBLIC (decl);

As the condition is longer than a line, please use
  return (!(flag_sanitize & SANITIZE_KERNEL_ADDRESS)
  && !DECL_ARTIFICIAL (decl)
  && !DECL_WEAK (decl)
  && TREE_PUBLIC (decl));
instead (i.e. one sub-condition per line, and ()s around the whole
condition.  Perhaps a short comment why we don't emit those for
-fsanitize=kernel-address would be useful too.

Ok for trunk with those changes.

Jakub