Re: Fwd: [PATCH v2] ld: Rewrite lang_size_relro_segment_1

[Nick Clifton <nickc@redhat.com>]

Hi H.J.

> Here is the v2 patch to track the maximum section alignment from
> the RELRO segment.  If the maximum page size >= the maximum
> section alignment, we can align the RELRO segment to the maximum
> page size.

I am having some trouble getting my head around this patch, so please
bare with me if I ask some silly questions:

    /* Find the first section in the relro segment. */
    for (sec = link_info.output_bfd->section_last; sec; sec = sec->prev)
      if ((sec->flags & SEC_ALLOC) != 0)
        {
  	if (last_sec_status != relro_sec_before
  	    && sec->alignment_power > max_alignment_power)
  	  max_alignment_power = sec->alignment_power;

  	if (sec->vma >= seg->base
  	    && sec->vma < seg->relro_end - seg->relro_offset)
  	  {
  	    relro_sec = sec;
  	    prev_sec = sec->prev;
  	    last_sec_status = relro_sec_in;
  	  }
  	else
  	  last_sec_status = relro_sec_before;
        }

Questions:
   Should a zero-sized section (with SHF_ALLOC) be treated as a
   viable candidate ?

   Once last_sec_status has been set to relro_sec_before, is there
   any point in continuing the scan ?

   If the section list contains a section which starts beyond the
   end of the segment (ie sec->vma >= seg->relro_end - seg->relro_offset)
   then a) is this an error ?  and b) should its alignment power be
   considered against max_alignment_power ?


    if (relro_sec)
      {
        /* Where do we want to put this section so that it ends as
  	 desired?  */

   By "desired" I assume you mean "with the desired alignment, but
   without any unnecessary padding".  Is that correct ?


       /* Find the first non-empty preceding section.  */
       for (; prev_sec; prev_sec = prev_sec->prev)
	if (prev_sec->size != 0 && (prev_sec->flags & SEC_ALLOC) != 0)
	  break;

   Given that prev_sec is only needed later on, inside an if-statement,
   you could move this loop - potentially saving a bit of time in some
   cases.


   Overall I think that the code could use some more comments, explaining
   what is going on with some examples of how each part of the algorithm
   operates.

Cheers
   Nick