Is it ok to have an unfinished borrow in a basic block terminated with `unreachable` instruction?

Development Compiler
1

I've added some unit tests in my PR, and they are triggering LinearLifetimeChecker. This seems to be an existing issue, not related to my changes, but reproducible by newly added tests. Reported as A leak due to a consuming post-dominance failure · Issue #91645 · swiftlang/swift · GitHub.

When running tests with --param swift_test_mode=optimize_none_with_opaque_values function convertDirectToIndirectFunctionArgs() inserts a load_borrow in the beginning of the function, and end_borrow in all function-existing blocks. But unreachable block does not count as function-exiting, and non end_borrow is inserted, which LinearLifetimeChecker complains about.

What would be the best way to fix it?

  1. Teach LinearLifetimeChecker to ignore unreachable blocks.
  2. Insert end_borrow by maintaining separate list of program-ending blocks (don't change pass.exitingInsts).
  3. Insert end_borrow by inserting unreachable-terminated blocks into pass.exitingInsts, update OpaqueValueVisitor::canonicalizeReturnValues(), ReturnRewriter::rewriteReturns() and ReturnRewriter::rewriteThrows() to expect UnreachableInst.
  4. Return true from isFunctionExiting() for UnreachableInst, update even more code.

So far, I'm leaning towards 2.

WDYT?

1 Like
2

The fix using #2 - Fix 91645 by nickolas-pohilets · Pull Request #91678 · swiftlang/swift · GitHub

3

I agree. The "complete lifetimes" property of OSSA requires an end_borrow on all paths including unreachable-terminated blocks (aka dead-end blocks). @Erik_Eckstein recently enabled complete lifetimes but AddressLowering probably needed to be updated.

1 Like