Open swift-ci opened 3 years ago
@swift-ci create
In the 10/29 development snapshot, we trip an assertion for running out of closure discriminators:
Assertion failed: (discriminator != InvalidDiscriminator), function setDiscriminator, file /Users/buildnode/jenkins/workspace/oss-swift-package-osx/swift/include/swift/AST/Expr.h, line 3604.
Please submit a bug report (https://swift.org/contributing/#reporting-bugs) and include the project and the crash backtrace.
Stack dump:
[snip]
1. Apple Swift version 5.3-dev (LLVM 6df6611ced20c9f, Swift 142b7267089e96d)
2. While evaluating request TypeCheckSourceFileRequest(source_file "/Users/brent/Downloads/PstReader-Segfault-example/Tests/PstReaderTests/FullDumpTests.swift")
3. While evaluating request TypeCheckFunctionBodyRequest(PstReaderTests.(file).FullDumpTests.testOst()@/Users/brent/Downloads/PstReader-Segfault-example/Tests/PstReaderTests/FullDumpTests.swift:14:10)
0 swift-frontend 0x000000010f8c5705 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 37
1 swift-frontend 0x000000010f8c4965 llvm::sys::RunSignalHandlers() + 85
2 swift-frontend 0x000000010f8c5cd6 SignalHandler(int) + 262
3 libsystem_platform.dylib 0x00007fff6d3a5d7d _sigtramp + 29
4 swift-frontend 0x0000000112467438 cmark_strbuf__initbuf + 200307
5 libsystem_c.dylib 0x00007fff6d25f520 abort + 120
6 libsystem_c.dylib 0x00007fff6d25e7d6 err + 0
7 swift-frontend 0x000000010fd473b3 (anonymous namespace)::ContextualizeClosures::walkToExprPre(swift::Expr*) (.cold.7) + 35
8 swift-frontend 0x000000010c00c301 (anonymous namespace)::ContextualizeClosures::walkToExprPre(swift::Expr*) + 753
9 swift-frontend 0x000000010c25a7bf swift::ASTVisitor<(anonymous namespace)::Traversal, swift::Expr*, swift::Stmt*, bool, swift::Pattern*, bool, void>::visit(swift::Expr*) + 4863
10 swift-frontend 0x000000010c25b91f (anonymous namespace)::Traversal::visitApplyExpr(swift::ApplyExpr*) + 207
11 swift-frontend 0x000000010c25bacb swift::ASTVisitor<(anonymous namespace)::Traversal, swift::Expr*, swift::Stmt*, bool, swift::Pattern*, bool, void>::visit(swift::Stmt*) + 219
12 swift-frontend 0x000000010c25bc7c swift::ASTVisitor<(anonymous namespace)::Traversal, swift::Expr*, swift::Stmt*, bool, swift::Pattern*, bool, void>::visit(swift::Stmt*) + 652
13 swift-frontend 0x000000010c25bb5f swift::ASTVisitor<(anonymous namespace)::Traversal, swift::Expr*, swift::Stmt*, bool, swift::Pattern*, bool, void>::visit(swift::Stmt*) + 367
The invalid discriminator is 0xFFFF, so discriminators currently max out at 16 bits. By my count, we are currently using no more than 11 bits of the Bits field of AbstractClosureExpr and its subclasses, so we could expand this to 21 bits without any other changes if we wanted to, or move the discriminator into a separate 32- or 64-bit field.
hughbe (JIRA User), as a workaround, try to avoid having more than 2^16-1 closures or autoclosures in a single function.
After some discussion with Becca, here's a breakdown of what is going on here:
1. We have a total of 64-bits available for storing information, regardless of whether the host is 32-bit or 64-bit. This is indicated by the uint64_t OpaqueBits
in Expr.h
.
NumExprBits = 9
are being used by Expr
. So 64 - NumExprBits
are available to subclasses. As an example, CollectionExpr is using those 64 - NumExprBits
bits. AbstractClosureExpr
is currently using 18 bits (16 for the discriminator, and it has two subclasses AutoClosureExpr
and AbstractClosureExpr
which take a maximum of 2 bits between them) for useful things, and 16 - NumExprBits
for padding. This adds up to a total of 34 - NumExprBits
. 64 - NumExprBits
available; but AbstractClosureExpr
is only using 34 - NumExprBits
. So there's still 30 bits worth of free real estate. One unfortunate consequence of this is that we can't align the Discriminator to a 4-byte boundary without changing way too much code. I think that's okay. We can leave the padding 16 - NumExprBits
as-is (so the Discriminator will be 32-bits but aligned at a 2-byte boundary instead of 4). That still leaves 16 bits for the children, which is more than enough.
Understanding what's going is somewhat tricky, but I suspect the fix will likely take \< 50 LoC worth of changes. I think someone can potentially tackle this as a somewhat ambitious StarterBug.
Please feel free to ask questions here; the bit-field macro soup can be a bit of a headache to understand.
Work style notes: It may be helpful to come up with a minimal test case before making the change and make sure it hits the assert (maybe make a middle commit here marking the test as XFAIL); and then fix the code and make sure the test passes. Iterating on this change will take time as you'll be modifying widely-used headers a fair bit, so a lot of code will need to be recompiled whenever you make a change.
Environment
macOS Catalina 10.15.7 (19H2) Xcode Version 12.0.1 (12A7300)Additional Detail from JIRA
| | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug, StarterBug, TypeChecker | |Assignee | None | |Priority | Medium | md5: 2ec8b445c93ac5fadb92557f27dbf8d7Issue Description:
Steps to reproduce:
Clone this branch: https://github.com/hughbe/PstReader/tree/Segfault-example
Open Package.swift in Xcode
Build the tests
Expected:
Although the file is obviously disgustingly long, it should not crash the compiler
Actual:**
Segmentation fault: 11