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 <th...@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 >