Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
jckarter opened this issue Feb 14, 2020 · 8 comments
Open

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

jckarter opened this issue Feb 14, 2020 · 8 comments
Labels
compiler The Swift compiler in itself

Comments

@jckarter
Copy link
Member

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:

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
Copy link
Collaborator

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

@beccadax
Copy link
Contributor

@swift-ci create

@jckarter
Copy link
Member Author

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

@slavapestov
Copy link
Member

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
Copy link
Member Author

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
Copy link
Collaborator

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
Copy link
Member

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
Copy link
Member Author

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?

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler The Swift compiler in itself
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@slavapestov @jckarter @beccadax @theblixguy