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.

Sign up for GitHub

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

[QoI] Cleanup AST after trying to shrink constraint system of invalid expression #6780

Merged
merged 1 commit into from
Jan 13, 2017
Merged

[QoI] Cleanup AST after trying to shrink constraint system of invalid expression #6780

merged 1 commit into from
Jan 13, 2017

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Jan 13, 2017

Since ConstraintSystem::shrink is going to attempt to type-check
sub-expressions separately it's essential to clean-up AST if constraint
generation or solving of the such expressions fails, otherwise if
such solving resulted in creation of implicit expression type variables
might leak to the outside.

@xedin
Copy link
Contributor Author

xedin commented Jan 13, 2017

/cc @rudkx @slavapestov

@practicalswift I think it solved all of the #keyPath related crashers, but I left the ones I couldn't reproduce locally in place, so it would be great if you would re-check them too.

@xedin
[QoI] Cleanup AST after trying to shrink constraint system of invalid…
9b7d632 
… expression

Since `ConstraintSystem::shrink` is going to attempt to type-check
sub-expressions separately it's essential to clean-up AST if constraint
generation or solving of the such expressions fails, otherwise if
such solving resulted in creation of implicit expression type variables
might leak to the outside.
@slavapestov
Copy link
Contributor

@swift-ci Please smoke test

@practicalswift
Copy link
Contributor

@xedin Great! I'll take a look once this PR is merged! :-)

@practicalswift
Copy link
Contributor

Seems to pass all tests. Merge:able? :-)

@rudkx
Copy link
Contributor

rudkx commented Jan 13, 2017

LGTM. I'll hopefully wrap up my side table work soon and a lot of this kind of stuff will go away.

@rudkx rudkx merged commit a94b2dd into swiftlang:master Jan 13, 2017
@xedin
Copy link
Contributor Author

xedin commented Jan 13, 2017

Thanks, @rudkx! Looking forward to that :)

@aschwaighofer
Copy link
Contributor

This very likely cause a heap-use-after-freen ASAN error on one of our bots.

=================================================================
==12213==ERROR: AddressSanitizer: heap-use-after-free on address 0x621002d8b874 at pc 0x000102b2f5b2 bp 0x7fff5eb61e60 sp 0x7fff5eb61e58
READ of size 2 at 0x621002d8b874 thread T0
#0 0x102b2f5b1 in swift::CleanupIllFormedExpressionRAII::doIt(swift::Expr*, swift::ASTContext&)::CleanupIllFormedExpression::walkToExprPre(swift::Expr*) Types.h:413
#1 0x102ec3fbd in swift::Expr::walk(swift::ASTWalker&) ASTWalker.cpp:953
#2 0x102b0a264 in swift::CleanupIllFormedExpressionRAII::~CleanupIllFormedExpressionRAII() Expr.h:524
#3 0x102993eb9 in swift::constraints::ConstraintSystem::Candidate::solve() CSSolver.cpp:1512
#4 0x10299a8a7 in swift::constraints::ConstraintSystem::shrink(swift::Expr*) CSSolver.cpp:1840
#5 0x10299ae92 in swift::constraints::ConstraintSystem::solve(swift::Expr*&, swift::Type, swift::ExprTypeCheckListener*, llvm::SmallVectorImplswift::constraints::Solution&, swift::FreeTypeVariableBinding) CSSolver.cpp:1870
#6 0x102b0a7ad in swift::TypeChecker::solveForExpression(swift::Expr*&, swift::DeclContext*, swift::Type, swift::FreeTypeVariableBinding, swift::ExprTypeCheckListener*, swift::constraints::ConstraintSystem&, llvm::SmallVectorImplswift::constraints::Solution&, swift::OptionSet<swift::TypeCheckExprFlags, unsigned int>) TypeCheckConstraints.cpp:1400
#7 0x102b15e66 in swift::TypeChecker::typeCheckExpression(swift::Expr*&, swift::DeclContext*, swift::TypeLoc, swift::ContextualTypePurpose, swift::OptionSet<swift::TypeCheckExprFlags, unsigned int>, swift::ExprTypeCheckListener*, swift::constraints::ConstraintSystem*) TypeCheckConstraints.cpp:1592
#8 0x102ca802e in swift::ASTVisitor<(anonymous namespace)::StmtChecker, void, swift::Stmt*, void, void, void, void>::visit(swift::Stmt*) TypeCheckStmt.cpp:440
#9 0x102ca48f1 in swift::ASTVisitor<(anonymous namespace)::StmtChecker, void, swift::Stmt*, void, void, void, void>::visit(swift::Stmt*) TypeCheckStmt.cpp:365
#10 0x102ca229d in (anonymous namespace)::StmtChecker::typeCheckBody(swift::BraceStmt*&) TypeCheckStmt.cpp:365
#11 0x102c9f488 in swift::TypeChecker::typeCheckFunctionBodyUntil(swift::FuncDecl*, swift::SourceLoc) TypeCheckStmt.cpp:1353
#12 0x102c9ef2d in swift::TypeChecker::typeCheckAbstractFunctionBodyUntil(swift::AbstractFunctionDecl*, swift::SourceLoc) TypeCheckStmt.cpp:1308
#13 0x102ca180b in swift::TypeChecker::typeCheckAbstractFunctionBody(swift::AbstractFunctionDecl*) TypeCheckStmt.cpp:1327
#14 0x102cdc0d1 in typeCheckFunctionsAndExternalDecls(swift::TypeChecker&) TypeChecker.cpp:509
#15 0x102cde0e6 in swift::performTypeChecking(swift::SourceFile&, swift::TopLevelContext&, swift::OptionSet<swift::TypeCheckingFlags, unsigned int>, unsigned int, unsigned int) TypeChecker.cpp:693
#16 0x10211d630 in swift::CompilerInstance::performSema() Frontend.cpp:530
#17 0x101154579 in swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*) FrontendTool.cpp:389
#18 0x10109855f in main driver.cpp:167
#19 0x7fffc3a7e234 in start (libdyld.dylib+0x5234)

