[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-stable] [PATCH v3 05/11] linux-user: fix mmap/munmap/mprotect/
From: |
Max Filippov |
Subject: |
Re: [Qemu-stable] [PATCH v3 05/11] linux-user: fix mmap/munmap/mprotect/mremap/shmat |
Date: |
Tue, 6 Mar 2018 09:50:49 -0800 |
On Tue, Mar 6, 2018 at 9:39 AM, Laurent Vivier <address@hidden> wrote:
> Le 06/03/2018 à 18:28, Max Filippov a écrit :
>> On Tue, Mar 6, 2018 at 4:02 AM, Laurent Vivier <address@hidden> wrote:
>>> Le 01/03/2018 à 18:36, Max Filippov a écrit :
>>>> #define GUEST_ADDR_MAX (reserved_va ? reserved_va : \
>>>> - (1ul <<
> TARGET_VIRT_ADDR_SPACE_BITS) - 1)
>>>> + (2ul << (TARGET_VIRT_ADDR_SPACE_BITS - 1)) - 1)
>>>
>>> I don't understand why you do this change. Could you explain?
>>
>> For a lot of targets TARGET_VIRT_ADDR_SPACE_BITS is the same as
>> HOST_LONG_BITS, so the shift results in undefined behavior. E.g. without
>> this change I see the following warnings when building such targets for
>> x86_64 host:
>>
>> linux-user/syscall.c:4905:10: note: in expansion of macro
> ‘guest_range_valid’
>> if (!guest_range_valid(shmaddr, shm_info.shm_segsz)) {
>> ^~~~~~~~~~~~~~~~~
>> include/exec/cpu-all.h:163:30: warning: left shift count >= width of
>> type [-Wshift-count-overflow]
>> (1ul << (TARGET_VIRT_ADDR_SPACE_BITS)) - 1)
>>
>> Changing it to 2ul << (TARGET_VIRT_ADDR_SPACE_BITS - 1)
>> fixes undefined behavior. Probably that deserves a comment in the
>> changelog.
>>
>
> Perhaps you can keep the scheme used originally with h2g_valid(), it's
> easier to read:
>
> #if HOST_LONG_BITS <= TARGET_VIRT_ADDR_SPACE_BITS
> #define GUEST_ADDR_MAX (reserved_va ? reserved_va : ~0ul)
> #else
> #define GUEST_ADDR_MAX (reserved_va ? reserved_va : \
> (1ul << TARGET_VIRT_ADDR_SPACE_BITS) - 1)
> #endif
Ok, will do.
--
Thanks.
-- Max