Skip to content

Commit 17e3cf0

Browse files
committed
Fix unchecked_bitwise_cast to "escape" its operand's ownership
This makes it impossible for OSSA utilities to reason about the lifetime of the value being cast. This fixes bugs in which SILGen generates bitwise casts without protecting that lifetime. Fixes rdar://100527903 (Overrelease of optional closure when using shorthand syntax) The previous OSSA strategy was to gradually eliminate all "pointer escape" SIL patterns. That would define away a class of bugs in the OSSA utilities themselves. This was not friendly to developers working on SILGen, and we still don't have verification that can catch these bugs. The new strategy is to make all OSSA utilities aware of pointer escapes. This acknowledges reality that SIL is imperfect. Instead, optimizer support for transforming SIL always needs to recognize potentially dangerous patterns. The downside of this new strategy is that it hides performance problems because SIL optimizations can give up whenever they happen to see a "pointer escape". We need to keep working on this problem without being motivated by miscompiling code.
1 parent af276b7 commit 17e3cf0

File tree

1 file changed

+23
-1
lines changed

1 file changed

+23
-1
lines changed

lib/SIL/IR/OperandOwnership.cpp

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,9 +238,31 @@ OPERAND_OWNERSHIP(PointerEscape, ProjectExistentialBox)
238238
OPERAND_OWNERSHIP(PointerEscape, UncheckedOwnershipConversion)
239239
OPERAND_OWNERSHIP(PointerEscape, ConvertEscapeToNoEscape)
240240

241+
// UncheckedBitwiseCast ownership behaves like RefToUnowned. It produces an
242+
// Unowned values from a non-trivial value, without consuming or borrowing the
243+
// non-trivial value. Unlike RefToUnowned, a bitwise cast works on a compound
244+
// value and may truncate the value. The resulting value is still Unowned and
245+
// should be immediately copied to produce an owned value. These happen for two
246+
// reasons:
247+
//
248+
// (1) A Builtin.reinterpretCast is used on a nontrivial type. If the result is
249+
// non-trivial, then, as part of emitting the cast, SILGen emits a copy_value
250+
// immediately after the unchecked_bitwise_cast for all uses of the cast.
251+
//
252+
// (2) SILGen emits special conversions using SILGenBuilder's
253+
// createUncheckedBitCast utility. For non-trivial types, this emits an
254+
// unchecked_bitwise_cast immediately followed by a copy.
255+
//
256+
// The only thing protecting the lifetime of the Unowned value is the cast
257+
// operand's PointerEscape ownership, which prevents OSSA analysis and blocks
258+
// most optimization of the incoming value.
259+
//
260+
// TODO: Verify that Unowned values are only used by copies and that the cast
261+
// operand's lifetime exceeds the copies.
262+
OPERAND_OWNERSHIP(PointerEscape, UncheckedBitwiseCast)
263+
241264
// Instructions that escape reference bits with unenforced lifetime.
242265
// TODO: verify that BitwiseEscape results always have a trivial type.
243-
OPERAND_OWNERSHIP(BitwiseEscape, UncheckedBitwiseCast)
244266
OPERAND_OWNERSHIP(BitwiseEscape, ValueToBridgeObject)
245267
OPERAND_OWNERSHIP(BitwiseEscape, RefToRawPointer)
246268
OPERAND_OWNERSHIP(BitwiseEscape, UncheckedTrivialBitCast)

0 commit comments

Comments
 (0)