Lint idea: warn about implicit drop

[m-mueller678]

When writing low level code it is sometimes desirable to not have implicit drop calls.

For instance when interfacing with Lua one often has Rust function called by Lua, which in turn call Lua functions. One can execute a Lua function using lua_call. Errors in the Lua code are handled using longjmp, which can jump to a handler in the original Lua caller thus skipping the Execution of the rest of the Rust function. In such a situation one wants to make sure no code is executed implicitly after lua_call (because it might be skipped). Ideally this lint can be enabled both for single values that should not be dropped implicitly as well as functions in which nothing should be dropped implicitly

[Manishearth]

Given that most people will not be using this lint, I am concerned about the perf impact of the implementation, as currently all lint code is unconditionally run (with lint output being suppressed when lints are disabled). This can and should be changed ( https://github.com/rust-lang/rust/issues/59024) but in the current state lints requiring complex block analysis probably should not be added unless they are on by default

On Wed, Apr 3, 2019, 1:18 AM m-mueller678 notifications@github.com wrote:

When writing low level code it is sometimes desirable to not have implicit drop calls.

For instance when interfacing with Lua one often has Rust function called by Lua, which in turn call Lua functions. One can execute a Lua function using lua_call https://www.lua.org/manual/5.3/manual.html#lua_call. Errors in the Lua code are handled using longjmp, which can jump to a handler in the original Lua caller thus skipping the Execution of the rest of the Rust function. In such a situation one wants to make sure no code is executed implicitly after lua_call (because it might be skipped). Ideally this lint can be enabled both for single values that should not be dropped implicitly as well as functions in which nothing should be dropped implicitly

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust-clippy/issues/3915, or mute the thread https://github.com/notifications/unsubscribe-auth/ABivSKrt5UZucATA6ZisDFv94N_AlcCwks5vdGO8gaJpZM4cZ4tz .

[oli-obk]

Is that even safe? I'm thinking about a situation like below

fn foo(f: impl FnOnce()) {
    // do some setup
    catch_panic(f);
    // do some cleanup
}

and if you call this as

foo(|| {
    lua_call(...);
});

and lua_call does a longjmp, the cleanup from foo is never executed, even though execution continued. This isn't just about leaking memory and forgetting drops, this can cause actual UB.

[m-mueller678]

Is that even safe? I'm thinking about a situation like below

fn foo(f: impl FnOnce()) {
    // do some setup
    catch_panic(f);
    // do some cleanup
}

and if you call this as

foo(|| {
    lua_call(...);
});

and lua_call does a longjmp, the cleanup from foo is never executed, even though execution continued. This isn't just about leaking memory and forgetting drops, this can cause actual UB.

lua_call is indeed unsafe. Rust release notes 1.24.1 has some information on a related issue and seems to imply that usage is safe as long as:

• Lua never makes longjmp over Rust stackframes with non-copy types (I assume already dropped is fine)
• Rust code does not unwind into Lua code

rlua seems to handle this by only using lua_pcall which catches errors. I have not looked into it, but assume this messes with Lua error handlers. Error handlers in Lua are invoked immediately after an error is thrown e. g. to collect stacktraces. I think this can be worked around somewhat, but it is a niche feature anyways. So this approach is an acceptable workaround for this usecase.

I still think this could be a useful feature once rust-lang/rust#59024 is resolved.