Re: [Qemu-devel] [PATCH] linux-user: Do not subtract offset from end address

[Jonas Maebe]

On 07 Nov 2014, at 08:23, Riku Voipio wrote:

> On Thu, Nov 06, 2014 at 01:43:13PM -0600, Tom Musta wrote:
> > 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.
>
> Thanks, I'll test this and try to get it applied ASAP. Jonas, can you
> have a look and provide your Acked-by/Tested-by ?

Good catch! Sorry for not noticing that. I've verified and the patched  
version also still works with my ARM binary.

While looking at that, I noticed that the code under "#ifdef  
CONFIG_USE_FDPIC" in linux-user/elfload.c at line 1858 may need a  
similar adjustment as performed by my original patch. At least http://lxr.free-electrons.com/source/fs/binfmt_elf.c#L829 
 makes the offset adjustment both for binaries with and without a  
"load_bias". I'm not sure what this is for (some uCLinux-specific  
format?), nor do I have binaries that exercise this functionality, so  
I can't/won't provide a patch for this.

Thanks,


Jonas

> > 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;
> >             }
> > --
> > 1.7.1