0x621002d8b874 is located 2932 bytes inside of 4096-byte region [0x621002d8ad00,0x621002d8bd00)
freed by thread T0 here:
#0 0x10ff40b56 in wrap_free (libclang_rt.asan_osx_dynamic.dylib+0x53b56)
#1 0x109ee190a in llvm::BumpPtrAllocatorImpl<llvm::MallocAllocator, 4096ul, 4096ul>::~BumpPtrAllocatorImpl() Allocator.h:102
#2 0x102a1d869 in swift::constraints::ConstraintSystem::~ConstraintSystem() Allocator.h:164
#3 0x102993eb0 in swift::constraints::ConstraintSystem::Candidate::solve() CSSolver.cpp:1512
#4 0x10299a8a7 in swift::constraints::ConstraintSystem::shrink(swift::Expr*) CSSolver.cpp:1840
#5 0x10299ae92 in swift::constraints::ConstraintSystem::solve(swift::Expr*&, swift::Type, swift::ExprTypeCheckListener*, llvm::SmallVectorImplswift::constraints::Solution&, swift::FreeTypeVariableBinding) CSSolver.cpp:1870
#6 0x102b0a7ad in swift::TypeChecker::solveForExpression(swift::Expr*&, swift::DeclContext*, swift::Type, swift::FreeTypeVariableBinding, swift::ExprTypeCheckListener*, swift::constraints::ConstraintSystem&, llvm::SmallVectorImplswift::constraints::Solution&, swift::OptionSet<swift::TypeCheckExprFlags, unsigned int>) TypeCheckConstraints.cpp:1400
#7 0x102b15e66 in swift::TypeChecker::typeCheckExpression(swift::Expr*&, swift::DeclContext*, swift::TypeLoc, swift::ContextualTypePurpose, swift::OptionSet<swift::TypeCheckExprFlags, unsigned int>, swift::ExprTypeCheckListener*, swift::constraints::ConstraintSystem*) TypeCheckConstraints.cpp:1592
#8 0x102ca802e in swift::ASTVisitor<(anonymous namespace)::StmtChecker, void, swift::Stmt*, void, void, void, void>::visit(swift::Stmt*) TypeCheckStmt.cpp:440
#9 0x102ca48f1 in swift::ASTVisitor<(anonymous namespace)::StmtChecker, void, swift::Stmt*, void, void, void, void>::visit(swift::Stmt*) TypeCheckStmt.cpp:365
#10 0x102ca229d in (anonymous namespace)::StmtChecker::typeCheckBody(swift::BraceStmt*&) TypeCheckStmt.cpp:365
#11 0x102c9f488 in swift::TypeChecker::typeCheckFunctionBodyUntil(swift::FuncDecl*, swift::SourceLoc) TypeCheckStmt.cpp:1353
#12 0x102c9ef2d in swift::TypeChecker::typeCheckAbstractFunctionBodyUntil(swift::AbstractFunctionDecl*, swift::SourceLoc) TypeCheckStmt.cpp:1308
#13 0x102ca180b in swift::TypeChecker::typeCheckAbstractFunctionBody(swift::AbstractFunctionDecl*) TypeCheckStmt.cpp:1327
#14 0x102cdc0d1 in typeCheckFunctionsAndExternalDecls(swift::TypeChecker&) TypeChecker.cpp:509
#15 0x102cde0e6 in swift::performTypeChecking(swift::SourceFile&, swift::TopLevelContext&, swift::OptionSet<swift::TypeCheckingFlags, unsigned int>, unsigned int, unsigned int) TypeChecker.cpp:693
#16 0x10211d630 in swift::CompilerInstance::performSema() Frontend.cpp:530
#17 0x101154579 in swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*) FrontendTool.cpp:389
#18 0x10109855f in main driver.cpp:167
#19 0x7fffc3a7e234 in start (libdyld.dylib+0x5234)

