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.

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

This naked function SIGSEGVs #32490

Closed
nagisa opened this issue Mar 25, 2016 · 10 comments · Fixed by #93153
Closed

This naked function SIGSEGVs #32490

nagisa opened this issue Mar 25, 2016 · 10 comments · Fixed by #93153
Labels
A-naked Area: `#[naked]`, prologue and epilogue-free, functions, https://git.io/vAzzS C-bug Category: This is a bug. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. requires-nightly This issue requires a nightly compiler in some way. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@nagisa
Copy link
Member

nagisa commented Mar 25, 2016

#![feature(naked_functions)]

#[naked]
fn naked(x: u32) -> u32 {
    x + 1
}

fn main(){
    naked(42);
}

SIGSEGVs on my system, but it only happens in builds without -O or -g flags, making it hard to debug.

@arielb1
Copy link
Contributor

arielb1 commented Mar 25, 2016

Naked functions that contain anything other than an asm blocks are at the very least unspecified behavior.

@nagisa
Copy link
Member Author

nagisa commented Mar 25, 2016

@arielb1 I do not disagree, however the RFC does not have any such requirements, and the RFC discussion seemed to imply it “just works”.

@nagisa
Copy link
Member Author

nagisa commented Mar 25, 2016

At the very least we should forbid non-single-asm-statement naked functions.

@nagisa
Copy link
Member Author

nagisa commented Mar 25, 2016

cc #32408

@ranma42
Copy link
Contributor

ranma42 commented Mar 26, 2016

Would it make sense to allow non-single-asm-statement naked parameterless functions?
AFAICT on supported platforms it should be possible to write a naked function in pure Rust as long as:

  • no parameters are passed
  • no unwinding is needed

@nagisa
Copy link
Member Author

nagisa commented Mar 27, 2016

Right, both of these must make rustc fail, but doesn’t currently.
Either way, this example also sigsegvs with many of the supported CCs
and with unsafe.

On Sun, 27 Mar, 2016 at 5:49 PM, Kai Noda [email protected]
wrote:

Defining a naked function with the default (Rust) ABI is an error,

naked functions must be declared unsafe or contain no non-unsafe
statements in the body.

These statements in RFC1201 drew my attention.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

@arielb1
Copy link
Contributor

arielb1 commented Mar 27, 2016

@nagisa

I don't think that we should prevent random undefined uses of naked functions from compiling unless we have a good set of rules. Maybe lint against them.

@tari
Copy link
Contributor

tari commented Apr 1, 2016

Asm-only naked functions are a very large potential footgun, mentioned as an unresolved question in the RFC since there wasn't consensus on what a reasonable approach is. Opinions tend to be split between those who are okay with the correctness of their code being at the whims of the code generator (allowing Rust code in naked functions, trading safety guarantees for improved efficiency) and those who prefer to ensure it is impossible to generate wrong code by accident.

The compromise solution is requiring the body of a naked function be unsafe (#32489), but that means (if made unsafe), I don't think the code sample in OP is a Rust bug.


The actual correctness of non-asm code in naked functions depends on the optimizer and code generator, which in general we cannot make any guarantees about what it will do. A rule like "no parameters allowed" is not sufficient to ensure generated code is correct. I believe the only way to properly check for implied stack allocations is to perform some kind of checking pass after codegen, which would at the least require some significant compiler complexity.

A lint against non-asm code in naked functions sounds like a reasonable approach to further help avoid foot-shooting to me.

@nagisa nagisa added the A-naked Area: `#[naked]`, prologue and epilogue-free, functions, https://git.io/vAzzS label Jun 2, 2016
@parched
Copy link
Contributor

parched commented Jun 26, 2016

I've hit an issue here too. I had a naked function as the entry point to some baremetal code. The first thing the function does is set up the stack pointer with some asm! then it has some rust code with automatic variables. I expected this to work but when I had a look at the llvm-ir the alloca for the automatic variable was placed before my asm!, not after.

I have gone with the pure asm! in the naked function which branches to another function with the rust code. This is fine but it is a little annoying needing the unnecessary branch.

edef1c pushed a commit to edef1c/libfringe that referenced this issue Aug 13, 2016
The core::intrinsics::unreachable() we used at the end of every naked
function is essentially pointless, as #[naked] implies that intrinsic
at the end of the function. rustc currently does not implement that
behavior (rust-lang/rust#32487), but it is a bug. On top of that,
anything except a single asm!() in naked functions is likely to be
disallowed in the future (rust-lang/rust#32490).

A nice side effect is that we avoid the core_intrinsics feature,
which will be never stabilized, though neither asm nor
naked_functions are likely to be stabilized soon.
edef1c pushed a commit to edef1c/libfringe that referenced this issue Feb 25, 2017
The core::intrinsics::unreachable() we used at the end of every naked
function is essentially pointless, as #[naked] implies that intrinsic
at the end of the function. rustc currently does not implement that
behavior (rust-lang/rust#32487), but it is a bug. On top of that,
anything except a single asm!() in naked functions is likely to be
disallowed in the future (rust-lang/rust#32490).

A nice side effect is that we avoid the core_intrinsics feature,
which will be never stabilized, though neither asm nor
naked_functions are likely to be stabilized soon.
@Mark-Simulacrum Mark-Simulacrum added C-bug Category: This is a bug. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. labels Jul 24, 2017
@jonas-schievink jonas-schievink added requires-nightly This issue requires a nightly compiler in some way. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Nov 22, 2019
@npmccallum
Copy link
Contributor

@nagisa This is probably fixed in this patch: #74105

@nagisa nagisa added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Aug 5, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 22, 2022
…nctions, r=Amanieu

Reject unsupported naked functions

Transition unsupported naked functions future incompatibility lint into an error:

* Naked functions must contain a single inline assembly block. Introduced as future incompatibility lint in 1.50 rust-lang#79653. Change into an error fixes a soundness issue described in rust-lang#32489.

* Naked functions must not use any forms of inline attribute. Introduced as future incompatibility lint in 1.56 rust-lang#87652.

Closes rust-lang#32490.
Closes rust-lang#32489.

r? `@Amanieu` `@npmccallum` `@joshtriplett`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 22, 2022
…nctions, r=Amanieu

Reject unsupported naked functions

Transition unsupported naked functions future incompatibility lint into an error:

* Naked functions must contain a single inline assembly block. Introduced as future incompatibility lint in 1.50 rust-lang#79653. Change into an error fixes a soundness issue described in rust-lang#32489.

* Naked functions must not use any forms of inline attribute. Introduced as future incompatibility lint in 1.56 rust-lang#87652.

Closes rust-lang#32490.
Closes rust-lang#32489.

r? ``@Amanieu`` ``@npmccallum`` ``@joshtriplett``
@bors bors closed this as completed in a8f64c0 Jan 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-naked Area: `#[naked]`, prologue and epilogue-free, functions, https://git.io/vAzzS C-bug Category: This is a bug. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. requires-nightly This issue requires a nightly compiler in some way. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants