Skip to content
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

[C++-Interop] Missing a matching ObjC ARC retain for C++ Copy Constructor to match C++ Destructor #59735

Open
plotfi opened this issue Jun 27, 2022 · 0 comments
Assignees
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. c++ interop Feature: Interoperability with C++

Comments

@plotfi
Copy link
Contributor

plotfi commented Jun 27, 2022

Description

Case is where there is a C++ type that contains an ARC type such as an ObjC Block where the type is passed across the Swift/C++ language boundary by value. The default copy constructor generation needed to facilitate this doesn't seem to account for the ARC retain in the cctor but in the dtor there is an ARC release. The he ObjC release in the callee functions as a double free. I have attached sanitized IR comparing the pass by const ref and pass by value case to this issue as well.

There is a repro project at https://github.com/plotfi/cxx-interop-test/tree/_Block_copy that covers this repro as well.

Reproducer

C++ Header (CXX.h)

typedef int (^UITextBlock)();

struct UITextAttribute {
  UITextAttribute() : textBlock((^{ return 42; })){};
  UITextAttribute(int text) : textBlock(^{ return text; }){};
  UITextBlock textBlock;

  // NOTE: If we manually add the right copy constructor then we avoid the loss 
  //       of the needed ARC retain at the ctor call site.
  #if 0
  UITextAttribute(const UITextAttribute &textAttribute) {
    textBlock = textAttribute.textBlock;
  }
  #endif
};

struct UIAccessibilityContext {
  // NOTE: if ints accessibilityIdentifier or extra are removed, then args
  //       handling code in IRGen seems to assert.
  int accessibilityIdentifier;
  UITextAttribute accessibilityLabel;
  int extra;

  // NOTE: If following line is pass by reference, we avoid the crash since we
  //       dont get an additional dtor ARC release for the reference pass.
  static UIAccessibilityContext build(
    #if 1
    UITextAttribute accessibilityLabel
    #else
    const UITextAttribute &accessibilityLabel
    #endif
  ) {
    return {.accessibilityLabel = accessibilityLabel};
  }
};

module.modulemap

module CXX {
  header "CXX.h"
  requires cplusplus
}

Swift (main.swift)

import CXX

var textAttr1 = UITextAttribute(24)
var a = textAttr1.textBlock()
print("Before. \(a)")

var context = UIAccessibilityContext.build(textAttr1)
var b = context.accessibilityLabel.textBlock()
print("Done. \(b)")

Repro

Copy above files CXX.h, module.modulemap and main.swift respectively into a test directory, cd into that directory and run:

  /path/to/swiftc -o test.exe  \
    -sdk /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk \
    -Xfrontend -enable-objc-interop -Xfrontend -enable-experimental-cxx-interop \
    -I ./ main.swift
  
  ./text.exe

Expected

% ./test.exe
Before. 24
Done. 24

IR.zip

Actual

% ./test.exe
Before. 24
zsh: segmentation fault  ./test.exe

Mitigation

For now I see a mitigation as either implementing a manual copy constructor or passing by const ref. Without this the ObjC release in the callee functions as a double free.

@plotfi plotfi added bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. c++ interop Feature: Interoperability with C++ labels Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. c++ interop Feature: Interoperability with C++
Projects
None yet
Development

No branches or pull requests

3 participants