Re: ELF bugfixes

[phcoder]

Robert Millan wrote:
> On Mon, Mar 02, 2009 at 01:35:06AM +0100, phcoder wrote:
> > +       * include/grub/elf.h: added missing attributes
>
> This should be a bit more descriptive.
>
> >    for (i = 0; i < ehdr->e_phnum; i++)
> >      if (phdr(i)->p_type == PT_LOAD && phdr(i)->p_filesz != 0)
> >        {
> > -       if (phdr(i)->p_paddr < phdr(lowest_segment)->p_paddr)
> > +	if (lowest_segment == -1 
> > +	    || phdr(i)->p_paddr < phdr(lowest_segment)->p_paddr)
> >           lowest_segment = i;
> > -       if (phdr(i)->p_paddr > phdr(highest_segment)->p_paddr)
> > +       if (highest_segment == -1
> > +           || phdr(i)->p_paddr > phdr(highest_segment)->p_paddr)
> >           highest_segment = i;
> >        }
>
> Why?

Because if first segment doesn't have the PT_LOAD attribute set then it 
should be considered in this comparison
> > -  grub_multiboot_payload_entry_offset = ehdr->e_entry - 
> > phdr(lowest_segment)->p_vaddr;
> > +  grub_multiboot_payload_entry_offset = ehdr->e_entry - 
> > phdr(lowest_segment)->p_paddr;
>
> Are you sure about this?  IIRC e_entry is in the virtual address space.  I
> think we had some trouble with this (with NetBSD?), which lead to the current
> use of p_vaddr in this line.
Actually now thinking I see that the problem is more deep. The section 
which is loaded at the lowest address isn't necessarily the section 
which contains entry point. I'll fix this part cleanly and will resubmit 
the patch
--

Regards
Vladimir 'phcoder' Serbinenko