Re: [asan] WIP protection of globals

[Jakub Jelinek <jakub at redhat dot com>]

On Tue, Oct 16, 2012 at 02:41:42PM -0700, Xinliang David Li wrote:
> > +bool
> > +asan_protect_global (tree decl)
> > +{
> > +  rtx rtl, symbol;
> > +  section *sect;
> > +
> > +  if (TREE_CODE (decl) != VAR_DECL
> > +      || DECL_THREAD_LOCAL_P (decl)
> > +      || DECL_EXTERNAL (decl)
> > +      || !TREE_ASM_WRITTEN (decl)
> > +      || !DECL_RTL_SET_P (decl)
> > +      || DECL_ONE_ONLY (decl)
> > +      || DECL_COMMON (decl)
> 
> Why the above two condition? If the linker picks the larger size one,
> it is ok to do the instrumentation.

For DECL_ONE_ONLY, what LLVM does is plain wrong, it puts address of
even vars exported from shared libraries (which can be overridden) into
the array passed to __asan_*register_globals.  That can't work, you don't
know if the var that is found first was compiled with -fasan or not.
We need to use a local alias in that case (yeah, my WIP patch doesn't do
that yet, but I wanted to post at least something), and I believe local
aliases might be an issue with DECL_ONE_ONLY (and anyway, if a -fno-asan
DECL_ONE_ONLY var wins over -fasan one, there is no padding after it).

LLVM does other things wrong, it increases the size of the vars which is
IMHO a big no no, say for copy relocs etc., and last I'd say the relocations
in the description variable are unnecessary, would be better if it e.g. used
PC relative relocations and could made the array passed to
__asan_*register_globals read-only.

And for DECL_COMMON, you can't put any padding after a common variable
without making the size of the common var larger (and increasing its
alignment), both are undesirable for -fasan/-fno-asan mixing.

> > +      || (DECL_SECTION_NAME (decl) != NULL_TREE
> > +         && !DECL_HAS_IMPLICIT_SECTION_NAME_P (decl))
> 
> Why is this condition? Is it related to -fdata-sections ?

-fdata-sections will have non-NULL DECL_SECTION_NAME, but still
DECL_HAS_IMPLICIT_SECTION_NAME_P.  The above condition is not to break
various packages that put say a struct into some user section and expect
the section then to contain an array of those structs.
E.g. Linux kernel does this, systemtap probes, prelink, ...
If padding is inserted, all those would break.

	Jakub