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.

Sign up for GitHub

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

Jump to bottom

[SR-12845] Pointers to values of address-only types should not be always marked nocapture (and maybe noalias) #55291

Open
gribozavr opened this issue May 20, 2020 · 6 comments
Open

[SR-12845] Pointers to values of address-only types should not be always marked nocapture (and maybe noalias) #55291

gribozavr opened this issue May 20, 2020 · 6 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. c++ interop Feature: Interoperability with C++ compiler The Swift compiler in itself IRGen LLVM IR generation

Comments

@gribozavr
Copy link
Collaborator

Previous ID SR-12845
Radar rdar://problem/83423213
Original Reporter @gribozavr
Type Bug
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug, CxxInterop, IRGen, LLVM
Assignee None
Priority Medium

md5: e4f14f97860551c0ab6bdf0ea47d957c

Issue Description:

Based on my understanding, I don't think pointers to values of address-only types can be always marked nocapture (and maybe noalias as well).

Here's a Swift example with corresponding LLVM IR:

class MyClass {}

public struct Test {
  weak var x: AnyObject?
}

public func makeTest() -> Test {
  return Test(x: MyClass())
}
define protected swiftcc void @"$s4main8makeTestAA0C0VyF"(%T4main4TestV* noalias nocapture sret %0)

I think we can't mark an indirectly returned value of address-only type Test as nocapture because the address of the weak reference is registered in the side table. I think noalias is wrong for the same reason.

One could argue that maybe it is kinda-OK in the case of Swift address-only types that contain weak references because all manipulations of the side table are hidden from LLVM in the runtime, which is compiled separately. However, C++ address-only types can violate the expectations of nocapture and noalias in inline functions that are visible to LLVM, possibly leading to miscompiles.

Compare with Clang's code generation. Return values in C++:

struct S {
  S *s;
  char pass_me_indirectly[16];
};

// define dso_local void @_Z13has_nocapturev(%struct.S* noalias nocapture sret align 8 %0) local_unnamed_addr
S has_nocapture() {
  S s;
  s.s = 0;
  return s;
}

// define dso_local void @_Z17without_nocapturev(%struct.S* noalias sret align 8 %0) local_unnamed_addr
S without_nocapture() {
  S s;
  s.s = &s;
  return s;
}

Parameters in C++:

struct S {
  S(const S&);
  S &operator=(const S&);
  void opaque();
};

// define dso_local void @_Z13has_nocapture1S(%struct.S* nocapture %0) local_unnamed_addr
void has_nocapture(S s) {}

// define dso_local void @_Z17without_nocapture1S(%struct.S* %0) local_unnamed_addr
void without_nocapture(S s) {
    s.opaque();
}
@atrick
Copy link
Member

atrick commented May 21, 2020

In general, we assume that memory access within the Swift runtime is not visible to LLVM when compiling Swift code. Maybe we'd like it to be visible, but that requires a lot of work plumbing Swift types through LLVM, maybe using TBAA.

Also, I think we want to make assumptions about Swift types that don't apply to C++ types, so we better have a way for IRGen to tell them apart. Is it possible for a Swift struct to contain a C++ class without the type system knowing? Can we force Swift structs to opt-in to this feature via conformance? Or do we need to assume the worst about every Swift struct that doesn't have a fixed layout?

@gribozavr
Copy link
Collaborator Author

Thanks for taking a look, Andy! So judging from your response I think you agree that we have a bug at least for C++ types – could you confirm?

Is it possible for a Swift struct to contain a C++ class without the type system knowing?

If there is a resilience or generics boundary, the type system might know that it can't know. If a module exposes a resilient struct, it might include a C++ class by value now or in future.

Can we force Swift structs to opt-in to this feature via conformance?

Since we're just talking about cases where there is an abstraction boundary (like resilience), I think it would be acceptable. For values of non-concrete types in generic functions these LLVM attributes probably don't matter for optimization (right?) since code does not directly access the pointed-to memory anyway.

However, Swift's typical approach for library evolution is not to opt into flexibility, but to opt out of it – so it would be another attribute like @frozen. But what is the distinction that we're trying to draw here?

@atrick
Copy link
Member

atrick commented May 21, 2020

That sounds reasonable to me, but I really haven't thought about C++ support. My biggest concern with hiding C++ types within Swift types would be if the C++ type is non-copyable. Maybe we simply ban that outright. @rjmccall has given this some thought.

@gribozavr
Copy link
Collaborator Author

Noncopyable C++ types are a separate issue. There can be copyable C++ types that are address-only because they are non-trivially copyable, because, for example, they care about their memory location. for example, std::string with an SSO buffer can have a "data" pointer pointing either to the heap, or to the SSO buffer.

@rjmccall
Copy link
Member

C++ things can certainly end up visible to LLVM. If we need to be more conservative about `noalias` and `nocapture` when passing types that we don't fully understand statically, that seems reasonable to me. For the most important such case — an opaque generic type, or at least a type containing one — It's highly unlikely that there would be any interesting LLVM-level optimizations that `nocapture` would enable anyway, since all property accesses on a generic type would be done via opaque accessors.

@zoecarver
Copy link
Collaborator

@swift-ci create

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@zoecarver zoecarver added c++ interop Feature: Interoperability with C++ and removed CxxInterop labels Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. c++ interop Feature: Interoperability with C++ compiler The Swift compiler in itself IRGen LLVM IR generation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@rjmccall @gribozavr @atrick @zoecarver