[IRGen] Fix ObjC method signatures for .cxx_construct and .cxx_destruct (not 4.2)

[fay59]

This PR is the same as #17884, except that it is opened targeting master for CI purposes. It uses a different branch because Github won't let you start a second PR with the same branch to the same base, but the head is the same.

Explanation

For aeons past, the Swift compiler has emitted incorrect method signatures for .cxx_construct and .cxx_destruct methods. The signature was hard-coded to "@?", which is the Objective-C type signature for a block. Type signatures and method signatures are different, and this is not a valid method signature.

Scope

Only Swift classes that inherit from NSObject get .cxx_{construct,destruct} methods. It wouldn't be possible to call a Swift .cxx_construct or .cxx_destruct with NSInvocation, although I'm not entirely sure at which point that would fail. This bug is essentially annoying only to people who look at binaries.

Issue

https://bugs.swift.org/browse/SR-8203

Risk

Negligible. We know from experience that you can put garbage in the method signature of .cxx_destruct and it'll go unnoticed for years. In the unlikely, worst-case scenario in which I got it wrong, the garbage will have changed and someone else will step up in 2023 to fix it without anyone else ever noticing that there was a problem.

Testing

Added a test in test/IRGen/objc_methods.swift.

Reviewed By

You?

[jrose-apple]

(already reviewed on the previous PR)

[jrose-apple]

@swift-ci Please test

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - 917ad05

[fay59]

Sorry, I wasn't careful enough. 6311df3 fixes these test issues.

[fay59]

Side note: I noticed that this stripped whitespace from test/IRGen/objc_bridge.swift. How hard should I try to maintain whitespace in files where it hasn't been stripped?

[jrose-apple]

It's generally frowned upon to edit lines that aren't near what you're changing. git annotate may not be perfect, but interfering with it for no real reason is unfortunate. You just have one line in here, though, so it's not the end of the world.

[jrose-apple]

…whoops, it's more than one line. Yeah, might be better to put that back.

[fay59]

Updated it.

[jrose-apple]

Thank you!

@swift-ci Please test

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - a207347

[jrose-apple]

Fun times. On 32-bit platforms, the length of the string is different. You can use a regex match between double braces {{ }} in FileCheck lines, so something like (6|8) is probably fine.

[jrose-apple]

If you want to run these tests locally, you can invoke lit.py on the build/$SUBDIR/swift-macosx-x86_64/test-iphonesimulator-i386/ folder. (See docs/Testing.md.)

[fay59]

Why does it fail only in objc_subclass.swift? Shouldn't objc_methods.swift have the same problem?

// CHECK: [[NO_ARGS_SIGNATURE:@.*]] = private unnamed_addr constant [8 x i8] c"v16@0:8\00"
// ...
// CHECK:   [2 x { i8*, i8*, i8* }] [{
// CHECK:     i8* getelementptr inbounds ([14 x i8], [14 x i8]* @"\01L_selector_data(.cxx_destruct)", i64 0, i64 0),
// CHECK:     i8* getelementptr inbounds ([8 x i8], [8 x i8]* [[NO_ARGS_SIGNATURE]], i64 0, i64 0),
// CHECK:     i8* bitcast (void (%6*, i8*)* @"$S12objc_methods16ObjcDestructibleCfETo" to i8*) }]
// CHECK:   }]

The constant should be private unnamed_addr constant [7 x i8] c"v8@0:4\00" on a 32-bit platform, and that wouldn't match.

[jrose-apple]

It looks like that test is currently limited to x86_64 (see the "REQUIRES: CPU=x86_64" at the top). That's…not wonderful, but changing it to support the other platforms is probably a lot of extra CHECK lines.

[jrose-apple]

@swift-ci Please test

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - a207347

[swift-ci]

Build failed
Swift Test Linux Platform
Git Sha - a207347

[fay59]

Why did CI try to build a207347 when the HEAD is c625b56?

[jrose-apple]

It's a known issue with the GitHub/Jenkins integration after a force push. We added a check for it that'll auto-restart the build when it happens.

[jrose-apple]

Thanks, Félix!

[fay59]

Thank you for tending to it! Hopefully the next one will go smoother.