Skip to content

Conversation

@vedantk
Copy link
Contributor

@vedantk vedantk commented May 14, 2018

SIL optimizations may rewrite profiling intrinsics in a way that IRGen
can't lower (r://39146527). Don't claim that a coverage mapping has a
guaranteed associated symbol table entry when this happens.

rdar://40133800

vedantk added 3 commits May 14, 2018 10:39
SIL optimizations may rewrite profiling intrinsics in a way that IRGen
can't lower (r://39146527). Don't claim that a coverage mapping has a
guaranteed associated symbol table entry when this happens.

I have not added a test, as this is a defensive workaround until we can
land add a SIL verifier check that prevents profiling intrinsics from
being rewritten.

rdar://40133800
(cherry picked from commit bdfd220)
(cherry picked from commit b74abea)
This adds SIL printer/parser support for SILCoverageMaps representing
top-level code decls.

(cherry picked from commit e7b55a1)
This adds a test case to exercise a path in IRGen which discards
ill-formed profiling intrinsics.

rdar://40133800 & r://39146527
(cherry picked from commit 39194a8)
@vedantk
Copy link
Contributor Author

vedantk commented May 14, 2018

@swift-ci Please test

Copy link
Member

@dcci dcci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine as a temporary workaround.

@vedantk
Copy link
Contributor Author

vedantk commented May 15, 2018

@swift-ci please nominate
Explanation: Swift IRGen can't lower instructions used for profiling if the SIL optimizer rewrites them.
When this happens, code coverage data for the function the instruction is in may be malformed. This PR prevents swift from emitting invalid coverage mapping data.
Scope: This affects availability of code coverage information in the IDE for swift projects with optimizations enabled
Radar/SR Issue: rdar://40133800 tracks this fix, and r://39146527 tracks a longer-term fix to add a SIL verifier check, so that the optimizer never rewrites profiling intrinsics in the first place
Risk: Low
Testing: Running the swift regression tests, and adding a test for the newly-introduced code path
Reviewer: Davide Italiano

@vedantk vedantk merged commit 269cd09 into swiftlang:swift-4.2-branch-04-30-2018 May 15, 2018
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