[SR-15871] [IRGen] Crash while compiling PythonKit in release mode

[philipturner]

Previous ID	SR-15871
Radar	None
Original Reporter	@philipturner
Type	Bug

Attachment: Download

Additional Detail from JIRA

Votes	0
Component/s
Labels	Bug
Assignee	None
Priority	Medium

md5: e94e403b786cb56d8c14e47dcd662f6b

Issue Description:

PythonKit recently added lambdas and subclassing, which requires the "PyCapsule" API. The PyCapsule_New function takes a function pointer as a parameter, which is @convention(c).

The compiler crashed in release mode when it encountered the new PyCapsule_New function pointer. This crash went away when the closure parameter was removed, or the @convention(c) decorator was removed from the closure. The crash also went away when compiling in debug mode. The following code reproduces the crash.

public func pythonObject() {
  _ = PyCapsule_New({ _ in })
}

let PyCapsule_New: (@convention(c) (UnsafeMutableRawPointer?) -> Void) -> Void =
  loadSymbol(name: "PyCapsule_New")

func loadSymbol<T>(name: String, type: T.Type = T.self) -> T {
  fatalError()
}

The crash happened on toolchains varying from November 12, 2021 to February 03, 2022. This means it has existed since at least November 2021. A log is attached below as "log.txt". I narrowed down PythonKit, erasing everything except one file, which includes the excerpt above. Then, I compiled with swift build -c release.

Here is the shortened crash stack trace:

1.  Apple Swift version 5.7-dev (LLVM cd2992b90c95d01, Swift ab85807f0646caa)
2.  Compiling with the current language version
3.  While emitting IR SIL function "@$s9PythonKit13PyCapsule_New_WZ".
 for declaration 0x14e94d890 (at /Users/philipturner/Documents/building-tensorflow/PythonKit/PythonKit/Python.swift:5:1)
...
6  swift-frontend           0x000000010512894c (anonymous namespace)::IRGenDebugInfoImpl::getOrCreateType(swift::irgen::DebugTypeInfo) (.cold.30) + 0
7  swift-frontend           0x0000000100f8c214 bool swift::SILLocation::isNode<swift::ConstructorDecl>(llvm::PointerUnion<swift::Stmt*, swift::Expr*, swift::Decl*, swift::Pattern*>) const + 0
8  swift-frontend           0x0000000100f8c974 (anonymous namespace)::IRGenDebugInfoImpl::createParameterType(llvm::SmallVectorImpl<llvm::Metadata*>&, swift::SILType) + 124
9  swift-frontend           0x0000000100f8c6d4 (anonymous namespace)::IRGenDebugInfoImpl::createParameterTypes(swift::CanTypeWrapper<swift::SILFunctionType>) + 404
10 swift-frontend           0x0000000100f85f14 (anonymous namespace)::IRGenDebugInfoImpl::emitFunction(swift::SILDebugScope const*, llvm::Function*, swift::SILFunctionTypeRepresentation, swift::SILType, swift::DeclContext*) + 1732
11 swift-frontend           0x0000000100f856b4 (anonymous namespace)::IRGenDebugInfoImpl::getOrCreateScope(swift::SILDebugScope const*) + 948
12 swift-frontend           0x0000000100f85478 (anonymous namespace)::IRGenDebugInfoImpl::getOrCreateScope(swift::SILDebugScope const*) + 376
13 swift-frontend           0x0000000100f84650 (anonymous namespace)::IRGenDebugInfoImpl::setCurrentLoc(swift::irgen::IRBuilder&, swift::SILDebugScope const*, swift::SILLocation) + 80
14 swift-frontend           0x0000000100fada5c (anonymous namespace)::IRGenSILFunction::emitSILFunction() + 7556
15 swift-frontend           0x0000000100fab6e4 swift::irgen::IRGenModule::emitSILFunction(swift::SILFunction*) + 896
16 swift-frontend           0x0000000100e9abc0 swift::irgen::IRGenerator::emitLazyDefinitions() + 1400
17 swift-frontend           0x0000000100f75118 swift::performIRGeneration(swift::ModuleDecl*, swift::IRGenOptions const&, swift::TBDGenOptions const&, std::__1::unique_ptr<swift::SILModule, std::__1::default_delete<swift::SILModule> >, llvm::StringRef, swift::PrimarySpecificPaths const&, llvm::ArrayRef<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, llvm::GlobalVariable**) + 1360
18 swift-frontend           0x0000000100c03bac performCompileStepsPostSILGen(swift::CompilerInstance&, std::__1::unique_ptr<swift::SILModule, std::__1::default_delete<swift::SILModule> >, llvm::PointerUnion<swift::ModuleDecl*, swift::SourceFile*>, swift::PrimarySpecificPaths const&, int&, swift::FrontendObserver*) + 1636
19 swift-frontend           0x0000000100c031d8 swift::performCompileStepsPostSema(swift::CompilerInstance&, int&, swift::FrontendObserver*) + 1028
20 swift-frontend           0x0000000100c04c74 swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*) + 2940
21 swift-frontend           0x0000000100b40900 swift::mainEntry(int, char const**) + 500
22 dyld                     0x000000010d6190f4 start + 520

[slavapestov]

