SR-10333: Calendar.current.dateComponents([.weekday], from: Date()).weekday segfaults

[spevans]

• Override _NSCopyOnWriteCalendar._cfObject to return the backingCalendar created
  from CFCalendarCopyCurrent().

[spevans]

@swift-ci test

[spevans]

@swift-ci test

[spevans]

cc @millenomi @weissi

Not sure if this is the correct fix or even complete but it seemed to make sense.

[weissi]

Wow thanks Simon! As you say, looks like it makes sense, deferring to @millenomi for a proper review :)

[spevans]

Hmm.. This was actually removed in 662f631, I wonder if that was a mistake or there was a good reason for it.

[spevans]

@swift-ci test

[millenomi]

This isn't consistent with the semantics of Darwin's copy on write. If you get a COW calendar and you pass it to CF calls that alter the calendar, the backing calendar must not be altered if it hasn't been copied already.

We should treat ._cfObject as a copying operation here if we take this patch.

I also have an additional objection here: bridging should work with this object without need to alter ._cfObject. What is the underlying bug here exactly?

[millenomi]

i.e.: if I pass NSCOW to CF functions, ._cfObject should be self, and they should bridge into _CFSwiftCalendar…, and they should invoke NSCOW's methods.

[spevans]

What would be the correct way to retrieve needsLocking_backingCalendar? It is protected by an NSLock() so I dont see how just an unsafeBitCast() would be correct?

Note the error is coming from the call:

return _CFCalendarDecomposeAbsoluteTimeV(_cfObject, date.timeIntervalSinceReferenceDate, compDesc, vecBuffer.baseAddress!, Int32(compDesc.count - 1))

where _cfObject ends up being a cast of an empty NSCalendar otherwise and ultimately _locale is set to nil which causes the bad access.

[millenomi]

Here's what's happening:

1. return _CFCalendarDecomposeAbsoluteTimeV(_cfObject /* (1) this call needs to execute */, date.timeIntervalSinceReferenceDate, compDesc, vecBuffer.baseAddress!, Int32(compDesc.count - 1))

2. NSCalendar._cfCalendar is invoked with self == the NSCOW calendar. It returns return unsafeBitCast(self, to: CFCalendar.self) (this is correct)

3. In a perfect world, this would bridge to an appropriate Swift API. But that call isn't bridging; which is somewhat correct — there is no method for it to bridge to. :(

So this needs to perhaps be figured out longer-term, but: your patch is correct if and only if you add a call like this:

var _cfObject: … {
     copyBackingCalendarIfNeededWithMutation { _ in }
     return self.backingCalendar._cfObject
}

so that any CF mutation function (e.g. CFCalendarSetTimeZone) does not change the original calendar.

[spevans]

Ive added the copy, but just thinking when calling not-mutating _CFCalendar...() functions inside NSCalendar could we add an extra private var _cfObjectRO... which doesn't do the copy to use for those specific calls?

[spevans]

@swift-ci test

[spevans]

@swift-ci test

[millenomi]

@spevans Yeah, that works.

[spevans]

@swift-ci test and merge

[spevans]

@swift-ci test and merge