[SR-12206] Symbol collision when multiple enum cases with same base name have default arguments

[jckarter]

Previous ID	SR-12206
Radar	rdar://problem/59496023
Original Reporter	@jckarter
Type	Sub-task

Additional Detail from JIRA

Votes	0
Component/s	Compiler
Labels	Sub-task
Assignee	None
Priority	Medium

md5: 536212832ea3b9588bb83c4037b9f167

Parent-Task:

• SR-14638 ☂️ Full support for overloaded enum cases (SE-0155)

Issue Description:

This:

enum Foo {
  case foo(bar: Int = 0)
  case foo(bas: Int = 0)
  case bar
}

crashes because we mangle the same symbol name for both default argument generators:

Assertion failed: (F->empty() && "already emitted function?!"), function preEmitFunction, file /Users/jgroff/swift/public/swift/lib/SILGen/SILGen.cpp, line 727.
Stack dump:
0.  Program arguments: /Users/jgroff/swift/public/build/Ninja-RelWithDebInfoAssert/swift-macosx-x86_64/bin/swift -frontend -c -primary-file /Users/jgroff/foo.swift -target x86_64-apple-darwin19.0.0 -enable-objc-interop -color-diagnostics -module-name foo -o /var/folders/t7/glhhlkys4272r_9964jlvp4c0000gn/T/foo-7e9e81.o 
1.  Swift version 5.2-dev (LLVM 9505d8980c, Swift 1403e75a0e)
2.  While evaluating request SILGenSourceFileRequest(SIL Generation for file "/Users/jgroff/foo.swift")
0  swift                    0x000000010ca52328 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 40
1  swift                    0x000000010ca51575 llvm::sys::RunSignalHandlers() + 85
2  swift                    0x000000010ca52926 SignalHandler(int) + 262
3  libsystem_platform.dylib 0x00007fff6725db1d _sigtramp + 29
4  swift                    0x000000010f264398 cmark_strbuf__initbuf + 190843
5  libsystem_c.dylib        0x00007fff67133a08 abort + 120
6  libsystem_c.dylib        0x00007fff67132cc2 err + 0
7  swift                    0x000000010cba56a3 swift::Lowering::SILGenModule::preEmitFunction(swift::SILDeclRef, llvm::PointerUnion<swift::ValueDecl*, swift::Expr*>, swift::SILFunction*, swift::SILLocation) (.cold.6) + 35
8  swift                    0x0000000108cdd853 swift::Lowering::SILGenModule::preEmitFunction(swift::SILDeclRef, llvm::PointerUnion<swift::ValueDecl*, swift::Expr*>, swift::SILFunction*, swift::SILLocation) + 787
9  swift                    0x0000000108ce5544 swift::Lowering::SILGenModule::emitDefaultArgGenerator(swift::SILDeclRef, swift::ParamDecl*)::$_4::operator()(swift::SILFunction*) const + 116
10 swift                    0x0000000108cdf02a swift::Lowering::SILGenModule::emitDefaultArgGenerator(swift::SILDeclRef, swift::ParamDecl*) + 394
11 swift                    0x0000000108cdde3d swift::Lowering::SILGenModule::emitDefaultArgGenerators(llvm::PointerUnion<swift::ValueDecl*, swift::AbstractClosureExpr*>, swift::ParameterList*) + 93
12 swift                    0x0000000108dbb68b (anonymous namespace)::SILGenType::emitType() + 619
13 swift                    0x0000000108dbb419 swift::Lowering::SILGenModule::visitNominalTypeDecl(swift::NominalTypeDecl*) + 25
14 swift                    0x0000000108ce1933 (anonymous namespace)::SILGenModuleRAII::emitSourceFile(swift::SourceFile*) + 883
15 swift                    0x0000000108ce1506 swift::SILGenSourceFileRequest::evaluate(swift::Evaluator&, swift::SILGenDescriptor) const + 182
16 swift                    0x0000000108daf3b1 swift::SimpleRequest<swift::SILGenSourceFileRequest, std::__1::unique_ptr<swift::SILModule, std::__1::default_delete<swift::SILModule> > (swift::SILGenDescriptor), (swift::CacheKind)0>::evaluateRequest(swift::SILGenSourceFileRequest const&, swift::Evaluator&) + 33
17 swift                    0x0000000108ce7644 llvm::Expected<swift::SILGenSourceFileRequest::OutputType> swift::Evaluator::getResultUncached<swift::SILGenSourceFileRequest>(swift::SILGenSourceFileRequest const&) + 340
18 swift                    0x0000000108ce2c95 swift::performSILGeneration(swift::FileUnit&, swift::Lowering::TypeConverter&, swift::SILOptions const&) + 85
19 swift                    0x00000001089ff4e9 performCompile(swift::CompilerInstance&, swift::CompilerInvocation const&, llvm::ArrayRef<char const*>, int&, swift::FrontendObserver*, swift::UnifiedStatsReporter*) + 8425
20 swift                    0x00000001089fc460 swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*) + 4480
21 swift                    0x000000010899012d main + 861
22 libdyld.dylib            0x00007fff6705c405 start + 1
<unknown>:0: error: unable to execute command: Abort trap: 6
<unknown>:0: error: compile command failed due to signal 6 (use -v to see invocation)

[theblixguy]

Does changing the mangling have ABI implications? (Also see discussion in SR-10077)

[beccadax]

@swift-ci create

[jckarter]

@theblixguy I don't think the enum case accessors are ever generated as public symbols. Am I right about that @slavapestov?

[slavapestov]

When resilience is enabled, the enum case mangling is used to emit a public symbol with the case index. So yes, unfortunately we'll need to hack in compatibility somehow.

[jckarter]

Well, the good news is that pattern matching cases with the same name seems to be generally broken, so breaking the ABI in cases where there are multiple cases with the same base name might not be a problem in practice: https://bugs.swift.org/browse/SR-12229

[theblixguy]

Okay. I suppose the ASTMangler needs to mangle argument labels too? I assumed appendDeclType (or appendFunction which is called later) would already handle it (like it does for regular functions) but seems like it doesn't here.

For compatibility, would a build flag be reasonable? and if so, it would need to be added implicitly by default I suppose so everything builds with it?

[slavapestov]

Yes, we need to mangle argument labels too. It appears that the mangler is only mangling the payload type and not the labels, which is incorrect.

Unfortunately this will break existing code, so you have to continue to emit the old symbols and alias them in the resilience case.

[jckarter]

To preserve the ABI for the case index symbols, maybe we could mangle them using the base name and payload type, but use the full name for other non-ABI symbols related to the case?