Re: [PATCH 2/7] Libsanitizer merge from upstream r249633.

[Maxim Ostapenko <m.ostapenko@partner.samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 1781 bytes --]

On 14/10/15 10:30, Jakub Jelinek wrote:
> On Tue, Oct 13, 2015 at 02:16:23PM +0300, Maxim Ostapenko wrote:
>> This patch introduces required compiler changes. Now, we don't version
>> asan_init, we have a special __asan_version_mismatch_check_v[n] symbol for
>> this.
> For this, I just have to wonder what is the actual improvement over what we
> had.  To me it looks like a step in the wrong direction, it will only bloat
> the size of the ctors.  I can live with it, but just want to put on record I
> think it is a mistake.
>
>> Also, asan_stack_malloc_[n] doesn't take a local stack as a second parameter
>> anymore, so don't pass it.
> I think this is another mistake, but this time with actually bad fix on the
> compiler side for it.  If I read the code well, previously
> __asan_stack_malloc_n would return you the local stack if it failed for
> whatever reason, which is actually what you want as fallback.
> But, the new code returns NULL instead, so I think you would need to compare
> the return value of __asan_stack_malloc_n with NULL and if it is NULL, use
> the addr instead of what it returned; which is not what your asan.c change
> does.  Now, what is the improvement here?  Bloat the compiler generated
> code... :(
>

Ah, right, fixing this now. Does this looks better now?

>> 2015-10-12  Maxim Ostapenko  <m.ostapenko@partner.samsung.com>
>>
>> config/
>>
>> 	* bootstrap-asan.mk: Replace ASAN_OPTIONS=detect_leaks with
>> 	LSAN_OPTIONS=detect_leaks
> Missing . at the end, and the config/ hunk missing from the patch.
>
>> gcc/
>>
>> 	* asan.c (asan_emit_stack_protection): Don't pass local stack to
>> 	asan_stack_malloc_[n] anymore.
>> 	(asan_finish_file): Instert __asan_version_mismatch_check_v[n] call.
> s/Instert/Instead/

Fixed now.

>
> 	Jakub
>


[-- Attachment #2: 2.diff --]
[-- Type: text/x-patch, Size: 3926 bytes --]

2015-10-12  Maxim Ostapenko  <m.ostapenko@partner.samsung.com>

config/

	* bootstrap-asan.mk: Replace ASAN_OPTIONS=detect_leaks with
	LSAN_OPTIONS=detect_leaks.

gcc/

	* asan.c (asan_emit_stack_protection): Don't pass local stack to
	asan_stack_malloc_[n] anymore. Check if asan_stack_malloc_[n] returned
	NULL and use local stack than.
	(asan_finish_file): Insert __asan_version_mismatch_check_v[n] call
	in addition to __asan_init.
	* sanitizer.def (BUILT_IN_ASAN_INIT): Rename to __asan_init.
	(BUILT_IN_ASAN_VERSION_MISMATCH_CHECK): Add new builtin call.

gcc/testsuite/

	g++.dg/asan/default-options-1.C: Adjust testcase.

Index: gcc/asan.c
===================================================================
--- gcc/asan.c	(revision 228817)
+++ gcc/asan.c	(working copy)
@@ -1132,12 +1132,16 @@
       snprintf (buf, sizeof buf, "__asan_stack_malloc_%d",
 		use_after_return_class);
       ret = init_one_libfunc (buf);
-      rtx addr = convert_memory_address (ptr_mode, base);
-      ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2,
+      ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 1,
 				     GEN_INT (asan_frame_size
 					      + base_align_bias),
-				     TYPE_MODE (pointer_sized_int_node),
-				     addr, ptr_mode);
+				     TYPE_MODE (pointer_sized_int_node));
+      /* __asan_stack_malloc_[n] returns a pointer to fake stack if succeeded
+	 and NULL otherwise.  Check if RET value is NULL and jump over the
+	 BASE reassignment in this case.  Otherwise, reassign BASE to RET.  */
+      int very_unlikely = REG_BR_PROB_BASE / 2000 - 1;
+      emit_cmp_and_jump_insns (ret, const0_rtx, EQ, NULL_RTX,
+			       VOIDmode, 0, lab, very_unlikely);
       ret = convert_memory_address (Pmode, ret);
       emit_move_insn (base, ret);
       emit_label (lab);
@@ -2470,6 +2474,8 @@
     {
       tree fn = builtin_decl_implicit (BUILT_IN_ASAN_INIT);
       append_to_statement_list (build_call_expr (fn, 0), &asan_ctor_statements);
+      fn = builtin_decl_implicit (BUILT_IN_ASAN_VERSION_MISMATCH_CHECK);
+      append_to_statement_list (build_call_expr (fn, 0), &asan_ctor_statements);
     }
   FOR_EACH_DEFINED_VARIABLE (vnode)
     if (TREE_ASM_WRITTEN (vnode->decl)
Index: gcc/sanitizer.def
===================================================================
--- gcc/sanitizer.def	(revision 228817)
+++ gcc/sanitizer.def	(working copy)
@@ -27,8 +27,11 @@
    for other FEs by asan.c.  */
 
 /* Address Sanitizer */
-DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_INIT, "__asan_init_v4",
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_INIT, "__asan_init",
 		      BT_FN_VOID, ATTR_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_VERSION_MISMATCH_CHECK,
+		      "__asan_version_mismatch_check_v6",
+		      BT_FN_VOID, ATTR_NOTHROW_LEAF_LIST)
 /* Do not reorder the BUILT_IN_ASAN_{REPORT,CHECK}* builtins, e.g. cfgcleanup.c
    relies on this order.  */
 DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REPORT_LOAD1, "__asan_report_load1",
Index: gcc/testsuite/g++.dg/asan/default-options-1.C
===================================================================
--- gcc/testsuite/g++.dg/asan/default-options-1.C	(revision 228817)
+++ gcc/testsuite/g++.dg/asan/default-options-1.C	(working copy)
@@ -12,4 +12,4 @@
   return 0;
 }
 
-// { dg-output "Using the defaults from __asan_default_options:.* foo=bar.*(\n|\r\n|\r)" }
+// { dg-output "WARNING: found 1 unrecognized flag\\(s\\):(\n|\r\n|\r).*foo(\n|\r\n|\r)" }
Index: config/bootstrap-asan.mk
===================================================================
--- config/bootstrap-asan.mk	(revision 228817)
+++ config/bootstrap-asan.mk	(working copy)
@@ -1,7 +1,7 @@
 # This option enables -fsanitize=address for stage2 and stage3.
 
 # Suppress LeakSanitizer in bootstrap.
-export ASAN_OPTIONS="detect_leaks=0"
+export LSAN_OPTIONS="detect_leaks=0"
 
 STAGE2_CFLAGS += -fsanitize=address
 STAGE3_CFLAGS += -fsanitize=address