[C++-Interop] C++ struct passed by value to an objc_msgSend is passed as a pointer type of ABIArgInfo::Direct instead of a Indirect byval

[plotfi]

For the following ObjC implementation:

@implementation ContainerObjCWraper
+ (instancetype)newWithContainer:(Container)context { /* ... */}
@end

Any ObjC method call to this new method would pass the Container argument with a LLVM IR byval attribute, and the LLVM function define for this method includes the byval attribute:

define internal noundef i8* @"\01+[ContainerObjCWraper newWithContainer:]"
(i8* noundef %self, i8* noundef %_cmd, %struct.Container* noundef byval(%struct.Container) align 8 %context) #1 {

The Swift ClangImporter in the presence of C++-Interop does not include this byval (byval(%struct.Container)) and at the callsite instead produces:

call %1* bitcast (void ()* @objc_msgSend to %1* (i8*, i8*, %struct.Container*)*)(i8* %52, i8* %51, %struct.Container* nonnull  %2)

where I think it could possibly instead produce:

call %1* bitcast (void ()* @objc_msgSend to %1* (i8*, i8*, %struct.Container*)*)(i8* %52, i8* %51,
                                                                                      %struct.Container* nonnull byval(%struct.Container) %2)

I am still trying to understand why, but I believe it is because in the importer the ObjCMethodDecl is handled in a way where it is assumed that the argument is ABIArgInfo::Direct versus an ABIArgInfo::Indirect with a byval flag.

