| Message ID | 561CE807.9060503@partner.samsung.com |
|---|---|
| State | New |
| Headers | show |
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... :( > 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/ Jakub
On 10/13/2015 02:16 PM, 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. > > Also, asan_stack_malloc_[n] doesn't take a local stack as a second > parameter anymore, so don't pass it. Did you compare libasan.so and libclang_rt-asan.so for other ABI incompatibilities e.g. via libabigail? -Y
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. (asan_finish_file): Instert __asan_version_mismatch_check_v[n] call. * 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 228704) +++ gcc/asan.c (working copy) @@ -1132,12 +1132,10 @@ 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)); ret = convert_memory_address (Pmode, ret); emit_move_insn (base, ret); emit_label (lab); @@ -2470,6 +2468,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 228704) +++ 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 228704) +++ 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)" }