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