Re: PATCH: PRs binutils/3281/5037: Properly handle PT_GNU_RELRO segment

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

On Sun, Sep 16, 2007 at 12:55:41PM -0700, H.J. Lu wrote:
> +		      else if (sect->next != NULL
> +			  && (sect->next->flags & SEC_ALLOC) != 0)
> +			p->p_filesz = sect->next->lma - lp->p_vaddr;

Subtracting a vma from an lma?  This can't be correct.

> +		      else
> +			p->p_filesz += p->p_vaddr - lp->p_vaddr;

What is going on here?

>  static bfd_boolean
> -rewrite_elf_program_header (bfd *ibfd, bfd *obfd)
> +rewrite_elf_program_header (bfd *ibfd, bfd *obfd,
> +			    enum elf_section_status status)

I think the extra parameter should be "bfd_boolean remove_relro",
and put all the code dealing with whether we can keep the gnu_relro
header or not in copy_private_bfd_data.

> +  if (status == unknown)
> +    {
> +      /* We need to find out how we are changed.  */
> +      num_segments = elf_elfheader (ibfd)->e_phnum;
> +      for (i = 0, segment = elf_tdata (ibfd)->phdr;
> +	   i < num_segments;
> +	   i++, segment++)
> +	{
> +	  for (section = ibfd->sections;
> +	       section != NULL; section = section->next)
> +	    {
> +	      osec = section->output_section;
> +
> +	      /* Check if this section is covered by the segment.  */
> +	      this_hdr = &(elf_section_data(section)->this_hdr);
> +	      if (ELF_IS_SECTION_IN_SEGMENT_FILE (this_hdr, segment))
> +		{
> +		  if (osec == NULL)
> +		    status = removed;
> +		  else if (section->flags != osec->flags
> +			   || section->lma != osec->lma
> +			   || section->vma != osec->vma
> +			   || section->size != osec->size
> +			   || section->rawsize != osec->rawsize
> +			   || section->alignment_power != osec->alignment_power)
> +		    {
> +		      /* Stop if a section is modified.  */
> +		      status = modified;
> +		      break;
> +		    }
> +		}
> +	    }
> +	}
> +    }

Why copy all the above?  Can't you set "status" in the existing loop?

-- 
Alan Modra
Australia Development Lab, IBM