This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] bfd/elf.c: fix section overlap warning again


On Thu, Jul 15, 2010 at 12:49:42AM -0400, DJ Delorie wrote:
> 
> > s/segment/section/  "segment" is what the program header describes.
> 
> isn't P an "Elf_Internal_Phdr *" at that point?

Yes.  To me that means a "segment".  If you say "previous segment" in
a comment then I'm looking for the previous p.  p points to the
*current* segment as it is constructed from sections.  Thus the value
p->p_paddr + p->p_memsz is one past the last byte of the previous
section allocated to the current segment.  Oh, and another thing, the
comment wrongly says VMA when we are operating with LMAs at this point.

> > > +	      /* The first comparison checks for the usual case.  The
> > > +		 second checks for segments which overflow the address
> > > +		 space.  */
> > > +	      if (sec->lma < p->p_paddr + p->p_memsz
> > > +		  || (p_end < p->p_paddr && p->p_paddr < sec->lma))
> > 
> > Isn't it an overflow if p->p_addr == sec->lma in the second case?
> > I'd write
> > 
> > 	      if (sec->lma < p_end
> > 		  || (p_end < p->p_paddr && sec->lma >= p->p_paddr))
> 
> If "p" is the previous segment/section/whatever, then normally p->lma
> is less than sec->lma, because the new section starts after the old
> one.  One would hope, at least, else this whole chunk of logic is
> wrong.
> 
> The normal case is like this:
> 
> p->lma ... p->end ... sec->lma ... sec->end
> 
> The normal overlap case is like this:
> 
> p->lma      ...      p->end
>          sec->lma      ...      sec->end
> 
> (i.e. the new section has an AT that isn't far enough past the end of
> P)
> 
> If P overflows, you have this instead:
> 
> p->end . . . . . . p->lma
>                            sec->lma ... sec->end
> 
> So yeah, the == case should be included.  I hope nobody intentionally
> *starts* two sections at the same address!

They can be if the size of the first section is zero, which is why I
sent the followup mail you quote below.

> > Hmm, on thinking some more, it's an overflow if
> >   sec->lma + sec->size > p->p_addr
> 
> Only if P overflows, but you'd then have to also test for sec
> overflowing, and in the case I had, sec ended right at the border, so
> sec+size was 0x1_0000_0000, and pretty much guaranteed to always be
> that way (it's the reset vector's section)

Possibly we still don't have all the corner cases covered.

> The code in question doesn't test for the case where sec->lma < p->lma
> anyway, or the general O(n^2) overlap problem, just the normal "s
> follows p" overlap case.

Well, we know we don't have to handle the general case.  The sections
have been sorted by lma.  See elf_sort_sections.  Hmm.  I'm a little
confused as to how you managed to have p->p_addr + p->p_memsz
wrapping, then additional sections.  Testcase?

-- 
Alan Modra
Australia Development Lab, IBM


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]