Fixed
phcoder wrote:
> 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