[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] linux-user/syscall: Silence warning from the undefined behav
From: |
Laurent Vivier |
Subject: |
Re: [PATCH] linux-user/syscall: Silence warning from the undefined behavior sanitizer |
Date: |
Fri, 12 Feb 2021 01:35:41 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 |
Le 11/02/2021 à 22:28, Laurent Vivier a écrit :
> Le 11/02/2021 à 14:29, Thomas Huth a écrit :
>> When compiling QEMU with -fsanitize=undefined, there is a warning when
>> running "make check-tcg":
>>
>> TEST linux-test on m68k
>> ../linux-user/syscall.c:10499:34: runtime error: member access within
>> misaligned address 0x00008006df3c for type 'struct linux_dirent64',
>> which requires 8 byte alignment
>> 0x00008006df3c: note: pointer points here
>> 00 00 00 00 68 03 28 00 00 00 00 00 5b 96 3e e4 61 4b 05 26 18 00 04 2e
>> 00 00 00 00 da 3f 18 00
>> ^
>>
>> It's likely not an issue in reality, since I assume that on hosts where
>> the alignment really matters (like sparc64), the Linux kernel likely
>> adds the right padding. Anyway, let's use the stw_p() / stq_p() accessor
>> helpers here to silence the warning and thus to allow to compile the code
>> with -fsanitize=undefined, too.
Wait... if the alignment differs between m68k and the host, I guess the size of
the structure differs?
In this case we cannot use the guest memory to call the host syscall, we must
allocate a host
structure and copy the values into the guest structure.
Thanks,
Laurent
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> linux-user/syscall.c | 16 +++++++++-------
>> 1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 34760779c8..50de535ade 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -10491,20 +10491,22 @@ static abi_long do_syscall1(void *cpu_env, int
>> num, abi_long arg1,
>> return -TARGET_EFAULT;
>> ret = get_errno(sys_getdents64(arg1, dirp, count));
>> if (!is_error(ret)) {
>> - struct linux_dirent64 *de;
>> + char *de;
>> int len = ret;
>> int reclen;
>> - de = dirp;
>> + de = (char *)dirp;
>> + #define de64(x) offsetof(struct linux_dirent64, x)
>
> Do we really need the cast to the "(char *)"?
>
> can't we use "&de->XXX" with the accessors?
> We don't access the memory, only read the address, the compiler should be
> happy.
>
>
>> while (len > 0) {
>> - reclen = de->d_reclen;
>> + reclen = lduw_he_p(de + de64(d_reclen));
>
> to avoid human error, it would be better to let the compiler take the good
> accessor:
>
> ldn_he_p(&de->d_reclen, sizeof(de->d_reclen))
>
>> if (reclen > len)
>> break;
>> - de->d_reclen = tswap16(reclen);
>> - tswap64s((uint64_t *)&de->d_ino);
>> - tswap64s((uint64_t *)&de->d_off);
>> - de = (struct linux_dirent64 *)((char *)de + reclen);
>> + stw_p(de + de64(d_reclen), reclen);
>> + stq_p(de + de64(d_ino), ldq_he_p(de + de64(d_ino)));
>> + stq_p(de + de64(d_off), ldq_he_p(de + de64(d_off)));
>
> and stwn_he_p() here too.
>
>> + de += reclen;
>> len -= reclen;
>> }
>> + #undef de64
>> }
>> unlock_user(dirp, arg2, ret);
>> }
>>
>
> Thank you Thomas for your help.
>
> Laurent
>