Re: RFC: PATCH: ld/12942: Plugin not handling correctly resolution of COMDATs.

[Alan Modra <amodra at gmail dot com>]

On Mon, Jul 04, 2011 at 03:53:28PM -0700, H.J. Lu wrote:
> Hi,
> 
> This adds SEC_LTO_COMDAT and GNU_LTO_COMAT_SECTION_NAME.  They are
> used to support LTO COMDAT symbols. Any comments?

This is a hard patch to review, almost as much work as writing it in
the first place, because you don't explain why you need to do what you
do.  Also, with a slightly out of date gcc I couldn't reproduce the
problem on powerpc or powerpc64, so is there something special about
x86_64 or did gcc recently start using comdat groups for lto?

>  	    case SEC_LINK_DUPLICATES_DISCARD:
> +	      if (info->loading_lto_outputs
> +		  && (l_owner->flags & BFD_PLUGIN) != 0)

It took me a while to convince myself this was correct, so I'd like to
see this test commented.  Perhaps
	      /* If we found an LTO IR match for this comdat group on
		 the first pass, replace it with the LTO output on the
		 second pass.  We can't simply choose real object
		 files over IR because the first pass may contain a
		 mix of LTO and normal objects and we must keep the
		 first match, be it IR or real.  */

> +		{
> +		  /* Replace the plugin dummy with the LTO output.  */
> +		  l->linked = *linked;
> +		  return;
> +		}
>  	      break;

> +	  l_owner = l_sec->owner;
> +	  l_sec = l->linked.u.sec;

Ordering problem above.  Did this compile?

Please explain why the following change is necessary.  I'd like to
know why you need to have the correct symbol on the first pass.

> +	  /* A symbol with comdat key in IR dummy BFD may override
> +	     the comdat symbol in a real BFD.  */
> +	  if (link_info.loading_lto_outputs
> +	      || (h->u.def.section->flags & SEC_LTO_COMDAT) == 0)
> +	    {
> +	      h->type = bfd_link_hash_undefweak;
> +	      h->u.undef.abfd = h->u.def.section->owner;
> +	    }
> +	  else
> +	    h->non_ir_ref = TRUE;

-- 
Alan Modra
Australia Development Lab, IBM