swiftlang / swift-corelibs-foundation

The Foundation Project, providing core utilities, internationalization, and OS independence
swift.org
Apache License 2.0
5.27k stars 1.13k forks source link

[SR-14288] CFRunLoop assumes mach_absolute_time() is in TSR units #4220

Open swift-ci opened 3 years ago

swift-ci commented 3 years ago
Previous ID SR-14288
Radar rdar://problem/74876063
Original Reporter 3405691582 (JIRA User)
Type Bug
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Foundation | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: 5562f0c8b2baa9b93e8beb0f428cabc4

Issue Description:

There are several assumptions in CFRunLoop that assume that the units obtained from mach_absolute_time() and __CFTimeIntervalToTSR are commensurate.

mach_absolute_time(), on non-Windows platforms, is the value of CLOCK_MONOTONIC converted into nanosecond units.

In CFDate.c, when clock_getres indicates that CLOCK_MONOTONIC has 1 ns resolution, __CFTSRRate is 1000000000 (the number of nanoseconds in 1 second). __CFTimeIntervalToTSR multiplies the CFTimeInterval – which is a double of fractional seconds – by __CFTSRRate. In other words, when __CFTSRRate is 1000000000, __CFTimeIntervalToTSR essentially converts fractional seconds to nanoseconds, and TSR units are identical to nanosecond units.

However, if clock_getres indicates that CLOCK_MONOTONIC has greater than 1 ns resolution, this no longer holds. __CFTSRRate will be set to that resolution multiplied by 1000000000. In this case, this means __CFTimeIntervalToTSR multiplies fractional seconds by the granularity and 1000000000. TSR units now are not equal to nanosecond units.

For example, with a 41 ns CLOCK_MONOTONIC resolution, __CFTimeIntervalToTSR is 41000000000 TSR units. CFRunLoopTimerCreate sets _fireTSR by adding mach_absolute_time() to the duration of the timer converted to TSR units with __CFTimeIntervalToTSR(). With a 0.5 second timer, this is 20500000000 TSR units, but this is added directly to mach_absolute_time() resulting in a _fireTSR equivalent to 20.5 seconds in the future and not 0.5 seconds.

Therefore, performing arithmetic with mach_absolute_time() and TSR units is incorrect as currently written.

It is a simple task to write a __CFNanosecondsToTSR that returns its input on platforms with a 1ns clock resolution, and otherwise converts the nanosecond input to a CFTimeInterval before converting with __CFTimeIntervalToTSR.

However, it is not quite obvious that the TSR calculations in CFDate.c are what is necessarily intended. If, for example, I want to know how many CLOCK_MONOTONIC ticks are in a given number seconds, I want to convert this to nanoseconds, then divide by the resolution to get clock-granular units.

For example, with a granularity of 41 ns, 0.5 seconds is 500000000 ns, so this is 12195121.9512 clock-granular units.

This isn't how clock-granular units are actually used in CFRunLoop, in the alternative, it may instead be reasonable to instead remove the clock_getres calculations altogether and instead treat TSR units as synonymous to nanosecond conversions for the purposes of this module; indeed, TSR units are only publicly used in CFRunLoop.

typesanitizer commented 3 years ago

@swift-ci create

3405691582 commented 2 years ago

Fixed in apple/swift-corelibs-foundation#3004.

3405691582 commented 2 years ago

Unfortunately, this fix may not be complete since we needed to roll the fix back. The conditions which demonstrated this bug no longer hold, unfortunately, so a proper fix may need to be revisited in future by future porters if needed.