Re: [Qemu-devel] [PATCH] linux-user: fix mremap for 64bit targets on 32bit hosts

[Felix Janda]

Peter Maydell wrote:
> On 17 September 2016 at 18:20, Felix Janda <address@hidden> wrote:
> > Signed-off-by: Felix Janda <address@hidden>
> > ---
> >  linux-user/mmap.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> > index c4371d9..4882816 100644
> > --- a/linux-user/mmap.c
> > +++ b/linux-user/mmap.c
> > @@ -682,7 +682,7 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong 
> > old_size,
> >
> >      if (flags & MREMAP_FIXED) {
> >          host_addr = (void *) syscall(__NR_mremap, g2h(old_addr),
> > -                                     old_size, new_size,
> > +                                     (size_t) old_size, (size_t) new_size,
> >                                       flags,
> >                                       g2h(new_addr));
> >
> > @@ -701,7 +701,7 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong 
> > old_size,
> >              host_addr = MAP_FAILED;
> >          } else {
> >              host_addr = (void *) syscall(__NR_mremap, g2h(old_addr),
> > -                                         old_size, new_size,
> > +                                         (size_t) old_size, (size_t) 
> > new_size,
> >                                           flags | MREMAP_FIXED,
> >                                           g2h(mmap_start));
> >              if (reserved_va) {
> > --
> > 2.7.3
> 
> Rather than this, I think it would be better to switch to
> using the mremap() library call rather than direct syscall
> here, which then matches the other mremap()s later in the
> function. (That will work right because mremap()'s prototype
> says it takes size_t arguments, whereas syscall() is a
> generic thing which doesn't, and so the C default promotions
> do the wrong thing with the abi_ulongs.)
> 
> The use of syscall(__NR_mremap, ...) originally dates back to 2008:
> https://lists.gnu.org/archive/html/qemu-devel/2008-12/msg01087.html
> https://lists.gnu.org/archive/html/qemu-devel/2008-12/msg00480.html
> 
> and was to permit compilation with glibc 2.4 which didn't
> support the 5-argument mremap() or define MREMAP_FIXED.
> 
> Since glibc 2.4 dates back to a decade ago now, we no longer
> need to carry this ugly (and buggy) workaround for it.

This sounds like a good idea. Thanks also for digging up the history.

I will prepare a new patch.

Thanks,
Felix