Stream: t-lang/wg-unsafe-code-guidelines

Topic: https://github.com/rust-lang/rust/issues/63787


gnzlbg (Aug 23 2019 at 09:08, on Zulip):

I have some questions about this issue

gnzlbg (Aug 23 2019 at 09:08, on Zulip):

@RalfJ said that it was important to answer the question: Do we want non-repr(transparent)-structs containing references to be marked noalias?

gnzlbg (Aug 23 2019 at 09:08, on Zulip):

but this question feels like the wrong question to ask to me.

gnzlbg (Aug 23 2019 at 09:09, on Zulip):

It boils down to answering the question: Do we want struct Foo<'a, T>(&'a mut T) to be equivalent to &'a mut T ?

gnzlbg (Aug 23 2019 at 09:10, on Zulip):

There, Foo is not noalias, its only field is.

gnzlbg (Aug 23 2019 at 09:11, on Zulip):

When calling fn foo<'a, T>(x: Foo<'a, T>), whether x will be noalias or not, depends on the Abi of Foo, and the calling convention used.

rkruppe (Aug 23 2019 at 09:12, on Zulip):

Everything about the LLVM IR generated is ultimately influenced by "encoding details" such as how we translate the Rust function signature to LLVM function signatures. That doesn't seem very interesting or relevant to me.

gnzlbg (Aug 23 2019 at 09:12, on Zulip):

The interesting bit is whether soundness requires encoding details in a particular way

gnzlbg (Aug 23 2019 at 09:13, on Zulip):

E.g. if a field is of reference type and part of a non-repr-transparent Adt then its ABI is not "Reference" but "RawPointer"

gnzlbg (Aug 23 2019 at 09:14, on Zulip):

a direct consequence being there is a big and subtle difference between Foo and &mut T

rkruppe (Aug 23 2019 at 09:15, on Zulip):

If for silly reasons we don't communicate aliasing information we have to LLVM, then that's not a soundness issue, just a missed optimization.
I guess it's true that the real question could better be stated at a higher level (e.g., about retagging recursing into structures) which then justifies (or not) LLVM IR we generate, but this is just nitpicking IMO.

gnzlbg (Aug 23 2019 at 09:15, on Zulip):

The main question I have from that issue is whether the library types violate the "validity invariant" for references

gnzlbg (Aug 23 2019 at 09:15, on Zulip):

if we want references to be noalias, and their validity invariant makes sure that's always correct, how can passing Foo by memory or by value make a difference ?

rkruppe (Aug 23 2019 at 09:16, on Zulip):

Validity of reference isn't what justifies noalias, never was

gnzlbg (Aug 23 2019 at 09:16, on Zulip):

I thought it was part of validity that, e.g., &mut T references can't alias each other ?

gnzlbg (Aug 23 2019 at 09:16, on Zulip):

or is that only part of the aliasing model ?

rkruppe (Aug 23 2019 at 09:18, on Zulip):

Leaving aside the question of how you'd even say that in the validity predicate, noalias holds for the entirety of the function while validity is only asserted at typed copies, so validity can't very well ensure noalias is applicable

gnzlbg (Aug 23 2019 at 09:18, on Zulip):

So how do we know when we can use noalias ?

rkruppe (Aug 23 2019 at 09:19, on Zulip):

Stacked borrows (at least it's supposed to, there's open UCG issues about mismatches IIRC)

gnzlbg (Aug 23 2019 at 09:19, on Zulip):

In the examples, I have the feeling that we can make them correct, by instead of requiring noalias which holds for the whole function, using something else that only holds till last use.

rkruppe (Aug 23 2019 at 09:19, on Zulip):

That's not noalias though

gnzlbg (Aug 23 2019 at 09:19, on Zulip):

No

gnzlbg (Aug 23 2019 at 09:19, on Zulip):

It would be something that probably doesn't exist in LLVM

rkruppe (Aug 23 2019 at 09:20, on Zulip):

and for good reasons

gnzlbg (Aug 23 2019 at 09:20, on Zulip):

But @RalfJ mentions that we could make this a language bug

gnzlbg (Aug 23 2019 at 09:20, on Zulip):

A language fix would be, e.g., to only require in the aliasing model for the reference to not alias till last use

gnzlbg (Aug 23 2019 at 09:21, on Zulip):

which doesn't hold for these examples, and therefore noalias would be incorrect