previously allocated by thread T0 here:
#0 0x10ff4098c in wrap_malloc (libclang_rt.asan_osx_dynamic.dylib+0x5398c)
#1 0x1070207eb in llvm::BumpPtrAllocatorImpl<llvm::MallocAllocator, 4096ul, 4096ul>::Allocate(unsigned long, unsigned long) Allocator.h:95
#2 0x102a2161b in swift::constraints::ConstraintSystem::getConstraintLocator(swift::Expr*, llvm::ArrayRefswift::constraints::ConstraintLocator::PathElement, unsigned int) ConstraintLocator.h:489
#3 0x102919fbc in swift::ASTVisitor<(anonymous namespace)::ConstraintGenerator, swift::Type, void, void, void, void, void>::visit(swift::Expr*) ConstraintSystem.h:1515
#4 0x10292de97 in (anonymous namespace)::ConstraintWalker::walkToExprPost(swift::Expr*) CSGen.cpp:2905
#5 0x102ed3616 in (anonymous namespace)::Traversal::visitApplyExpr(swift::ApplyExpr*) ASTWalker.cpp:961
#6 0x102ec70f2 in swift::ASTVisitor<(anonymous namespace)::Traversal, swift::Expr*, swift::Stmt*, bool, swift::Pattern*, bool, void>::visit(swift::Expr*) ExprNodes.def:120
#7 0x102ec4052 in swift::Expr::walk(swift::ASTWalker&) ASTWalker.cpp:98
#8 0x102913a70 in swift::constraints::ConstraintSystem::generateConstraints(swift::Expr*) CSGen.cpp:2991
#9 0x102993aed in swift::constraints::ConstraintSystem::Candidate::solve() CSSolver.cpp:1462
#10 0x10299a8a7 in swift::constraints::ConstraintSystem::shrink(swift::Expr*) CSSolver.cpp:1840
#11 0x10299ae92 in swift::constraints::ConstraintSystem::solve(swift::Expr*&, swift::Type, swift::ExprTypeCheckListener*, llvm::SmallVectorImplswift::constraints::Solution&, swift::FreeTypeVariableBinding) CSSolver.cpp:1870
#12 0x102b0a7ad in swift::TypeChecker::solveForExpression(swift::Expr*&, swift::DeclContext*, swift::Type, swift::FreeTypeVariableBinding, swift::ExprTypeCheckListener*, swift::constraints::ConstraintSystem&, llvm::SmallVectorImplswift::constraints::Solution&, swift::OptionSet<swift::TypeCheckExprFlags, unsigned int>) TypeCheckConstraints.cpp:1400
#13 0x102b15e66 in swift::TypeChecker::typeCheckExpression(swift::Expr*&, swift::DeclContext*, swift::TypeLoc, swift::ContextualTypePurpose, swift::OptionSet<swift::TypeCheckExprFlags, unsigned int>, swift::ExprTypeCheckListener*, swift::constraints::ConstraintSystem*) TypeCheckConstraints.cpp:1592
#14 0x102ca802e in swift::ASTVisitor<(anonymous namespace)::StmtChecker, void, swift::Stmt*, void, void, void, void>::visit(swift::Stmt*) TypeCheckStmt.cpp:440
#15 0x102ca48f1 in swift::ASTVisitor<(anonymous namespace)::StmtChecker, void, swift::Stmt*, void, void, void, void>::visit(swift::Stmt*) TypeCheckStmt.cpp:365
#16 0x102ca229d in (anonymous namespace)::StmtChecker::typeCheckBody(swift::BraceStmt*&) TypeCheckStmt.cpp:365
#17 0x102c9f488 in swift::TypeChecker::typeCheckFunctionBodyUntil(swift::FuncDecl*, swift::SourceLoc) TypeCheckStmt.cpp:1353
#18 0x102c9ef2d in swift::TypeChecker::typeCheckAbstractFunctionBodyUntil(swift::AbstractFunctionDecl*, swift::SourceLoc) TypeCheckStmt.cpp:1308
#19 0x102ca180b in swift::TypeChecker::typeCheckAbstractFunctionBody(swift::AbstractFunctionDecl*) TypeCheckStmt.cpp:1327
#20 0x102cdc0d1 in typeCheckFunctionsAndExternalDecls(swift::TypeChecker&) TypeChecker.cpp:509
#21 0x102cde0e6 in swift::performTypeChecking(swift::SourceFile&, swift::TopLevelContext&, swift::OptionSet<swift::TypeCheckingFlags, unsigned int>, unsigned int, unsigned int) TypeChecker.cpp:693
#22 0x10211d630 in swift::CompilerInstance::performSema() Frontend.cpp:530
#23 0x101154579 in swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*) FrontendTool.cpp:389
#24 0x10109855f in main driver.cpp:167
#25 0x7fffc3a7e234 in start (libdyld.dylib+0x5234)