I have some example code that is affected by this behavior where on X86_64 (found with iOS Simulator) passing the mentioned C++ struct type by value results an a corruption in the parameter passing (often the resulting value is passed with some 8-16 bytes offset incorrectly.

The example code is as follows:

Implementation:

// CXX.mm

#include <CXX.h>

@implementation ContainerObjCWraper
+ (instancetype)newWithContainer:(Container)context {
  context.dump();
  return nil;
}
@end

Header:

// CXX.h

#import <Foundation/Foundation.h>

NS_ASSUME_NONNULL_BEGIN

struct Container {
  NSString *_Nullable a; NSString *_Nullable b; NSString *_Nullable c;
  NSString *_Nullable d; NSString *_Nullable e; NSString *_Nullable f;

  void dump() const {
    NSLog(@"a: %@", a == nil ? @"" : a); NSLog(@"b: %@", b == nil ? @"" : b);
    NSLog(@"c: %@", c == nil ? @"" : c); NSLog(@"d: %@", d == nil ? @"" : d);
    NSLog(@"e: %@", e == nil ? @"" : e); NSLog(@"f: %@", f == nil ? @"" : f);
  }

  static Container
  build(NSString *_Nullable a, NSString *_Nullable b, NSString *_Nullable c,
        NSString *_Nullable d, NSString *_Nullable e, NSString *_Nullable f) {
    return { .a = a, .b = b, .c = c, .d = d, .e = e, .f = f };
  }
};

@interface ContainerObjCWraper : NSObject
+ (instancetype)newWithContainer:(Container)context;
@end

NS_ASSUME_NONNULL_END

Swift:

// main.swift

import Foundation
import CXX

print("building context...")
let context = Container.build(nil, nil, "Hello World", nil, nil, nil)

print("dump context before pass by value to objc new...")
context.dump()

print("dump context after pass by value to objc new...")
_ = ContainerObjCWraper.new(with: context)

print("DONE")

Module Map (module.modulemap):

module CXX {
  header "CXX.h"
}

Compile and running should result in:

$ ./run.sh ~/local/S/build/RelWithDebInfo/BinaryCache/toolchain/bin/
Using /Users/plotfi/local/S/build/RelWithDebInfo/BinaryCache/toolchain/bin/ as toolchain...
building context...
dump context before pass by value to objc new...
2022-11-03 22:32:18.538 test-x86_64-apple-macosx12.0.0.exe[74361:3430041] a:
2022-11-03 22:32:18.538 test-x86_64-apple-macosx12.0.0.exe[74361:3430041] b:
2022-11-03 22:32:18.538 test-x86_64-apple-macosx12.0.0.exe[74361:3430041] c: Hello World
2022-11-03 22:32:18.538 test-x86_64-apple-macosx12.0.0.exe[74361:3430041] d:
2022-11-03 22:32:18.538 test-x86_64-apple-macosx12.0.0.exe[74361:3430041] e:
2022-11-03 22:32:18.538 test-x86_64-apple-macosx12.0.0.exe[74361:3430041] f:
dump context after pass by value to objc new...
2022-11-03 22:32:18.538 test-x86_64-apple-macosx12.0.0.exe[74361:3430041] a:
2022-11-03 22:32:18.538 test-x86_64-apple-macosx12.0.0.exe[74361:3430041] b:
2022-11-03 22:32:18.538 test-x86_64-apple-macosx12.0.0.exe[74361:3430041] c: Hello World
2022-11-03 22:32:18.538 test-x86_64-apple-macosx12.0.0.exe[74361:3430041] d:
2022-11-03 22:32:18.538 test-x86_64-apple-macosx12.0.0.exe[74361:3430041] e:
2022-11-03 22:32:18.538 test-x86_64-apple-macosx12.0.0.exe[74361:3430041] f:
DONE

But instead result in the following which is offset and contains garbage (or possibly even a segfault):

$ ./run.sh ~/local/S/build/RelWithDebInfo/BinaryCache/toolchain/bin/
Using /Users/plotfi/local/S/build/RelWithDebInfo/BinaryCache/toolchain/bin/ as toolchain...
building context...
dump context before pass by value to objc new...
2022-11-03 22:30:49.318 test-x86_64-apple-macosx12.0.0.exe[74177:3428829] a:
2022-11-03 22:30:49.319 test-x86_64-apple-macosx12.0.0.exe[74177:3428829] b:
2022-11-03 22:30:49.319 test-x86_64-apple-macosx12.0.0.exe[74177:3428829] c: Hello World
2022-11-03 22:30:49.319 test-x86_64-apple-macosx12.0.0.exe[74177:3428829] d:
2022-11-03 22:30:49.319 test-x86_64-apple-macosx12.0.0.exe[74177:3428829] e:
2022-11-03 22:30:49.319 test-x86_64-apple-macosx12.0.0.exe[74177:3428829] f:
dump context after pass by value to objc new...
2022-11-03 22:30:49.319 test-x86_64-apple-macosx12.0.0.exe[74177:3428829] a: KDº@[Þ*
2022-11-03 22:30:49.319 test-x86_64-apple-macosx12.0.0.exe[74177:3428829] b:
2022-11-03 22:30:49.319 test-x86_64-apple-macosx12.0.0.exe[74177:3428829] c:
2022-11-03 22:30:49.319 test-x86_64-apple-macosx12.0.0.exe[74177:3428829] d: Hello World
2022-11-03 22:30:49.319 test-x86_64-apple-macosx12.0.0.exe[74177:3428829] e:
2022-11-03 22:30:49.319 test-x86_64-apple-macosx12.0.0.exe[74177:3428829] f:
DONE

I have attached a zip file that includes a run.sh file and the above sources to build and run all of this with a ./run.sh /path/to/toolchain/bin

cxx_interop_byval.zip

[plotfi]

I believe what is happening is the byval is getting incorrectly set because the type coming from SIL is incorrect.

When I wrote comparable code in ObjC++ I get the type

RecordType 0x14116da80 'struct Container'
`-CXXRecord 0x14116d9e8 'Container'

but in swift in GenCall.cpp at

https://github.com/apple/swift/blob/683985e8247df06cf223fbfef4a6658d9ca4880c/lib/IRGen/GenCall.cpp#L1399

the type that IRGen gets is

PointerType 0x124c98840 'const struct Container *' imported
`-QualType 0x124967851 'const struct Container' const
  `-RecordType 0x124967850 'struct Container' imported
    `-CXXRecord 0x124967740 'Container'

I believe this is happening because getClangType is returning the pointer type at

    if (params.isFormalIndirect()) {
      auto constTy =
        getClangASTContext().getCanonicalType(clangType.withConst());
      return getClangASTContext().getPointerType(constTy);
    }

due to a @in type rather than an @in_guaranteed

[hyp]

I'm investigating making AdressOnly C++ records fix where by we check if the struct is passed in registers directly.