rkruppe (Aug 23 2019 at 09:21, on Zulip):

where did he suggest that?

gnzlbg (Aug 23 2019 at 09:21, on Zulip):

https://github.com/rust-lang/rust/issues/63787#issuecomment-523968728

gnzlbg (Aug 23 2019 at 09:21, on Zulip):

They suggest that we can make this either a language bug, or a library bug

gnzlbg (Aug 23 2019 at 09:22, on Zulip):

For this to be a library bug, noalias would need to be correct, which means that the aliasing model would somehow have to require on function arguments that references don't alias for the whole function

gnzlbg (Aug 23 2019 at 09:22, on Zulip):

For this to be a language bug, noalias would need to be incorrect, which means the language would not have to require the above, but something else

rkruppe (Aug 23 2019 at 09:23, on Zulip):

I don't see the suggestion you describe there. On its own it just suggests we could fix the soundness issue by not using noalias there, which is true but doesn't imply we'll add anything more

gnzlbg (Aug 23 2019 at 09:23, on Zulip):

How do we justify no using noalias there is the question

rkruppe (Aug 23 2019 at 09:23, on Zulip):

By changing how stacked borrows (or whatever else justifies noalias) treats newtypes around references, obviously

gnzlbg (Aug 23 2019 at 09:24, on Zulip):

It's unclear to me whether that's better than calling this a library bug

rkruppe (Aug 23 2019 at 09:25, on Zulip):

Congratulations, you now have formed an opinion on the question at hand ;)

gnzlbg (Aug 23 2019 at 09:25, on Zulip):

I mean, without having an aliasing model at least, we can't really tell who is at fault here

gnzlbg (Aug 23 2019 at 09:25, on Zulip):

We can go either way

rkruppe (Aug 23 2019 at 09:25, on Zulip):

That's precisely the question

gnzlbg (Aug 23 2019 at 09:25, on Zulip):

But fixing this at the library level is trivial, and fixing this at the language level requires subtle changes to the Abi and calling conventions

rkruppe (Aug 23 2019 at 09:26, on Zulip):

I get the impression you've had your questions-about-the-question answered and are now thinking out loud. Have fun with that

gnzlbg (Aug 23 2019 at 09:26, on Zulip):

Thanks

gnzlbg (Aug 23 2019 at 09:27, on Zulip):

I'll think out loud in the issue

RalfJ (Aug 24 2019 at 11:24, on Zulip):

we could say we only care about aliasing of "bare" references. that doesn't change anything about their ABI, but it does mean that newtyped references get fewer optimizations, which might be undesirable.

gnzlbg (Aug 24 2019 at 11:56, on Zulip):

@RalfJ

I think we wall agree that it is legal to have C FFI with type Option<&mut T> where C uses a pointer, right?

Can you explain how could that work ?

gnzlbg (Aug 24 2019 at 11:56, on Zulip):

In C, T* and T* restrict are incompatible types, so you can't write a function definition using one, and write a declaration using the other, without invoking undefined behavior.

gnzlbg (Aug 24 2019 at 11:57, on Zulip):

Whether you provide the definition/declaration in Rust/C doesn't matter. That is, in C, the equivalent of:

gnzlbg (Aug 24 2019 at 11:58, on Zulip):
// crate A:
pub extern "C" fn foo(x: &mut T) { ... }
pub extern "C" fn bar(x: NonNull<T>) { ... }
// crate B:
extern "C" {
    fn foo(x: NonNull<T>);
    fn bar(x: &mut T);
}
gnzlbg (Aug 24 2019 at 11:59, on Zulip):

is UB.

gnzlbg (Aug 24 2019 at 12:01, on Zulip):

Because in the bar declaration x is noalias, in C at least it would be fine, e.g., using LTO, to make the NonNull<T> in the bar definition noalias as well.

gnzlbg (Aug 24 2019 at 12:03, on Zulip):

So if you are compiling Rust code with a fn bar(x: &mut T) definition, and linking that with C code using void bar(T* x), which calls bar with aliasing pointers, LTO could propagate noalias to bar C definition, and compile it under the assumption that x doesn't alias.

gnzlbg (Aug 24 2019 at 12:04, on Zulip):

That wouldn't break the Rust code calling it via &mut T, but it might break other code that is using NonNull<T> to avoid noalias

gnzlbg (Aug 24 2019 at 12:06, on Zulip):

