Skip to content

[5.6] Fix OwnershipLiveRange for @owned values getting transformed to none values via switch_enum #60104

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 1 commit into from
Jul 25, 2022

Conversation

meg-gupta
Copy link
Contributor

@meg-gupta meg-gupta commented Jul 18, 2022

This is a cherry-pick of #60101
Fixes rdar://95039799

Description:
We are currently not computing OwnershipLiveRange correctly in the presence of switch_enum transforming the owned value to a none value in the presence of dead end blocks. We can end up with illegal SIL with LoadCopyToLoadBorrowOptimization, due to this. This won't cause a miscompile, but will raise a verifier error.

End-user impact: May see verifier error while using switch statement on enums with dead-end paths on all non-trivial paths.

Risk: Low
This change is just bailing out on seeing a switch_enum transforming the owned value to a none value, while computing OwnershipLiveRange.

@meg-gupta meg-gupta requested a review from a team as a code owner July 18, 2022 19:36
…values via switch_enum

rdar://95039799

We are currently not computing OwnershipLiveRange correctly in the presence of switch_enum transforming the owned value to a none value.

We can end up with illegal SIL with LoadCopyToLoadBorrowOptimization, due to this.

Example:

sil [ossa] @switch_enum_test : $@convention(thin) (@inout StructWithEnum) -> () {
 bb0(%0 : $*StructWithEnum):
   %1 = struct_element_addr %0 : $*StructWithEnum, #StructWithEnum.val
   %2 = struct_element_addr %0 : $*StructWithEnum, #StructWithEnum.val
   %3 = load [copy] %2 : $*EnumA
   destroy_addr %1 : $*EnumA
   switch_enum %3 : $EnumA, case #EnumA.sometrivial!enumelt: bb1, case #EnumA.somenontrivial!enumelt:bb2, case #EnumA.none!enumelt: bb3

 bb1(%6 : $TrivialStruct):
   %f = function_ref @get_enum : $@convention(thin) () -> @owned EnumA
   %r = apply %f() : $@convention(thin) () -> @owned EnumA
   %ele = struct_element_addr %0 : $*StructWithEnum, #StructWithEnum.val
   store %r to [init] %ele :$*EnumA
   %9999 = tuple()
   return %9999 : $()

 bb2(%7 : @owned $NonTrivialStruct):
  unreachable

 bb3:
  unreachable
 }

can get transformed to:

sil [ossa] @switch_enum_test : $@convention(thin) (@inout StructWithEnum) -> () {

bb0(%0 : $*StructWithEnum):
  %1 = struct_element_addr %0 : $*StructWithEnum, #StructWithEnum.val
  %2 = struct_element_addr %0 : $*StructWithEnum, #StructWithEnum.val
  %3 = load_borrow %2 : $*EnumA
  destroy_addr %1 : $*EnumA
  switch_enum %3 : $EnumA, case #EnumA.sometrivial!enumelt: bb1, case #EnumA.somenontrivial!enumelt: bb2, case #EnumA.none!enumelt: bb3

bb1(%6 : $TrivialStruct):
  end_borrow %3 : $EnumA

  %8 = function_ref @get_enum : $@convention(thin) () -> @owned EnumA
  %9 = apply %8() : $@convention(thin) () -> @owned EnumA
  %10 = struct_element_addr %0 : $*StructWithEnum, #StructWithEnum.val
  store %9 to [init] %10 : $*EnumA
  %12 = tuple ()
  return %12 : $()

bb2(%14 : @guaranteed $NonTrivialStruct):
  unreachable

bb3:
  unreachable
}

which is incorrect:
Write:   destroy_addr %1 : $*EnumA
SIL verification failed: Found load borrow that is invalidated by a local write?!: loadBorrowImmutabilityAnalysis.isImmutable(LBI)
Verifying instruction:
     %2 = struct_element_addr %0 : $*StructWithEnum, #StructWithEnum.val
->   %3 = load_borrow %2 : $*EnumA
     switch_enum %3 : $EnumA, case #EnumA.sometrivial!enumelt: bb1, case #EnumA.somenontrivial!enumelt: bb2, case #EnumA.none!enumelt: bb3
     end_borrow %3 : $EnumA
@meg-gupta meg-gupta force-pushed the cherrypickossafix branch from 33afd09 to b575e5a Compare July 18, 2022 19:38
@meg-gupta meg-gupta requested a review from gottesmm July 18, 2022 19:38
@meg-gupta
Copy link
Contributor Author

@swift-ci test

@tbkka tbkka changed the title Fix OwnershipLiveRange for @owned values getting transformed to none values via switch_enum [5.6] Fix OwnershipLiveRange for @owned values getting transformed to none values via switch_enum Jul 18, 2022
@meg-gupta meg-gupta requested a review from atrick July 18, 2022 21:36
Copy link
Member

@shahmishal shahmishal left a comment

Choose a reason for hiding this comment

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

Approved for 5.6.3

@shahmishal shahmishal merged commit 37dfabd into swiftlang:release/5.6 Jul 25, 2022
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.

3 participants