Incorrect reconstructed type for $syySvSgXCcXMTD
Original type:
(metatype_type thick
(function_type escaping
 (input=function_params num_params=1
   (param
     (function_type representation=c
       (input=function_params num_params=1
         (param
           (bound_generic_enum_type decl=Swift.(file).Optional
             (struct_type decl=Swift.(file).UnsafeMutableRawPointer))))
       (output=tuple_type num_elements=0))))
 (output=tuple_type num_elements=0)))
Reconstructed type:
(metatype_type thick
(function_type escaping
 (input=function_params num_params=1
   (param
     (function_type representation=c escaping
clang_type=PointerType 0x14e8d0e30 'void (*)(void *)'
`-FunctionProtoType 0x14e8d0e00 'void (void *)' cdecl
|-BuiltinType 0x14e81aa70 'void'
`-PointerType 0x14e81b7b0 'void *'
 `-BuiltinType 0x14e81aa70 'void'
       (input=function_params num_params=1
         (param
           (bound_generic_enum_type decl=Swift.(file).Optional
             (struct_type decl=Swift.(file).UnsafeMutableRawPointer))))
       (output=tuple_type num_elements=0))))
 (output=tuple_type num_elements=0)))

[philipturner]

#41393

[asl]

I'm unable to reproduce the issue (from the minimal reproducer) on ec20619

@philipturner will you please confirm whether it is fixed?

[philipturner]

The build failed, although my toolchain may not have been built 100% correctly. I added this in log2.txt. Could you reproduce the absence of a crash on arm64 macOS? log2.txt

[asl]

Sorry, my bad. Apparently I forgot to add both -g and -O

[asl]

This looks like a demangler issue. We always demangle function types with @convention(c) as @escaping. However, mangler simply ignores @escaping flag for such functions for obvious reasons. I guess the function with "C" calling convention cannot be @escaping, but maybe I'm missing something? Should we diagnose such cases as currently we do allow the following:

let PyCapsule_New: (@convention(c) @escaping (UnsafeMutableRawPointer?) -> Void) -> Void =
  loadSymbol(name: "PyCapsule_New")

[philipturner]

@asl I used your workaround to fix the PythonKit crash. See PR #50 on PythonKit for further information.

The crash seems to disappear if I compile using the Swift 5.5.2 release toolchain that ships with Xcode. But it does appear if I use the 5.5.3 release toolchain from Swift.org.

[asl]

@philipturner what was the crash?

[philipturner]

The same crash that is demonstrated in the logs above. For reference, I added a crash log from compiling PythonKit on the March 9, 2022 toolchain. Previous logs just showed crashes from the reproducer, not PythonKit itself.

pythonkit_log.txt

[asl]

In order to resolve the issue I'd like to hear some input from fellow swift-ers. Tagging @slavapestov@DougGregor

Here is the full story:

• Mangler essentially ignores @escaping / @noescape flag on convention(c) pointers. This does make sense to me as I cannot imagine how a C/C++ function could capture something from Swift world (please correct me if I'm wrong)

• Demangler reconstructs the type to the default flag. As a result, noescape C function name will be demangled as escaping

I'm seeing two solutions of this problem:

• Fix demangler. And disallow @escaping convention(c) functions (I do have patch for this). However, there are some tests which explicitly do @escaping convention(c). One prominent example is test/decl/protocol/special/Sendable_swift6.swift among the others.

• If indeed we need to distinguish escaping and noescaping C function pointers, then we'd need to extend mangler in order to do so. Likely one of the cases (escaping) should use the present mangling for backward compatibility and we'd need to add additional case for noescaping functions.

Please advice 🙂

[philipturner]

For the first solution mentioned above, it would break existing code that works with Swift right now. The unit tests could be modified to remove @convention(c) @escaping, but PythonKit and possibly other code bases have them. Maybe this combination of attributes could be prohibited in Swift 6.0, which could come with source-breaking changes.

There is a precedent to a source-breaking change without a major version change. After concurrency was released, someone realized that pointers should not conform to Sendable. Consequently, some code that ran in Swift 5.5 might fail to compile in Swift 5.6.

[asl]

@philipturner The existing code is already potentially broken. As we cannot distinguish between escaping and noescaping C functions (at symbol level). Certainly "disallow" could be downgraded from error to, say, warning. But we're talking about intended semantics here (which should come first). We can decide wrt the backward compatibility things later on. I do not see why PythonKit added @escaping here (as I mentioned, this was a temporary workaround, not the final solution). Likely this hastily change would need to be reverted.

[philipturner]

I added @escaping to PythonKit as part of the bug workaround mentioned above, but I can definitely make a PR to revert that in the future. For example, it could have something like this:

#if swift(>=5.7)
... (function with @convention(c))
#else
... (function with @escaping @convention(c))
#endif

That would make it compile without crashing on all Swift versions, except a few months worth of developer snapshots spanning from winter to summer 2022. This would be more tedious for me, because my work on S4TF relies on being able to test every snapshot that comes out for AutoDiff bug fixes. I would also need to copy that into Swift-Colab 2.0, which contains a copy of PythonKit's source files. Swift-Colab 2.0 could technically work for the snapshots ranging from winter to summer 2022. I would just have to hard-code a workaround that detects dates in that range and modifies JIT compilation accordingly. Overall, I think the correct choice is to make @escaping @convention(c) illegal.

[philipturner]

Nevermind. The @escaping attribute is removed entirely in pvieito/PythonKit#53.

[asl]

Fix submitted as #42189

[asl]

Fixed in #42478