I don't know if LLVM does this ""optimization"", but it could.

gnzlbg (Aug 24 2019 at 12:07, on Zulip):

E.g. a C header file can declare void foo(T* restrict x); and the C implementation file can implement it as void foo(T* x) { ... }, and you'd definitely want to make use of the fact that x is noalias when compiling the implementation

RalfJ (Aug 24 2019 at 12:12, on Zulip):

RalfJ

I think we wall agree that it is legal to have C FFI with type Option<&mut T> where C uses a pointer, right?

Can you explain how could that work ?

AFAIK many people do it? I recall being told by various people that this is a good way to write your FFI. And why would it not work? The FFI lint was even adjusted to be fine with this, IIRC. I am very confused about your opposition here.

As I said on GH, I don't think C's rules are very relevant unless LLVM copies them -- but in LLVM, attributes dont affect ABI.

RalfJ (Aug 24 2019 at 12:12, on Zulip):

also, do you know why C declares them incompatible? I don't quite see the point.

RalfJ (Aug 24 2019 at 12:13, on Zulip):

Similar to how in rust, mut isnt part of the signature (as in fn foo(mut x: i32), the same should apply to the aliasing stuff

rkruppe (Aug 24 2019 at 12:15, on Zulip):

[...] in LLVM, attributes dont affect ABI.

NB this is not generally true, consider sret or zeroext for example. I think it's true for noalias tho

RalfJ (Aug 24 2019 at 12:17, on Zulip):

hm okay. I was thinking of things like readonly etc as well

gnzlbg (Aug 24 2019 at 12:26, on Zulip):

To be more specific, in C, the following program has undefined behavior:

int foo(int* restrict x, int* restrict y);
int foo(int* x, int* y) { return *x + *y; }

Because the definition and the declaration must be compatible per C11 6.2.7p2 (https://port70.net/~nsz/c/c11/n1570.html#6.2.7p2) says:

All declarations that refer to the same object or function shall have compatible type; otherwise, the behavior is undefined.

For two function prototypes to be compatible, the function argument types must be compatible, per C11 6.7.6.3p15 (https://port70.net/~nsz/c/c11/n1570.html#6.7.6.3p15):

For two function types to be compatible, [...] corresponding parameters shall have compatible types

And two pointer types are only compatible if they are identically cvr-qualified, per C11 6.7.6.1p2 (https://port70.net/~nsz/c/c11/n1570.html#6.7.6.1p2) says:

For two pointer types to be compatible, both shall be identically qualified and both shall be pointers to compatible types.

RalfJ (Aug 24 2019 at 12:27, on Zulip):

I believe you that it does, but is there a good reason for this?

gnzlbg (Aug 24 2019 at 12:28, on Zulip):

It's quite common to properly qualify a declaration in a header file, which is where the API is specified, and then do "whatever" in the definition.

gnzlbg (Aug 24 2019 at 12:28, on Zulip):

C allows, for example, this code:

int foo();
int foo(int* x, int* y) { return *x + *y; }
gnzlbg (Aug 24 2019 at 12:29, on Zulip):

for compatibility with C89

gnzlbg (Aug 24 2019 at 12:29, on Zulip):

So backward compatibility with code written that way is probably a main reason as well.

RalfJ (Aug 24 2019 at 12:31, on Zulip):

how does adding more UB help with backwards compatibility?

gnzlbg (Aug 24 2019 at 12:40, on Zulip):

Note that C doesn't even require putting types on function declarations

gnzlbg (Aug 24 2019 at 12:41, on Zulip):

If you use the wrong types, or the wrong number of types, the behavior is undefined

gnzlbg (Aug 24 2019 at 12:42, on Zulip):

If your definition arguments are noalias, and you add a declaration whose types are not noalias, the behavior is undefined because the types do not exactly match.

gnzlbg (Aug 24 2019 at 12:43, on Zulip):

If your argument is that that is ok, then according to that, this is also ok

// crate A:
pub extern "C" fn foo(x: &mut T, y: &mut T) { ... }
// crate B:
extern "C" {
    fn foo(x: *mut T, y: *mut T);
}

for you right ?

gnzlbg (Aug 24 2019 at 12:46, on Zulip):

If the foo declaration is correct, then it is a correct prototype for foo, right? How do you then explain what's wrong with:

let mut x = ..
unsafe { foo(&mut x as *mut _, &mut x as *mut _) }
gnzlbg (Aug 24 2019 at 12:50, on Zulip):

In Rust, such a prototype is "incorrect, but possible to use correctly", and the function call uses it incorrectly, and that causes UB.

gnzlbg (Aug 24 2019 at 12:52, on Zulip):

I'm fine with explaining Option<&mut T> <-> T* that way, e.g., saying that in Rust it results in incorrect prototypes that can be used correctly, but that doing the exact same thing in C would be instant UB and who knows what LTO might do some day or whether C will fix that.

RalfJ (Aug 24 2019 at 16:07, on Zulip):

In Rust we can say that Option<&mut T>/Option<&T> are compatible with either T* _OR_ T* restrict, but not both

I disagree. They are incompatible on the C language level but we do not care about that, we only care about the ABI level, and there they are fine.

RalfJ (Aug 24 2019 at 16:08, on Zulip):

Why C does things this way would matter if we could change how C does things

no, it also matters to understand the rules. maybe there is a good reason. but if it's just C being silly, and LLVM didnt copy that silliness, there is no reason for us to even talk about it.

RalfJ (Aug 24 2019 at 16:09, on Zulip):

IIUC you want this to be correct UB free Rust code:

// crate A:
pub extern "C" fn foo(x: Option<&mut T>, y: Option<&mut T>) { ... }
// crate B:
extern "C" {
    fn foo(x: *mut T, y: *mut T);
}

we already guarantee that this works, IMO: https://rust-lang.github.io/unsafe-code-guidelines/layout/enums.html#discriminant-elision-on-option-like-enums

RalfJ (Aug 24 2019 at 16:10, on Zulip):

together with https://rust-lang.github.io/unsafe-code-guidelines/layout/pointers.html:

The layouts of &T, &mut T, *const T and *mut T are the same.

gnzlbg (Aug 24 2019 at 21:33, on Zulip):

I disagree. They are incompatible on the C language level but we do not care about that, we only care about the ABI level, and there they are fine.

This bans cross-lang-LTO

rkruppe (Aug 24 2019 at 21:34, on Zulip):

xLTO happens on the LLVM IR level, what are you talking about?

gnzlbg (Aug 24 2019 at 21:34, on Zulip):

Or would at least impose some extra requirements on it

gnzlbg (Aug 24 2019 at 21:35, on Zulip):

hmm, wait, in LLVM-IR, noalias is not part of fn declarations right ?

gnzlbg (Aug 24 2019 at 21:37, on Zulip):

it is

gnzlbg (Aug 24 2019 at 21:38, on Zulip):

@rkruppe two llvm modules, one with a declaration declare void @foo(i32* noalias); and one with a definition define void @foo(i32* %0) { ... }

gnzlbg (Aug 24 2019 at 21:38, on Zulip):

a valid optimization for C would be to make the i32* in the definition noalias, because a declaration exists that makes it noalias

rkruppe (Aug 24 2019 at 21:39, on Zulip):

For most (all?) of this thread so far you've argued in terms of C level types and rules, which is plainly irrelevant on LLVM IR level (and hence, irrelevant for xLTO)

gnzlbg (Aug 24 2019 at 21:39, on Zulip):

the declaration containing noalias could come from Rust code, the definition could be C code

rkruppe (Aug 24 2019 at 21:40, on Zulip):

If you think there is an issue with the LLVM IR and optimizations on it (in the face of xLTO or otherwise) then please state it on that level, don't mix in different abstraction levels

gnzlbg (Aug 24 2019 at 21:40, on Zulip):

noalias in LLVM-IR is modeled after C restrict

gnzlbg (Aug 24 2019 at 21:42, on Zulip):

@rkruppe it is a problem on all levels

gnzlbg (Aug 24 2019 at 21:43, on Zulip):

Problem 1 is that xLTO makes Rust and C visible to each other

gnzlbg (Aug 24 2019 at 21:44, on Zulip):

Problem 2 is that Option<&mut T> can't be equivalent to both T* and T* restrict because they are not equivalent

gnzlbg (Aug 24 2019 at 21:45, on Zulip):

@RalfJ is correct in that the layout of mut T and &mut T and T and T* restrict is the same

rkruppe (Aug 24 2019 at 21:45, on Zulip):

It mixes LLVM IR generated from both languages. So for example the stuff about cvr-qualified types you cited earlier is only relevant if and insofar it is reflected in the IR semantics. So whatever the problem is, I am very sure you can state it in terms of LLVM IR, which is also the best way to make it legible.

gnzlbg (Aug 24 2019 at 21:46, on Zulip):

but that does not mean that *mut T and &mut T can be freely substituted for each other

rkruppe (Aug 24 2019 at 21:46, on Zulip):

It clearly already doesn't mean that by Rust's rules alone, so this is not exactly a revelation

rkruppe (Aug 24 2019 at 21:47, on Zulip):

Yes, if you use &mut T in FFI signatures that also implies aliasing restrictions for the other side of the FFI. That's a far cry from there being an automatic ABI incompatibility.

gnzlbg (Aug 24 2019 at 21:49, on Zulip):

If C could see the declaration with mismatching cvr-quals, the behavior would be instantaneously undefined, per C own rules

gnzlbg (Aug 24 2019 at 21:50, on Zulip):

that's the problem, not ABI incompatibility - for ABI incompatibility, you'd at least need to call the function to trigger UB

rkruppe (Aug 24 2019 at 21:51, on Zulip):

I don't know how many more ways I can find to say that this is irrelevant because the interop and xLTO is in terms of LLVM IR, not C source code. I feel like I'm not getting this point through to you.

gnzlbg (Aug 24 2019 at 21:52, on Zulip):

In terms of LLVM-IR, the question is whether mismatching declarations and definitions when it comes to noalias are ok

rkruppe (Aug 24 2019 at 21:52, on Zulip):

Finally we agree on what's even the right question to ask ;)

rkruppe (Aug 24 2019 at 21:53, on Zulip):

A mismatch in the set of attributes isn't a problem per se. For example, attribute inference might add all sorts of attributes in one module where the body is visible, while being unable to affect the declarations in other modules. I see no reason why noalias would be different.

gnzlbg (Aug 24 2019 at 21:55, on Zulip):

With LTO, could LLVM propagate noalias from a declaration to a definition in another module ?

rkruppe (Aug 24 2019 at 21:55, on Zulip):

It's going to merge the attributes (or pick one of the signatures) ~somehow~ I guess

gnzlbg (Aug 24 2019 at 21:56, on Zulip):

That could be problematic if it were to happen

rkruppe (Aug 24 2019 at 21:56, on Zulip):

I don't see why this is all that interesting though. If the noalias is present anywhere and it's violated by passing aliasing pointers, that's UB right then and there, even if it may not be visible to the optimizer which looks at any particular module (or even set of modules).

gnzlbg (Aug 24 2019 at 21:56, on Zulip):

E.g. if C generates a definition without noalias, but because Rust uses Option<&mut T> in a declaration , LLVM merges them applying noalias to the definiton, something that the original C code did not do, and optimize the definition based on that

gnzlbg (Aug 24 2019 at 21:57, on Zulip):

Then some C code calls the definition with aliasing pointers, which would have been ok

gnzlbg (Aug 24 2019 at 21:58, on Zulip):

but because of the Rust declaration, now it is not

gnzlbg (Aug 24 2019 at 21:58, on Zulip):

The Rust code using Option<&mut T> might never pass aliasing pointers

gnzlbg (Aug 24 2019 at 21:58, on Zulip):

That isn't the issue

rkruppe (Aug 24 2019 at 21:58, on Zulip):

Oh I guess this is the same problem as https://github.com/rust-lang/rust/issues/46188

gnzlbg (Aug 24 2019 at 21:59, on Zulip):

I didn't knew about this issue

gnzlbg (Aug 24 2019 at 21:59, on Zulip):

but that's exactly what I had in mind

gnzlbg (Aug 24 2019 at 22:00, on Zulip):

The question is, can this also happen in Rust code ?

gnzlbg (Aug 24 2019 at 22:00, on Zulip):
// crate A:
fn foo(x: *mut T, y: *mut T) { ... }
// crate B:
extern "Rust" { fn foo(x: Option<&mut T>, y: Option<&mut T>); }
gnzlbg (Aug 24 2019 at 22:01, on Zulip):

Could the declaration in crate B result in x and y in the definition becoming noalias after LLVM optimizations ?

rkruppe (Aug 24 2019 at 22:01, on Zulip):

Sure, why not

gnzlbg (Aug 24 2019 at 22:01, on Zulip):

So then it isn't ok

gnzlbg (Aug 24 2019 at 22:02, on Zulip):

Option<&mut T> is not a type that you can use in an FFI declaration to interface with any pointer type at the other side

rkruppe (Aug 24 2019 at 22:02, on Zulip):

that's too strong a conclusion

gnzlbg (Aug 24 2019 at 22:02, on Zulip):

yes, i suppose we can be more careful with not emitting noalias in declarations

rkruppe (Aug 24 2019 at 22:03, on Zulip):

that's not what i meant

rkruppe (Aug 24 2019 at 22:04, on Zulip):

a declaration using Option<&mut T> implying it's noalias (and deref'able, and aligned, and ...) can be fine w.r.t. the definition, if it requires all those things anyway

gnzlbg (Aug 24 2019 at 22:05, on Zulip):

yes

gnzlbg (Aug 24 2019 at 22:05, on Zulip):

it could also be always fine, even if the definition doesn't require any of those

gnzlbg (Aug 24 2019 at 22:06, on Zulip):

we could say that it is a feature that you can write a definition without requirements, and that if you have a piece of code where you only use it with some requirements met, you can declare it with a "stronger" signature, and use that instead

gnzlbg (Aug 24 2019 at 22:09, on Zulip):

either way, this should be an explicit design decision, and not something that happens by accident

RalfJ (Aug 25 2019 at 08:40, on Zulip):
// crate A:
fn foo(x: *mut T, y: *mut T) { ... }
// crate B:
extern "Rust" { fn foo(x: Option<&mut T>, y: Option<&mut T>); }

won't the Rust stuff be mangled? Isn't this only a problem for extern "C" because there is no mangling?

RalfJ (Aug 25 2019 at 08:40, on Zulip):

or maybe mangling is always disabled for extern? I don't have a good model of extern "Rust"

RalfJ (Aug 25 2019 at 08:46, on Zulip):

to my knowledge, there's quite a bit of code out there that uses references (with or without Option) on the Rust side of an extern "C" function. some people say this helps them avoid mistakes when writing the FFI glue code.
For the case without Option, https://doc.rust-lang.org/nightly/nomicon/ffi.html#interoperability-with-foreign-code IMO says this is okay for references (and Box, when done right -- also see https://github.com/rust-lang/rust/pull/62514).
For the case with Option, https://doc.rust-lang.org/nightly/nomicon/ffi.html#the-nullable-pointer-optimization says very explicitly that this is okay for function pointers
I read the UCG documents as saying that this is also okay for references with and without Option.
For all of these cases we rely on it being okay in LLVM to have a function defined without noalias or dereferencable, and then to call it through a declaration with noalias etc.

gnzlbg (Aug 25 2019 at 09:13, on Zulip):

I don't have a good model of extern "Rust"

You do: extern "Rust" fn and fn are two ways of writing the same thing.

gnzlbg (Aug 25 2019 at 09:14, on Zulip):

For the case without Option, https://doc.rust-lang.org/nightly/nomicon/ffi.html#interoperability-with-foreign-code IMO says this is okay for references (and Box, when done right -- also see https://github.com/rust-lang/rust/pull/62514).

That is probably wrong, or at least, it did not consider all aspects of the problem. cc @Gankra

RalfJ (Aug 25 2019 at 09:14, on Zulip):

but fur extern "Rust" blocks... is there any name mangling?

gnzlbg (Aug 25 2019 at 09:14, on Zulip):

Rust one

gnzlbg (Aug 25 2019 at 09:14, on Zulip):

All Rust functions are extern "Rust"

gnzlbg (Aug 25 2019 at 09:15, on Zulip):

You just don't have to write it

RalfJ (Aug 25 2019 at 09:15, on Zulip):

I am talking specifically about extern blocks

gnzlbg (Aug 25 2019 at 09:15, on Zulip):

I know

gnzlbg (Aug 25 2019 at 09:15, on Zulip):

That provides declarations with some ABI

RalfJ (Aug 25 2019 at 09:15, on Zulip):

normal fns are not in an extern block, so your "all rust fns are" doesn't apply

gnzlbg (Aug 25 2019 at 09:16, on Zulip):

fn foo() { ... } is _exactly the same_ as extern "Rust" fn foo() { ... }

RalfJ (Aug 25 2019 at 09:16, on Zulip):

yes and that has nothing to do with my question :)

RalfJ (Aug 25 2019 at 09:16, on Zulip):

and if extern blocks have mangling that would mean it matters in which crate/module you define them? that makes little sense...

gnzlbg (Aug 25 2019 at 09:16, on Zulip):

when you call foo via other_crate::foo() that's exactly the same as doing extern "Rust" { fn other_crate_foo() } and calling that

gnzlbg (Aug 25 2019 at 09:17, on Zulip):

extern blocks have mangling

RalfJ (Aug 25 2019 at 09:17, on Zulip):

when you call foo via other_crate::foo() that's exactly the same as doing extern "Rust" { fn other_crate_foo() } and calling that

I dont think that can be right.

gnzlbg (Aug 25 2019 at 09:17, on Zulip):

the mangling of the ABI you choose (for C, that's none)

RalfJ (Aug 25 2019 at 09:18, on Zulip):

how does it figure out the module name etc?

gnzlbg (Aug 25 2019 at 09:18, on Zulip):

Good question

gnzlbg (Aug 25 2019 at 09:18, on Zulip):

In this case, they are in the same crate

gnzlbg (Aug 25 2019 at 09:18, on Zulip):

so that isn't an issue

gnzlbg (Aug 25 2019 at 09:18, on Zulip):

in different crates, I don't know

gnzlbg (Aug 25 2019 at 09:19, on Zulip):

The reference doesn't say

RalfJ (Aug 25 2019 at 09:19, on Zulip):

no mangling: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=6f976fe163c70645529ae6e1b15fd82a

RalfJ (Aug 25 2019 at 09:19, on Zulip):

(let it generate LLVM IR)

gnzlbg (Aug 25 2019 at 09:20, on Zulip):

So you'd need to write the appropriate #[link_name]

gnzlbg (Aug 25 2019 at 09:20, on Zulip):

or make the Rust item #[no_mangle]

gnzlbg (Aug 25 2019 at 09:20, on Zulip):

we should write that down in the reference

RalfJ (Aug 25 2019 at 09:21, on Zulip):

so I don't think your foo example above is an issue -- but if one would do the right mangling manually, it could be. that essentially a duplicate of https://github.com/rust-lang/rust/issues/28179 though.

RalfJ (Aug 25 2019 at 09:21, on Zulip):

we should write that down in the reference

yeah... https://doc.rust-lang.org/reference/items/external-blocks.html should probably say that there is no mangling?

gnzlbg (Aug 25 2019 at 09:24, on Zulip):

I dont think this has anything to do with #28179 .

gnzlbg (Aug 25 2019 at 09:24, on Zulip):
// crate A:
fn foo(x: *mut T, y: *mut T) { ... }
// crate B:
extern "Rust" { fn $mangled_foo(x: Option<&mut T>, y: Option<&mut T>); }
gnzlbg (Aug 25 2019 at 09:26, on Zulip):

You don't need any unsafe Rust code to make foo arguments noalias in its definition.

gnzlbg (Aug 25 2019 at 09:27, on Zulip):

And we have a well defined mangling scheme, so you could have a build.rs generate $mangled_foo and a macro expand it.

gnzlbg (Aug 25 2019 at 09:28, on Zulip):

This is a duplicate of https://github.com/rust-lang/rust/issues/46188

gnzlbg (Aug 25 2019 at 09:28, on Zulip):

It just happens that that issue is not restricted to extern "C"

gnzlbg (Aug 25 2019 at 09:35, on Zulip):

FWIW for noalias, at least right now, LLVM doesn't do this: https://llvm.godbolt.org/z/EPE_hE

RalfJ (Aug 25 2019 at 09:35, on Zulip):

has the same issue, no #[no_mangle] involved

yeah, not exactly the same issue but similar. there's also one there with link_name or so

gnzlbg (Aug 25 2019 at 09:35, on Zulip):

It inlines foo and applies noalias to a copy of foo

gnzlbg (Aug 25 2019 at 09:36, on Zulip):

(for noalias in argument position)

gnzlbg (Aug 25 2019 at 09:52, on Zulip):

Boom: https://llvm.godbolt.org/z/POZ7Xw

gnzlbg (Aug 25 2019 at 09:52, on Zulip):

It does happen for noalias in argument position

gnzlbg (Aug 25 2019 at 09:53, on Zulip):

(try commenting out the cfg(...), and see how the declarations are merged)

gnzlbg (Aug 25 2019 at 09:54, on Zulip):

What I haven't been able to trigger is to have that actually add noalias to a definition.

RalfJ (Aug 25 2019 at 10:07, on Zulip):

Boom: https://llvm.godbolt.org/z/POZ7Xw

so, what is that demonstrating?

RalfJ (Aug 25 2019 at 10:07, on Zulip):

looks like a fn ptr transmute to me

RalfJ (Aug 25 2019 at 10:07, on Zulip):

ah LLVM is applying the declaration of one fn to the other?

gnzlbg (Aug 25 2019 at 10:14, on Zulip):

yes

gnzlbg (Aug 25 2019 at 10:15, on Zulip):

LLVM transforms declare void @foo(i32*) to declare void @foo(i32* noalias align 4 dereferenceable(4))

gnzlbg (Aug 25 2019 at 10:16, on Zulip):

That means that even if the declaration of declare void @foo(i32*) would be correct, and code would use it correctly, maaaaybe UB could happen - it is unclear to me what the impact of those attributes on the declaration is for optimizations

gnzlbg (Aug 25 2019 at 10:17, on Zulip):

looking at the IR of good::baz(x), this function only calls a declaration, which requires x to be noalias, so AFAICT propagating all those attributes to baz argument would be ok

gnzlbg (Aug 25 2019 at 10:17, on Zulip):

but LLVM is not doing that

gnzlbg (Aug 25 2019 at 10:18, on Zulip):

That feels like a missed optimization though

gnzlbg (Aug 26 2019 at 11:32, on Zulip):

I wonder whether a part of the issue that says that:

fn foo(x: *mut T) { ... }
extern "Rust" {
    fn $foo_mangled(x: Option<&mut >);
}

is ok should be added to the reference.

gnzlbg (Aug 26 2019 at 11:32, on Zulip):

and the other issue should be used to track the bug.

gnzlbg (Aug 26 2019 at 11:33, on Zulip):

Or whether this being ok at all should be discussed first.

RalfJ (Aug 26 2019 at 17:13, on Zulip):

(dang, I posted this via reply-by-email and that does not seem to put things into the right topic. sorry.)

RalfJ (Aug 26 2019 at 17:14, on Zulip):

once that is settled, then what might remain is just the codegen bug that's already filled upstream

which codegen bug?

RalfJ (Aug 26 2019 at 17:15, on Zulip):

I don't know if that would fit the reference, or rust-lang/rust, or the rfc repo, but UCGs feels too niche for that

the UCG would be the right place for figuring out the rules. if you think we have consensus on the rules, this might be more of a reference or nomicon thing.

gnzlbg (Aug 26 2019 at 18:57, on Zulip):

which codegen bug?

https://github.com/rust-lang/rust/issues/46188

if you think we have consensus on the rules, this might be more of a reference or nomicon thing.

That boils down to whether we can say that this is ok or not:

#[no_mangle] extern "C" fn foo(x: *mut i32) { ... }
mod maybe_bad0 {
     extern "C" { fn foo(x: Option<&mut i32>); } // OK or UB ?
}
mod maybe_bad1 {
     extern "C" { fn foo(x: NonNull<i32>); } // OK or UB ?
}
// what if `foo` is defined in C? Does that change anything?
gnzlbg (Aug 26 2019 at 19:00, on Zulip):

I expect it there to be so much code using &mut T, NonNull<T>, or Option<&mut T> where C expects T*, that if we make any of that UB all FFI code would break.

gnzlbg (Aug 26 2019 at 19:01, on Zulip):

(deleted)

gnzlbg (Aug 26 2019 at 19:01, on Zulip):

Also, that code has no unsafe, so... if we make it UB, safe Rust with #[no_mangle] _or_ linking C would have UB, which it arguably already has, but it would be just another footgun

gnzlbg (Aug 26 2019 at 19:04, on Zulip):

If that code is OK, is it OK to add noalias to maybe_bad1::foo's argument ?

gnzlbg (Aug 26 2019 at 19:05, on Zulip):

Is it OK to add it to the foo definition ?

gnzlbg (Aug 26 2019 at 19:05, on Zulip):

Looks like there are enough questions for opening an UCG issue :/

RalfJ (Aug 26 2019 at 19:08, on Zulip):

;)

Last update: Nov 19 2019 at 18:30UTC