Re: [PATCH] Add Prefer_MAP_32BIT_EXEC for Silvermont

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Fri, Dec 11, 2015 at 8:23 AM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
> On 11-12-2015 13:59, H.J. Lu wrote:
>> On Fri, Dec 11, 2015 at 7:39 AM, Joseph Myers <joseph@codesourcery.com> wrote:
>>> > On Fri, 11 Dec 2015, H.J. Lu wrote:
>>> >
>>>> >> +++ b/sysdeps/unix/sysv/linux/x86_64/64/mmap.c
>>>> >> @@ -0,0 +1,49 @@
>>>> >> +/* Copyright (C) 2015 Free Software Foundation, Inc.
>>> >
>>> > All new files should have a descriptive first line before the copyright
>>> > notice.
>>> >
>> Here is the updated patch.
>
>>
>> diff --git a/sysdeps/unix/sysv/linux/x86_64/64/mmap.c b/sysdeps/unix/sysv/linux/x86_64/64/mmap.c
>> new file mode 100644
>> index 0000000..c34f633
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/x86_64/64/mmap.c
>
>> +
>> +__ptr_t
>> +__mmap (__ptr_t addr, size_t len, int prot, int flags, int fd, off_t offset)
>> +{
>> +  /* If the Prefer_MAP_32BIT_EXEC bit is set, try to map executable pages
>> +     with MAP_32BIT first.  */
>> +  if (addr == NULL
>> +      && (prot & PROT_EXEC) != 0
>> +      && HAS_ARCH_FEATURE (Prefer_MAP_32BIT_EXEC))
>> +    {
>> +      addr = (__ptr_t) INLINE_SYSCALL (mmap, 6, addr, len, prot,
>> +                                    flags | MAP_32BIT,
>> +                                    fd, offset);
>> +      if (addr != MAP_FAILED)
>> +     return addr;
>> +    }
>> +  return (__ptr_t) INLINE_SYSCALL (mmap, 6, addr, len, prot, flags,
>> +                                fd, offset);
>> +}
>> +
>
> I would advise not add another syscall variant implementation, but rather
> work on make a generic implementation with adjustments made by each platform.
> Something like
>
> __ptr_t
> __mmap (__ptr_t addr, size_t len, int prot, int flags, int fd, off_t offset)
> {
>   flags = MMAP_ARCH_FLAGS (flags);
>   return (__ptr_t) INLINE_SYSCALL( mmap, 6, addr, len, prot, flags, fd,  offset);
> }
>
> And then define MMAP_ARCH_FLAG for x86 to add the MAP_32BIT when required.
>

This won't work here since we fallback to the default mmap when
MAP_32BIT fails.

-- 
H.J.