Warn on member assignments capturing self (SR-8536)


(Ehud Adler) #1

Hi Swift Community! I've been excited to make my first contribution and was really pushed to starting after reading What should I learn if I want to contribute to the Swift compiler? so big thanks for everyone on that thread.

I'm working on Bug Report SR-8536 and am currently a bit stuck.

I was told that a good idea would be to add a new pass in the SILOptimizer called "DetectReferenceToSelf" or something of the sort. Speaking with @harlanhaskins I was able to create the pass but I now must implement the proper checks.

Heres where I am at:

// isReferenceToSelf function

static bool isReferenceToSelf(SILFunction *Fn, AliasAnalysis *AA) {

  for (auto &BB : *Fn) {
    auto I = BB.begin(), E = BB.end();
    while (I != E) {

      auto *Inst = &*I;
      auto kind = Inst->getKind();
      auto *AI = dyn_cast<AssignInst>(Inst);
      ++I;
      if (kind == SILInstructionKind::AssignInst) {
        SILValue V1 = AI->getSrc();
        SILValue V2 = AI->getDest();
        SILType Ty1 = V1->getType().getObjectType();
        SILType Ty2 = V2->getType().getObjectType();
        auto result = AA->alias(V1, V2, Ty1, Ty2);
        if (result != AliasAnalysis::AliasResult::NoAlias) {
          return true;
        }
      }
    }
  }
  return false;
}

I'm trying to use AliasAnalysis to check if to values in an assignment are aliases of one another.

I run this on the example project:

class Foo {
    var bar: () -> () = {}
    func baz() {}

    init() {
        self.bar = baz
    }
}

The SIL code produced that I was interested in was:

// Foo.init()
sil hidden @$S5hello3FooCACycfc : $@convention(method) (@owned Foo) -> @owned Foo {
// %0                                             // users: %2, %1
bb0(%0 : $Foo):
  debug_value %0 : $Foo, let, name "self", argno 1 // id: %1
  %2 = mark_uninitialized [rootself] %0 : $Foo    // users: %22, %21, %20, %15, %12, %11, %10, %5
  // function_ref variable initialization expression of Foo.bar
  %3 = function_ref @$S5hello3FooC3baryycvpfi : $@convention(thin) () -> @owned @callee_guaranteed () -> () // user: %4
  %4 = apply %3() : $@convention(thin) () -> @owned @callee_guaranteed () -> () // user: %8
  %5 = begin_borrow %2 : $Foo                     // users: %10, %6
  %6 = ref_element_addr %5 : $Foo, #Foo.bar       // user: %7
  %7 = begin_access [modify] [dynamic] %6 : $*@callee_guaranteed () -> () // users: %9, %8
  assign %4 to %7 : $*@callee_guaranteed () -> () // id: %8
  end_access %7 : $*@callee_guaranteed () -> ()   // id: %9
  end_borrow %5 from %2 : $Foo, $Foo              // id: %10
  %11 = begin_borrow %2 : $Foo                    // users: %20, %16
  %12 = begin_borrow %2 : $Foo                    // users: %15, %14
  // function_ref curry thunk of Foo.baz()
  %13 = function_ref @$S5hello3FooC3bazyyFTc : $@convention(thin) (@guaranteed Foo) -> @owned @callee_guaranteed () -> () // user: %14
  %14 = apply %13(%12) : $@convention(thin) (@guaranteed Foo) -> @owned @callee_guaranteed () -> () // user: %18
  end_borrow %12 from %2 : $Foo, $Foo             // id: %15
  %16 = ref_element_addr %11 : $Foo, #Foo.bar     // user: %17
  %17 = begin_access [modify] [dynamic] %16 : $*@callee_guaranteed () -> () // users: %19, %18
  assign %14 to %17 : $*@callee_guaranteed () -> () // id: %18
  end_access %17 : $*@callee_guaranteed () -> ()  // id: %19
  end_borrow %11 from %2 : $Foo, $Foo             // id: %20
  %21 = copy_value %2 : $Foo                      // user: %23
  destroy_value %2 : $Foo                         // id: %22
  return %21 : $Foo                               // id: %23
} // end sil function '$S5hello3FooCACycfc'

The line

assign %14 to %17 : $*@callee_guaranteed () -> () // id: %18

is where the warning should happen (I think) since

%14 --apply--> %13(%12) where %12 borrows %2
and
%17 --access--> %16 --ref_element--> %11 --borrow--> %2

So it appears that they both reference back to %2 which is [rootself]. I'm new to this so I'm not sure I'm on the right track here or that makes complete sense. I was hoping that AliasAnalysis would detect that they were both %2.

Unfortunately, when I build, compile and run I get no warning and when I attach lldb debugger and print the value of auto kind = Inst->getKind(); I don't get SILInstructionKind::AssignInst so I'm missing something there and it results in nothing ever entering the if statement.

Anyways, I hoped I gave a bit of where I'm at and my thought process. If anyone has suggestions on how to move forward, where my mistakes are, I'd appreciate it.