Re: PATCH: PR ld/4504: Copy relocation doesn't preserve section alignment

[Alan Modra <amodra at bigpond dot net dot au>]

On Mon, May 14, 2007 at 05:49:54PM -0700, H. J. Lu wrote:

I like the way you have consolidated this code, but do we really want
to waste space in .dynbss like this?  I'm thinking of cases where an
executable might need many variable in .dynbss, most of which do not
need large alignment.  Hmm, one way you could cut down on the wasted
space is to look at low bits of the symbol address in the shared lib.

> --- bfd/elflink.c.dynbss	2007-05-14 09:01:48.000000000 -0700
> +++ bfd/elflink.c	2007-05-14 17:43:58.000000000 -0700
> @@ -2649,6 +2649,45 @@ _bfd_elf_adjust_dynamic_symbol (struct e
>    return TRUE;
>  }
>  
> +/* Adjust the dynamic symbol, H, for copy in the dynamic bss section,
> +   DYNBSS.  */
> +
> +bfd_boolean
> +_bfd_elf_adjust_dynamic_copy (struct elf_link_hash_entry *h,
> +			      asection *dynbss)
> +{
> +  unsigned int power_of_two, orig_power_of_two;
> +  asection *sec = h->root.u.def.section;
> +
> +  /* The section aligment of definition is the maximum alignment
> +     requirement of symbols defined in the section.  Since we don't
> +     know the alignment requirement of individual symbols, we apply
> +     the maximum alignment to all symbols needing dynamic copy.  */
> +  power_of_two = bfd_get_section_alignment (dynbss->owner, dynbss);
> +  orig_power_of_two = bfd_get_section_alignment (sec->owner, sec);

so, add

  bfd_vma mask;

  mask = ((bfd_vma) 1 << orig_power_of_two) - 1;
  while ((h->root.u.def.value & mask) != 0)
    {
      mask >>= 1;
      --orig_power_of_two;
    }

and of course fix your comment.

> +
> +  if (orig_power_of_two > power_of_two)
> +    {
> +      /* Adjust the section alignment if needed.  */
> +      if (! bfd_set_section_alignment (dynbss->owner, dynbss,
> +				       orig_power_of_two))
> +	return FALSE;
> +    }
> +
> +  /* We make sure that the symbol will be aligned properly.  */
> +  dynbss->size = BFD_ALIGN (dynbss->size,
> +			    (bfd_size_type) (1 << orig_power_of_two));

Cast in the wrong place here.  You need a bfd_size_type 1.  (Or just
use "mask + 1" now that we have already performed the shift.)

> +  /* Define the symbol as being at this point in DYNBSS.  */
> +  h->root.u.def.section = dynbss;
> +  h->root.u.def.value = dynbss->size;
> +
> +  /* Increment the size of DYNBSS to make room for the symbol.  */
> +  dynbss->size += h->size;
> +
> +  return TRUE;
> +}
> +
>  /* Adjust all external symbols pointing into SEC_MERGE sections
>     to reflect the object merging within the sections.  */
>  

OK with those changes.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre