RE: [boost] Re: xtime_get unsafe on Windows for IA64

From: Ben Hutchings (ben.hutchings_at_[hidden])
Date: 2004-08-27 10:45:39

Michael Glassford <glassfordm_at_[hidden]> wrote:
> Ben Hutchings wrote:
> > The implementation of the boost::xtime_get() function in
> > libs/thread/src/xtime.cpp includes the following code for Windows:
<snip>
> > The pointer casts are not safe because although the representations
> > of FILETIME and __int64 are compatible, FILETIME is only 32-bit-
> > aligned whereas __int64 must be 64-bit-aligned on IA64 (in some
> > processor modes).
> > 
> > One can instead use union { FILETIME ft; __int64 i64; } and read out
> > i64, though I'm not sure this code should be unconditionally using
> > __int64 anyway.  There is a longer explanation at
> > <http://weblogs.asp.net/oldnewthing/archive/2004/08/25/220195.aspx>.
> 
> How about this instead:
> 
>      const boost::uint64_t TIMESPEC_TO_FILETIME_OFFSET =
>          (static_cast<boost::uint64_t>(27111902UL) << 32)
>          + 3577643008UL;

I think this should use the UINT64_C macro instead of messing about
with casts.

>      const boost::uint64_t ft64 =
>          (static_cast<boost::uint64_t>(ft.dwHighDateTime) << 32)
>          + ft.dwLowDateTime;
> 
>      xtp->sec = static_cast<int>(
>          (ft64 - TIMESPEC_TO_FILETIME_OFFSET) / 10000000
>      );

The cast should be to int_fast64_t rather than to int.

>      xtp->nsec = static_cast<int>((
>              ft64 - TIMESPEC_TO_FILETIME_OFFSET
>              - (static_cast<boost::uint64_t>(xtp->sec) * 10000000)
>          ) * 100
>      );
> 
> ?

The cast should be to int_fast32_t.  Also the use of multiplication
and subtraction to calculate the remainder seems to be a premature
optimisation that obscures what's really going on.  A smart compiler
can combine the division and remainder operations, and I have
confirmed that Visual C++ 7.1 does so when generating optimised i386
code (even for 64-bit division where it uses a subroutine rather than
a single instruction).  So I would suggest:

     static const boost::uint64_t TIMESPEC_TO_FILETIME_OFFSET =
        UINT64_C(116444736000000000);

     const boost::uint64_t ft64 =
         (static_cast<boost::uint64_t>(ft.dwHighDateTime) << 32)
         + ft.dwLowDateTime;
     // or use the union trick which MS appears to have blessed

     xtp->sec = static_cast<int_fast64_t>(
         (ft64 - TIMESPEC_TO_FILETIME_OFFSET) / 10000000
     );
     xtp->nsec = static_cast<int_fast32_t>(
         ((ft64 - TIMESPEC_TO_FILETIME_OFFSET) % 10000000) * 100
     );

Ben.