Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 8 Sep 2017 13:10:29 -0400
From:      John Baldwin <jhb@FreeBSD.org>
To:        Ian Lepore <ian@freebsd.org>, Maxim Sobolev <sobomax@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r323254 - head/sys/compat/freebsd32
Message-ID:  <c03c4eda-2a97-e934-1115-07acabf5c814@FreeBSD.org>
In-Reply-To: <1504882598.32063.3.camel@freebsd.org>
References:  <201709070429.v874Tvom033960@repo.freebsd.org> <6d00a409-2be8-fc15-b68f-993d97366aab@FreeBSD.org> <1504882598.32063.3.camel@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 9/8/17 10:56 AM, Ian Lepore wrote:
> On Fri, 2017-09-08 at 08:14 -0400, John Baldwin wrote:
>> On 9/7/17 12:29 AM, Maxim Sobolev wrote:
>>>
>>> Author: sobomax
>>> Date: Thu Sep  7 04:29:57 2017
>>> New Revision: 323254
>>> URL: https://svnweb.freebsd.org/changeset/base/323254
>>>
>>> Log:
>>>   In the recvmsg32() system call iterate over returned structure(s)
>>>   and convert any messages of types SCM_BINTIME, SCM_TIMESTAMP,
>>>   SCM_REALTIME and SCM_MONOTONIC from 64-bit to its 32-bit
>>>   representation. Otherwise we either run out of user-supplied
>>>   buffer to copy those out resulting in the MSG_CTRUNC or simply
>>>   return values that the userland 32-bit code is not going
>>>   to parse correctly. This fixes at least two regression tests
>>>   failing to function properly in 32-bit compat mode:
>>>   
>>>       tools/regression/sockets/udp_pingpong
>>>       tools/regression/sockets/unix_cmsg
>>>   
>>>   PR:             kern/222039
>>>   MFC after:	30 days
>> Is this correct on !amd64?  Other 32-bit platforms use a 64-bit time_t
>> (note the time32_t type defined earlier in freebsd32.h).  struct bintime32
>> should use time32_t for the seconds field, not uint32_t.  I think that
>> will be sufficient to make this correct on !amd64 (it also means that
>> bintime32 == bintime on !amd64 so you could perhaps use a simpler BT_CP
>> for !amd64, but the existing one is probably ok).
>>
> 
> The existing one now does *(uint64_t *) on a value that's only aligned
> to a 32-bit boundary.  That will work in practice because only i386 has
> a 32-bit time_t that will use this code, and it's not a strict-
> alignment platform.  It may still cause compiler warnings about
> alignment.

Mmmm, the code is used on ppc and mips as well (and mips will fault for
unaligned access at least).  However, once bintime32 is fixed to use
time32_t I think the fraction will be 64-bit aligned?  I would be fine
with BT_CP() being an #ifdef though that just uses "(dst) = (src)" for
!amd64.

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?c03c4eda-2a97-e934-1115-07acabf5c814>