Issue #3678: Remove wrappers and call foreign functions directly

[nikomatsakis]

r? @sanxiyn --- I'd appreciate it if you can take a look at the logic for C abis in particular. I added some comments with my best effort to understand what was going on. I hope to come back at some point and clean things up in that regard, since the code as is is written with different assumptions (i.e., that there will be a wrapper and so forth).

[brson]

Delightful 💃

[brendanzab]

yay!

[Aatch]

Needs rebase.

[nikomatsakis]

Come on bors.

[brendanzab]

whispers words of encouragement

[brendanzab]

You can do it @bors!

[Aatch]

@nikomatsakis I'm not sure what happened here. Silent failure while building std it seems.

It would be nice to get this in.

[MaikKlein]

Any news on this?

[nikomatsakis]

I rebased the patch and it now crashes in fail_with for some reason I have yet to determine. Now that #7262 is finished, I will turn my focus to that. I still don't know what causes the Windows failures either.

[brson]

@nikomatsakis It looks like this completely removes all wrappers from foreign calls? I believe this is incorrect and will result in similar bugs to the one that was causing OOM's in various benchmarks recently. What happens is that the callee claims to require a very large stack, then if it yields it will be holding on to the big stack. If many tasks are doing similarly then the process will use a very large amount of memory. Effectively:

#[fixedstacksegment]
fn call_native_fn() {
    libc::free();
    // Wasting a huge amount of stack by yielding to the scheduler
    yield();
}

It has been my understanding that fast_ffi requires foreign calls to have non-inlinable wrappers for this reason.

[nikomatsakis]

@brson This is certainly a possible side-effect, I just don't know
that I consider it a bug. There is no strategy that always does
the right thing with regard to switching stacks; sometimes explicit
Rust wrapper functions, possibly marked #[noinline], will be necessary.

So I guess the question is what we want the defaults to be. We could
continue to auto-generate such wrappers around foreign fns, but I
thought that we wanted to expose the raw C function pointers as the
default. There might still be a role for explicit, opt-in wrappers a
la deriving.

If we chose to continue generating wrappers by default, then the type
of an extern function would not be extern "C" fn but rather extern "Rust" fn, unless the user disabled wrapper generation.

One of the advantages of the old plan which made the stack switch
explicit is that it made the danger more evident to the user, but it
was certainly less convenient and possibly less efficient than the
FastFFI approach.

[thestinger]

Shouldn't the fixed_stack_segment attribute be on the C function, rather than the callee?

[nikomatsakis]

The way it's setup in this PR is that any Rust function which directly calls a C function requests a very large stack size (so that the C function has enough stack to run). This is the same as how the current setup works, except that in the current setup we auto-generate a Rust wrapper for every C function that is declared, whereas in this PR we expose the C functions directly, and expect people to write their own Rust wrappers. It's not clear to me what is the best policy, ultimately.

[brendanzab]

Any word on this? We really need it for OpenGL awesomeness. It would be great if Rust could have decent graphics support for 0.8, I'm itching to start promoting it. Along with GLFW/SDL, lmath (or another maths lib) and a GL function loader, it would make the language very compelling for graphics devs to start tinkering with.

We need this!

[MaikKlein]

@nikomatsakis Can't we just let the client decide?

[nikomatsakis]

ok, I found the windows problem, which was embarassingly simple: I wasn't calling CallWithConv but rather just Call.

[MaikKlein]

Any news on this?

I just had a deja-vu