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

Naked functions with arguments generate a prologue #42779

Closed
Ekleog opened this issue Jun 20, 2017 · 4 comments · Fixed by #74105 or #75417
Closed

Naked functions with arguments generate a prologue #42779

Ekleog opened this issue Jun 20, 2017 · 4 comments · Fixed by #74105 or #75417
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-naked Area: `#[naked]`, prologue and epilogue-free, functions, https://git.io/vAzzS C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Ekleog
Copy link

Ekleog commented Jun 20, 2017

When compiling

#[naked]
pub extern fn naked_test(_fubar: u64) {
    unsafe { asm!("int3") }
}

(https://is.gd/Q03g7b)

The generated assembly includes a prologue with unexpected mov's (as well as a trailing ret, already reported in #32487)

Expected behaviour would be to have no prologue (and prevent compilation of a #[naked] function that is not extern as this would require implementing an undefined ABI, contrarily to what seems to happen https://is.gd/sQFBIy).

This is treacherous, as it dereferences some memory locations that may not be accessible, as happened to me and led to hours of debugging to finally figure out the compiled function was not the one defined.

cc #32408

@Mark-Simulacrum Mark-Simulacrum added A-naked Area: `#[naked]`, prologue and epilogue-free, functions, https://git.io/vAzzS T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 23, 2017
@alexcrichton
Copy link
Member

IIRC this is invalid and rustc should reject such a function, naked functions can't take arguments and the compiler should probably enforce that.

@Ekleog
Copy link
Author

Ekleog commented Jun 29, 2017

Hmm, my understanding of the RFC (“The following variant is not an error because the C calling convention is well-defined and it is thus possible for the programmer to write a conforming function”, even if the example is not with an argument) would mean that if the function is declared extern, then the function can actually respect the said ABI, thus should be well-formed?

@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 27, 2017
@alistair23
Copy link
Contributor

Is there an update on this?

I was hit by this issue as well. I hit the problem when compiling a tock app (baremetal-ish and jumped to by the tock OS)

A naked function with arguments would produce this prolog (RV32):

20430040 <_start>:
#[cfg(target_arch = "riscv32")]
#[doc(hidden)]
#[no_mangle]
#[naked]
#[link_section = ".start"]
pub unsafe extern "C" fn _start(
20430040: fec42823 sw a2,-16(s0)
20430044: feb42623 sw a1,-20(s0)
20430048: fea42423 sw a0,-24(s0)
2043004c: fed42a23 sw a3,-12(s0)
mem_start: usize,

Which would result in an invalid access as s0 would be 0.

Removing the arguments would remove the prologue and fix the problem.

Removing the naked cfg would also work as teh stack pointer was already setup by the OS and result in this.

20430040 <_start>:
/// the stack then calls rust_start() for the remainder of setup.
#[cfg(target_arch = "riscv32")]
#[doc(hidden)]
#[no_mangle]
#[link_section = ".start"]
pub unsafe extern "C" fn _start(
20430040: 1101 addi sp,sp,-32
20430042: ce06 sw ra,28(sp)
20430044: cc22 sw s0,24(sp)
20430046: 1000 addi s0,sp,32
20430048: fec42823 sw a2,-16(s0)
2043004c: feb42623 sw a1,-20(s0)
20430050: fea42423 sw a0,-24(s0)
20430054: fed42a23 sw a3,-12(s0)

If arguments with naked functions aren't supported rustc should generate an error as this is a difficult problem to debug.

@comex
Copy link
Contributor

comex commented Sep 27, 2019

This is an LLVM bug. Has it been reported upstream?

alistair23 added a commit to alistair23/rust that referenced this issue Sep 27, 2019
As mentioned in issue: rust-lang#42779
a naked function with arguments will cause LLVM to generate some
prologue code for the arguments. This goes against the idea of a naked
function and can cause difficult to diagnose bugs.

The current situation is wrong and leaves users exposed to nasty bugs.
There are two possible solutions:

  1. Not allow any naked functions to have arguments. This is the method
     taken by this patch.
  2. Modify LLVM to not generate any prologue code for naked functions.
     This seems like a more controversial change as it will impact all
     LLVM users and I am not sure how other languages will handle this.

It seems unlikely that there are many naked functions that don't call
inline assembly as the first or only code in the function. In which case
the inline assembly can assess the registers used to pass arguments
still allowing arguments to be passed to naked functions.

Rust shouldn't be calling naked functions [1] so this is unlikely to be
a major concern.

1: https://github.com/rust-lang/rfcs/blob/master/text/1201-naked-fns.md
   "Because the calling convention of a naked function is not guaranteed
    to match any calling convention the compiler is compatible with, calls
    to naked functions from within Rust code are forbidden unless the function
    is also declared with a well-defined ABI."

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
@jonas-schievink jonas-schievink added requires-nightly This issue requires a nightly compiler in some way. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. labels Dec 6, 2019
@Elinvynia Elinvynia added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 9, 2020
@LeSeulArtichaut LeSeulArtichaut removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 9, 2020
npmccallum added a commit to npmccallum/rust that referenced this issue Jul 6, 2020
A function that has no prologue cannot be reasonably expected to support
debuginfo. In fact, the existing code (before this patch) would generate
invalid instructions that caused crashes. We can solve this easily by
just not emitting the debuginfo in this case.

Fixes rust-lang#42779
cc rust-lang#32408
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 8, 2020
Suppress debuginfo on naked function arguments

A function that has no prologue cannot be reasonably expected to support
debuginfo. In fact, the existing code (before this patch) would generate
invalid instructions that caused crashes. We can solve this easily by
just not emitting the debuginfo in this case.

Fixes rust-lang#42779
cc rust-lang#32408
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 8, 2020
Suppress debuginfo on naked function arguments

A function that has no prologue cannot be reasonably expected to support
debuginfo. In fact, the existing code (before this patch) would generate
invalid instructions that caused crashes. We can solve this easily by
just not emitting the debuginfo in this case.

Fixes rust-lang#42779
cc rust-lang#32408
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 10, 2020
Suppress debuginfo on naked function arguments

A function that has no prologue cannot be reasonably expected to support
debuginfo. In fact, the existing code (before this patch) would generate
invalid instructions that caused crashes. We can solve this easily by
just not emitting the debuginfo in this case.

Fixes rust-lang#42779
cc rust-lang#32408
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 10, 2020
Suppress debuginfo on naked function arguments

A function that has no prologue cannot be reasonably expected to support
debuginfo. In fact, the existing code (before this patch) would generate
invalid instructions that caused crashes. We can solve this easily by
just not emitting the debuginfo in this case.

Fixes rust-lang#42779
cc rust-lang#32408
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 10, 2020
Suppress debuginfo on naked function arguments

A function that has no prologue cannot be reasonably expected to support
debuginfo. In fact, the existing code (before this patch) would generate
invalid instructions that caused crashes. We can solve this easily by
just not emitting the debuginfo in this case.

Fixes rust-lang#42779
cc rust-lang#32408
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 10, 2020
Suppress debuginfo on naked function arguments

A function that has no prologue cannot be reasonably expected to support
debuginfo. In fact, the existing code (before this patch) would generate
invalid instructions that caused crashes. We can solve this easily by
just not emitting the debuginfo in this case.

Fixes rust-lang#42779
cc rust-lang#32408
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 11, 2020
Suppress debuginfo on naked function arguments

A function that has no prologue cannot be reasonably expected to support
debuginfo. In fact, the existing code (before this patch) would generate
invalid instructions that caused crashes. We can solve this easily by
just not emitting the debuginfo in this case.

Fixes rust-lang#42779
cc rust-lang#32408
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 11, 2020
Suppress debuginfo on naked function arguments

A function that has no prologue cannot be reasonably expected to support
debuginfo. In fact, the existing code (before this patch) would generate
invalid instructions that caused crashes. We can solve this easily by
just not emitting the debuginfo in this case.

Fixes rust-lang#42779
cc rust-lang#32408
npmccallum added a commit to npmccallum/rust that referenced this issue Jul 25, 2020
A function that has no prologue cannot be reasonably expected to support
debuginfo. In fact, the existing code (before this patch) would generate
invalid instructions that caused crashes. We can solve this easily by
just not emitting the debuginfo in this case.

Fixes rust-lang#42779
cc rust-lang#32408
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 30, 2020
Suppress debuginfo on naked function arguments

A function that has no prologue cannot be reasonably expected to support
debuginfo. In fact, the existing code (before this patch) would generate
invalid instructions that caused crashes. We can solve this easily by
just not emitting the debuginfo in this case.

Fixes rust-lang#42779
cc rust-lang#32408
@bors bors closed this as completed in 2567074 Jul 30, 2020
npmccallum added a commit to npmccallum/rust that referenced this issue Aug 11, 2020
Currently, the code spills operands onto the stack for the purpose of
debuginfo. However, naked functions can only contain an asm block. Therefore,
attempting to spill the operands on the stack is undefined behavior.

Fixes rust-lang#42779
cc rust-lang#32408
tmandry added a commit to tmandry/rust that referenced this issue Aug 14, 2020
Don't spill operands onto the stack in naked functions

Currently, the code spills operands onto the stack for the purpose of
debuginfo. However, naked functions can only contain an asm block. Therefore,
attempting to spill the operands on the stack is undefined behavior.

Fixes rust-lang#42779
cc rust-lang#32408

Note that this PR reverts rust-lang#74105 which ultimately didn't fix the problem.

cc @haraldh @Amanieu @matthewjasper
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-naked Area: `#[naked]`, prologue and epilogue-free, functions, https://git.io/vAzzS C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
8 participants