[stdlib] Fix cc mismatch violation on swift_isClassType

[kateinoigakukun]

_isClassType is mainly called by compiler-generated code, but it's also called by stdlib.
The call-site of stdlib used @_silgen_name to link the function, so it's called through swiftcc.
However its cc is defined as C-cc, so caller and callee used different cc. The definition is here: https://github.com/apple/swift/blob/6e3548797503b4a2bbabcebf7ffd156bc1cac245/stdlib/public/runtime/Casting.cpp#L1420-L1423

This patch adds C-compatible decl of swift_isClassType in shims so that it can be called by stdlib with C-cc.
This change doesn't break ABI because _isClassType in stdlib was defined as internal API.

I sent a similar patch #30938 before, but it breaks ABI. So I re-started it from a safe point.

CC: @MaxDesiatov @jckarter

[MaxDesiatov]

@swift-ci please smoke test

[kateinoigakukun]

I want to start a discussion about other cc violations of swift public APIs.
Other violations can be categorized into 3 types:

1. Defined as a C-cc in runtime but published as a Swift-cc public API in stdlib.

   • e.g. fwd decl and implementation defines it as a C-cc, but published as a Swift-cc API: Swift-side decl

   • swift_retainCount, swift_unownedRetainCount, swift_weakRetainCount, _swift_getObjectRuntimeFunctionCounters, _swift_getGlobalRuntimeFunctionCounters, and other RuntimeInvocationsTracking things

2. Emitted as a C-cc from user code, but called as a Swift-cc by stdlib indirectly.

   • e.g. emitted as C-cc, but called as a Swift thin function
   • keypath_get_arg_layout, keypath_destroy, keypath_copy, keypath_arg_init

3. Defined as a C-cc in runtime but linked as a Swift-cc by test binary

   • swift_demangle

[MaxDesiatov]

I want to start a discussion about other cc violations of swift public APIs.

Maybe it would be best to cross-post to Swift Forums? We would get more eyes on the issue then.

[kateinoigakukun]

@MaxDesiatov Yeah, that's just a heads-up. I'll post a topic in forum

[MaxDesiatov]

@swift-ci please test Windows platform

[compnerd]

I don't understand this. I guess I should look at the implementation. But this deserves a comment. You are passing the metatype structure as a pointer?

[kateinoigakukun]

According to here, a thick metatype is represented with a metadata pointer.
https://github.com/apple/swift/blob/dd32711a5f5c5bd28a6401fc3ede1a0bf344b49e/lib/IRGen/GenType.cpp#L2464-L2466

So the unsafeBitCast is a safe operation. But I agree this is not clear to everyone, so it should be described in comments.

[MaxDesiatov]

@swift-ci please smoke test

[rjmccall]

Contra @jckarter, functions that are just working with pointer-sized argument and return types should match between C and swiftcall, at least on the platforms that currently have stable ABIs. (This is, of course, why this code happens to work today.) So the only annotations in your original PR that seem problematic are the ones returning BoxPair, and those are already declared with swiftcall (in the header), they're just not redundantly declared that way at the implementation site.

The only concern I have about changing the CC for these functions is whether we'd have compile-time problems when linking IR coming from different translation units.

[MaxDesiatov]

The only concern I have about changing the CC for these functions is whether we'd have compile-time problems when linking IR coming from different translation units.

Should it be possible to verify that with tests? What kind of breakage should we expect?

[rjmccall]

The breakage I would expect is something like: if you LTO IR built with a previous compiler with IR built with a new compiler, you can end up with e.g. a swiftcc call to a function declared with the default CC, and we might miscompile that by deciding that the call has UB and must be unreachable. But I don't think that's a likely scenario; AFAIK Apple's use of bitcode doesn't do cross-version LTO, and it's not a natural result of a normal build process.

[MaxDesiatov]

The breakage I would expect is something like: if you LTO IR built with a previous compiler with IR built with a new compiler, you can end up with e.g. a swiftcc call to a function declared with the default CC, and we might miscompile that by deciding that the call has UB and must be unreachable.

It doesn't look to me like something that's feasible to cover with tests here. Is there anything else we can do with this PR to proceed? Do we need approvals from more people before this could be merged?

[rjmccall]

Well, so, what I'm saying is that I would prefer your old PR that actually makes these functions swiftcc, and I'd like to get @jckarter's opinions about whether he thinks that's acceptable given the CC compatibility on our stable platforms.

[jckarter]

It looks safe to use either CC for isClassType on existing platforms. The function is (at least intended to be) SPI between the runtime and stdlib, so hopefully third party binaries aren't using it to begin with.

[kateinoigakukun]

It seems there is no major use of swift_isClassType in public projects as far as I can see.
https://sourcegraph.com/search?q=context:global+swift_isClassType+-repo:%5Egithub%5C.com/apple/.*+-file:.*%5C.tbd+-file:.*%5C.bcsymbolmap+-file:.*%5C.dylib+-file:.*/swift/stdlib/.*+-file:.*/swift/include/.*&patternType=literal

Can we have approval and merge this patch?

[tbkka]

@swift-ci Please test

[tbkka]

Looks like Linux CI is having network issues. I'll try again in a little while.

[tbkka]

@swift-ci Please test

[MaxDesiatov]

@swift-ci please test

[MaxDesiatov]

Hi @tbkka, since CI is passing here, would you mind if this is merged?

[jckarter]

I'll run tests again just to make sure nothing's broken in the past couple months, then I'll merge.

[jckarter]

@swift-ci Please test