Re: [PATCH][PING^2] Fix for PR59600 (prohibit inlining if no_sanitize_address)

2014-02-04 Thread Yury Gribov

Richard wrote:

What about updated patch?


Ok if it passes bootstrap and regtesting.


It does, commited in r207497.

-Y


Re: [PATCH][PING^2] Fix for PR59600 (prohibit inlining if no_sanitize_address)

2014-02-04 Thread Richard Biener
On Tue, Feb 4, 2014 at 3:32 PM, Yury Gribov  wrote:
> Richard wrote:
>>
>> I think you can't rely on pointer equivalence of the lookup_attribute
>> result so you want instead of
>
>
> Thanks, makes sense.
>
>
>> Please also name CIF_OPTION_MISMATCH as CIF_ATTRIBUTE_MISMATCH
>> and say "function attribute mismatch" in the description.
>
>
> Done.
>
> What about updated patch?

Ok if it passes bootstrap and regtesting.

Thanks,
Richard.

> -Y


Re: [PATCH][PING^2] Fix for PR59600 (prohibit inlining if no_sanitize_address)

2014-02-04 Thread Yury Gribov

Richard wrote:

I think you can't rely on pointer equivalence of the lookup_attribute
result so you want instead of


Thanks, makes sense.


Please also name CIF_OPTION_MISMATCH as CIF_ATTRIBUTE_MISMATCH
and say "function attribute mismatch" in the description.


Done.

What about updated patch?

-Y
	PR sanitizer/59600

gcc/ChangeLog:

2014-01-21  ygribov  

	* cif-code.def (ATTRIBUTE_MISMATCH): New CIF code.
	* ipa-inline.c (report_inline_failed_reason): Handle mismatched
	sanitization attributes.
	(sanitize_attrs_match_for_inline_p): New function.
	(can_inline_edge_p): Likewise.

gcc/testsuite/ChangeLog:

2014-01-21  ygribov  

	* gcc.dg/asan/nosanitize-and-inline.c: : New test.

diff --git a/gcc/cif-code.def b/gcc/cif-code.def
index 5591f9a..71f3e39 100644
--- a/gcc/cif-code.def
+++ b/gcc/cif-code.def
@@ -123,3 +123,7 @@ DEFCIFCODE(OPTIMIZATION_MISMATCH, CIF_FINAL_ERROR,
 /* We can't inline because the callee refers to comdat-local symbols.  */
 DEFCIFCODE(USES_COMDAT_LOCAL, CIF_FINAL_NORMAL,
 	   N_("callee refers to comdat-local symbols"))
+
+/* We can't inline because of mismatched caller/callee attributes.  */
+DEFCIFCODE(ATTRIBUTE_MISMATCH, CIF_FINAL_NORMAL,
+	   N_("function attribute mismatch"))
diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
index 5f47e0b..ce24ea5 100644
--- a/gcc/ipa-inline.c
+++ b/gcc/ipa-inline.c
@@ -234,7 +234,25 @@ report_inline_failed_reason (struct cgraph_edge *e)
 }
 }
 
