From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57415) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xnt9U-0003z2-7L for qemu-devel@nongnu.org; Mon, 10 Nov 2014 12:53:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xnt9L-0004RF-60 for qemu-devel@nongnu.org; Mon, 10 Nov 2014 12:53:20 -0500 Message-ID: <5460FB7D.80406@gmail.com> Date: Mon, 10 Nov 2014 11:53:01 -0600 From: Tom Musta MIME-Version: 1.0 References: <1415302993-26599-1-git-send-email-tommusta@gmail.com> <545EB3C1.2090603@suse.de> In-Reply-To: <545EB3C1.2090603@suse.de> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH] linux-user: Do not subtract offset from end address List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?ISO-8859-15?Q?Andreas_F=E4rber?= , qemu-devel@nongnu.org, qemu-ppc@nongnu.org Cc: riku.voipio@iki.fi 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 >> --- >> >> 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; >> } >> > >