SUMMARY: AddressSanitizer: heap-use-after-free Types.h:413 in swift::CleanupIllFormedExpressionRAII::doIt(swift::Expr*, swift::ASTContext&)::CleanupIllFormedExpression::walkToExprPre(swift::Expr*)
Shadow bytes around the buggy address:
0x1c42005b16b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x1c42005b16c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x1c42005b16d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x1c42005b16e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x1c42005b16f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x1c42005b1700: fd fd fd fd fd fd fd fd fd fd fd fd fd fd[fd]fd
0x1c42005b1710: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x1c42005b1720: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x1c42005b1730: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x1c42005b1740: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd

@aschwaighofer
Copy link
Contributor

I will speculatively revert this.

@aschwaighofer aschwaighofer mentioned this pull request Jan 13, 2017
Merged
@xedin
Copy link
Contributor Author

xedin commented Jan 13, 2017

@aschwaighofer Thanks, I'll submit a separate PR which is not going to use RAII for cleanup, was hoping that I can re-use some of the existing stuff but apparently not :)

@xedin
Copy link
Contributor Author

xedin commented Jan 13, 2017

Please let me know how it goes after revert, because solveForExpression uses it exactly the same way and doesn't fail on ASAN...

@xedin
Copy link
Contributor Author

xedin commented Jan 14, 2017

Ok, excuse my stupidity, I actually know what the problem is, going to submit separate PR for this shortly.

@gparker42
Copy link
Contributor

ASAN test succeeded after the recert. Followup #6801 also failed in ASAN.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@xedin @slavapestov @practicalswift @rudkx @aschwaighofer @gparker42