-/* Decide if we can inline the edge and possibly update
+ /* Decide whether sanitizer-related attributes allow inlining. */
+
+static bool
+sanitize_attrs_match_for_inline_p (const_tree caller, const_tree callee)
+{
+  /* Don't care if sanitizer is disabled */
+  if (!(flag_sanitize & SANITIZE_ADDRESS))
+return true;
+
+  if (!caller || !callee)
+return true;
+
+  return !!lookup_attribute ("no_sanitize_address",
+  DECL_ATTRIBUTES (caller)) == 
+  !!lookup_attribute ("no_sanitize_address",
+  DECL_ATTRIBUTES (callee));
+}
+
+ /* Decide if we can inline the edge and possibly update
inline_failed reason.  
We check whether inlining is possible at all and whether
caller growth limits allow doing so.  
@@ -327,6 +345,12 @@ can_inline_edge_p (struct cgraph_edge *e, bool report,
   e->inline_failed = CIF_TARGET_OPTION_MISMATCH;
   inlinable = false;
 }
+  /* Don't inline a function with mismatched sanitization attributes. */
+  else if (!sanitize_attrs_match_for_inline_p (e->caller->decl, callee->decl))
+{
+  e->inline_failed = CIF_ATTRIBUTE_MISMATCH;
+  inlinable = false;
+}
   /* Check if caller growth allows the inlining.  */
   else if (!DECL_DISREGARD_INLINE_LIMITS (callee->decl)
 	   && !disregard_limits
diff --git a/gcc/testsuite/gcc.dg/asan/nosanitize-and-inline.c b/gcc/testsuite/gcc.dg/asan/nosanitize-and-inline.c
index e69de29..5853801 100644
--- a/gcc/testsuite/gcc.dg/asan/nosanitize-and-inline.c
+++ b/gcc/testsuite/gcc.dg/asan/nosanitize-and-inline.c
@@ -0,0 +1,57 @@
+/* { dg-do run } */
+
+/* This is a simplified version of what Emacs does internally,
+   when marking its stack.  */
+
+static unsigned long sum;
+static void *stack_base;
+
+/* A simple substitute for what Emacs actually does.  */
+static void
+mark_maybe_pointer (void *p)
+{
+  sum ^= (unsigned long) p;
+}
+
+static inline void __attribute__ ((no_sanitize_address))
+mark_memory (void **start, void **end)
+{
+  void **pp;
+
+  if (end < start)
+{
+  void **tem = start;
+  start = end;
+  end = tem;
+}
+
+  for (pp = start; pp < end; pp++)
+{
+  /* This is the dereference that we don't want sanitized.  */
+  void *p = *pp;
+
+  mark_maybe_pointer (p);
+}
+}
+
+static void
+mark_stack (void)
+{
+  void *end;
+  mark_memory (stack_base, &end);
+}
+
+void
+garbage_collect (void)
+{
+  mark_stack ();
+}
+
+int
+main (void)
+{
+  void *dummy;
+  stack_base = &dummy;
+  garbage_collect ();
+  return 0;
+}


Re: [PATCH][PING^2] Fix for PR59600 (prohibit inlining if no_sanitize_address)

2014-02-04 Thread Richard Biener
On Tue, Feb 4, 2014 at 4:39 AM, Yury Gribov  wrote:
>  Original Message 
> Subject: [PATCH][PING] Fix for PR59600 (prohibit inlining if
> no_sanitize_address)
> Date: Tue, 28 Jan 2014 09:13:10 +0400
> From: Yury Gribov 
> To: GCC Patches 
>
>  Original Message 
> Subject: [PATCH] Fix for PR59600
> Date: Tue, 21 Jan 2014 14:42:31 +0400
> From: Yury Gribov 
> To: GCC Patches 
>
> Hi,
>
> This patch fixes the problem reported in
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59600 : functions with
> mismatching no_sanitize_address attributes should not be considered for
> inlining, otherwise the meaning of no_sanitize_address will not be
> preserved.
>
> I didn't get feedback in Bugzilla so I'm sending the patch here.
>
> Bootstrapped/regtested on x64.

I think you can't rely on pointer equivalence of the lookup_attribute
result so you want instead of

+  return lookup_attribute ("no_sanitize_address",
+  DECL_ATTRIBUTES (caller)) ==
+lookup_attribute ("no_sanitize_address",
+  DECL_ATTRIBUTES (callee));

return (lookup_attribute ("no_sanitize_address", DECL_ATTRIBUTES
(caller)) != NULL_TREE)
  == (lookup_attribute ("no_sanitize_address", DECL_ATTRIBUTES
(callee)) != NULL_TREE);

Please also name CIF_OPTION_MISMATCH as CIF_ATTRIBUTE_MISMATCH
and say "function attribute mismatch" in the description.

Thanks,
Richard.

Richard.

> -Y
>
>
>
>
>
>