Skip to content

[Coverage] Audit uses of getCurrentCounter() (SR-4453) #8584

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

Merged
merged 2 commits into from
Apr 11, 2017

Conversation

vedantk
Copy link
Contributor

@vedantk vedantk commented Apr 6, 2017

Fix a crash in swift code coverage when handling ternary expressions in top-level decls. This has already been committed to master.

rdar://problem/31383534

vedantk added 2 commits April 6, 2017 10:31
We use getCurrentCounter() in one spot where there isn't guaranteed to
be an active region: the `else' part of an IfExpr in a TopLevelCodeDecl.

Fix this issue by assigning a pseudo-counter to the `else' part when the
region stack is empty. This patch also adds crash tests which cover
other code paths which rely on getCurrentCounter() when handling
TopLevelCodeDecls.

Fixes SR-4453.

(cherry picked from commit 7094a64)
When handling ternary expressions, we check that an active region exists
before attempting to access its counter. It's not necessary to check
that the parent AST node is null, and also that the region stack is
empty. The second check is stronger than the first one.

(cherry picked from commit d060eef)
@vedantk
Copy link
Contributor Author

vedantk commented Apr 6, 2017

@swift-ci Please smoke test

@vedantk
Copy link
Contributor Author

vedantk commented Apr 6, 2017

@swift-ci Please test Linux platform
@swift-ci Please test OS X platform

@vedantk
Copy link
Contributor Author

vedantk commented Apr 6, 2017

@swift-ci Please test Linux platform

@tkremenek tkremenek merged commit 2c17dff into swiftlang:swift-3.1-branch Apr 11, 2017
@vedantk vedantk deleted the swift-3.1-branch branch January 31, 2018 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants