[ownership] Do not lower copy_unowned_value to strong_retain_unowned.

The major important thing here is that by using copy_unowned_value we can
guarantee that the non-ownership SIL ARC optimizer will treat the release
associated with the strong_retain_unowned as on a distinc rc-identity from its
argument. As an example of this problem consider the following SILGen like
output:

----
%1 = copy_value %0 : $Builtin.NativeObject
%2 = ref_to_unowned %1
%3 = copy_unowned_value %2
destroy_value %1
...
destroy_value %3
----

In this case, we are converting a strong reference to an unowned value and then
lifetime extending the value past the original value. After eliminating
ownership this lowers to:

----
strong_retain %0 : $Builtin.NativeObject
%1 = ref_to_unowned %0
strong_retain_unowned %1
strong_release %0
...
strong_release %0
----

From an RC identity perspective, we have now blurred the lines in between %3 and
%1 in the previous example. This can then result in the following miscompile:

----
%1 = ref_to_unowned %0
strong_retain_unowned %1
...
strong_release %0
----

In this case, it is possible that we created a lifetime gap that will then cause
strong_retain_unowned to assert. By not lowering copy_unowned_value throughout
the SIL pipeline, we instead get this after lowering:

----
strong_retain %0 : $Builtin.NativeObject
%1 = ref_to_unowned %0
%2 = copy_unowned_value %1
strong_release %0
...
strong_release %2
----

And we do not miscompile since we preserved the high level rc identity
pairing.

There shouldn't be any performance impact since we do not really optimize
strong_retain_unowned at the SIL level. I went through all of the places that
strong_retain_unowned was referenced and added appropriate handling for
copy_unowned_value.

rdar://41328987

**NOTE** I am going to remove strong_retain_unowned in a forthcoming commit. I
just want something more minimal for cherry-picking purposes.
This commit is contained in:
Michael Gottesman
2018-06-26 20:49:06 -07:00
parent 33e547ce2a
commit 97367d3dd4
15 changed files with 99 additions and 26 deletions

View File

@@ -55,7 +55,7 @@ const uint16_t VERSION_MAJOR = 0;
/// describe what change you made. The content of this comment isn't important;
/// it just ensures a conflict if two people change the module format.
/// Don't worry about adhering to the 80-column limit for this line.
const uint16_t VERSION_MINOR = 421; // Last change: track whether xrefs come from Clang
const uint16_t VERSION_MINOR = 422; // Last change: {strong_retain,copy}_unowned
using DeclIDField = BCFixed<31>;