[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH] linux-user: Do not subtract offset f
From: |
Tom Musta |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH] linux-user: Do not subtract offset from end address |
Date: |
Mon, 10 Nov 2014 11:53:01 -0600 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 |
On 11/8/2014 6:22 PM, Andreas Färber wrote:
> Am 06.11.2014 um 20:43 schrieb Tom Musta:
>> When computing the upper address of a program segment, do not subtract the
>> offset from the virtual address; instead compute the sum of the virtual
>> address
>> and the memory size.
>
> Note that this reads a bit weird as both old and new code are adding,
> not subtracting.
>
> Regards,
> Andreas
>
I agree that it is not obvious from the patch, which needed one more line of
context:
abi_ulong a = phdr[i].p_vaddr - phdr[i].p_offset;
if (a < loaddr) {
loaddr = a;
}
a = phdr[i].p_vaddr + phdr[i].p_memsz;
if (a > hiaddr) {
hiaddr = a;
}
I think the description accurately captures what is being changed in the code.
But if you still disagree, I will reword and respin V2.
>>
>> Signed-off-by: Tom Musta <address@hidden>
>> ---
>>
>> Please include this patch in QEMU 2.2.
>>
>> Commit a93934fecd4dffc9d4b452b670c9506be5dea30d injected a regression of
>> Linux
>> User Mode that I was able to detect on PowerPC 64 (but not x86). I suspect
>> that
>> large page size on the host has something to do with it. In any case, that
>> commit
>> adjusted the lower address of a program segment by the program header's
>> offset
>> field. However, it also inadvertantly adjusted the upper address by the
>> offset also.
>>
>> linux-user/elfload.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index 84123ba..e2596a4 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -1824,7 +1824,7 @@ static void load_elf_image(const char *image_name, int
>> image_fd,
>> if (a < loaddr) {
>> loaddr = a;
>> }
>> - a += phdr[i].p_memsz;
>> + a = phdr[i].p_vaddr + phdr[i].p_memsz;
>> if (a > hiaddr) {
>> hiaddr = a;
>> }
>>
>
>