On Mon, Nov 07, 2016 at 11:31:18AM +0300, Maxim Ostapenko wrote:
> --- a/gcc/asan.c
> +++ b/gcc/asan.c
> @@ -1329,6 +1329,16 @@ asan_needs_local_alias (tree decl)
> return DECL_WEAK (decl) || !targetm.binds_local_p (decl);
> }
>
> +/* Return true if DECL, a global var, is an artificial ODR indicator symbol
> + therefore doesn't need protection. */
> +
> +static bool
> +is_odr_indicator (tree decl)
> +{
> + const char *sym_name = IDENTIFIER_POINTER (DECL_NAME (decl));
> + return strstr(sym_name, "_.__odr_asan_") == sym_name;
Formatting, missing space before (.
Plus strstr (x, y) == x is very inefficient, strncmp would be cheaper.
But more importantly, you are relying on what exactly does
ASM_GENERATE_INTERNAL_LABEL, that differs between targets, not all of them
e.g. allow . in symbol names, other targets use $, others can only use _,
etc. I think you'd better just add "asan odr indicator" attribute
(including the spaces, so it isn't something users can add to their
variables) to the artificial vars and lookup_attribute in the
is_odr_indicator predicate (after testing some cheap flags like
DECL_ARTIFICIAL).
> + tree var = build_decl (UNKNOWN_LOCATION, VAR_DECL, get_identifier
> (sym_name),
> + char_type_node);
> + TREE_ADDRESSABLE (var) = TREE_ADDRESSABLE (decl);
How is addressability of the original decl related to addressability of the
indicator? If you take address of the indicator (it is stored in the
structure), it should be just 1.
> + TREE_READONLY (var) = 0;
> + TREE_THIS_VOLATILE (var) = 1;
> + DECL_GIMPLE_REG_P (var) = DECL_GIMPLE_REG_P (decl);
Again, how is this related? Just store 0.
> + DECL_ARTIFICIAL (var) = 1;
> + DECL_IGNORED_P (var) = DECL_IGNORED_P (decl);
The indicators should be surely not recorded in debug info, so DEC_IGNORED_P
should be 1.
> + TREE_STATIC (var) = 1;
> + TREE_PUBLIC (var) = 1;
> + DECL_VISIBILITY (var) = DECL_VISIBILITY (decl);
Are they meant to have the same visibility and be exported from DSOs if the
original var is?
> @@ -2313,8 +2374,7 @@ asan_add_global (tree decl, tree type,
> vec<constructor_elt, va_gc> *v)
> else
> locptr = build_int_cst (uptr, 0);
> CONSTRUCTOR_APPEND_ELT (vinner, NULL_TREE, locptr);
> - /* TODO: support ODR indicators. */
> - CONSTRUCTOR_APPEND_ELT(vinner, NULL_TREE, build_int_cst (uptr, 0));
> + CONSTRUCTOR_APPEND_ELT(vinner, NULL_TREE, odr_indicator_ptr);
Formatting, missing space before (, both in this patch and in the previous
one.
Jakub
