Skip to content

Commit

Permalink
Rollup merge of rust-lang#128362 - folkertdev:naked-function-symbol-v…
Browse files Browse the repository at this point in the history
…isibility, r=bjorn3

add test for symbol visibility of `#[naked]` functions

tracking issue: rust-lang#90957

This test is extracted from rust-lang#128004

That PR attempts to generated naked functions as an extern function declaration, combined with a global asm block that provides the implementation for that declaration.

In order to link declaration and definition together, some flavor of external linking must be used: LLVM will error for other linkage types. Specifically the allowed options are `#[linkage = "external"]` and `#[linkage = "extern_weak"]`. That is kind of an implementation detail though: to the user, a naked function should just behave like a normal function.

Hence it should be visible to the linker under the same circumstances as a normal, vanilla function and have the same attributes (Weak, External). Getting this behavior right will require some care, so I think it's a good idea to lock it in now, before making any changes, to make sure we don't regress.

Are there any interesting cases that I missed here? E.g. is checking on different architectures worth it? I don't think the other binary types (rlib etc) are relevant here, but may be missing something.

r? `@bjorn3`
  • Loading branch information
tgross35 authored Aug 7, 2024
2 parents 9c09e87 + 1967951 commit 2e45a70
Show file tree
Hide file tree
Showing 2 changed files with 187 additions and 0 deletions.
89 changes: 89 additions & 0 deletions tests/run-make/naked-symbol-visibility/a_rust_dylib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
#![feature(naked_functions, asm_const, linkage)]
#![crate_type = "dylib"]

use std::arch::asm;

pub trait TraitWithConst {
const COUNT: u32;
}

struct Test;

impl TraitWithConst for Test {
const COUNT: u32 = 1;
}

#[no_mangle]
fn entry() {
private_vanilla();
private_naked();

public_vanilla_generic::<Test>();
public_naked_generic::<Test>();
}

extern "C" fn private_vanilla() -> u32 {
42
}

#[naked]
extern "C" fn private_naked() -> u32 {
unsafe { asm!("mov rax, 42", "ret", options(noreturn)) }
}

#[no_mangle]
pub extern "C" fn public_vanilla() -> u32 {
42
}

#[naked]
#[no_mangle]
pub extern "C" fn public_naked() -> u32 {
unsafe { asm!("mov rax, 42", "ret", options(noreturn)) }
}

pub extern "C" fn public_vanilla_generic<T: TraitWithConst>() -> u32 {
T::COUNT
}

#[naked]
pub extern "C" fn public_naked_generic<T: TraitWithConst>() -> u32 {
unsafe { asm!("mov rax, {}", "ret", const T::COUNT, options(noreturn)) }
}

#[linkage = "external"]
extern "C" fn vanilla_external_linkage() -> u32 {
42
}

#[naked]
#[linkage = "external"]
extern "C" fn naked_external_linkage() -> u32 {
unsafe { asm!("mov rax, 42", "ret", options(noreturn)) }
}

#[cfg(not(windows))]
#[linkage = "weak"]
extern "C" fn vanilla_weak_linkage() -> u32 {
42
}

#[naked]
#[cfg(not(windows))]
#[linkage = "weak"]
extern "C" fn naked_weak_linkage() -> u32 {
unsafe { asm!("mov rax, 42", "ret", options(noreturn)) }
}

// functions that are declared in an `extern "C"` block are currently not exported
// this maybe should change in the future, this is just tracking the current behavior
// reported in https://github.com/rust-lang/rust/issues/128071
std::arch::global_asm! {
".globl function_defined_in_global_asm",
"function_defined_in_global_asm:",
"ret",
}

extern "C" {
pub fn function_defined_in_global_asm();
}
98 changes: 98 additions & 0 deletions tests/run-make/naked-symbol-visibility/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
//@ ignore-windows
//@ only-x86_64
use run_make_support::object::read::{File, Object, Symbol};
use run_make_support::object::ObjectSymbol;
use run_make_support::targets::is_windows;
use run_make_support::{dynamic_lib_name, env_var, rfs, rustc};

fn main() {
let rdylib_name = dynamic_lib_name("a_rust_dylib");
rustc().arg("-Zshare-generics=no").input("a_rust_dylib.rs").run();

let binary_data = rfs::read(&rdylib_name);
let rdylib = File::parse(&*binary_data).unwrap();

// naked should mirror vanilla
not_exported(&rdylib, "private_vanilla");
not_exported(&rdylib, "private_naked");

global_function(&rdylib, "public_vanilla");
global_function(&rdylib, "public_naked");

not_exported(&rdylib, "public_vanilla_generic");
not_exported(&rdylib, "public_naked_generic");

global_function(&rdylib, "vanilla_external_linkage");
global_function(&rdylib, "naked_external_linkage");

// FIXME: make this work on windows (gnu and msvc). See the PR
// https://github.com/rust-lang/rust/pull/128362 for some approaches
// that don't work
//
// #[linkage = "weak"] does not work well on windows, we get
//
// lib.def : error LNK2001: unresolved external symbol naked_weak_linkage␍
// lib.def : error LNK2001: unresolved external symbol vanilla_weak_linkage
//
// so just skip weak symbols on windows (for now)
if !is_windows() {
weak_function(&rdylib, "vanilla_weak_linkage");
weak_function(&rdylib, "naked_weak_linkage");
}

// functions that are declared in an `extern "C"` block are currently not exported
// this maybe should change in the future, this is just tracking the current behavior
// reported in https://github.com/rust-lang/rust/issues/128071
not_exported(&rdylib, "function_defined_in_global_asm");

// share generics should expose the generic functions
rustc().arg("-Zshare-generics=yes").input("a_rust_dylib.rs").run();
let binary_data = rfs::read(&rdylib_name);
let rdylib = File::parse(&*binary_data).unwrap();

global_function(&rdylib, "public_vanilla_generic");
global_function(&rdylib, "public_naked_generic");
}

#[track_caller]
fn global_function(file: &File, symbol_name: &str) {
let symbols = find_dynamic_symbol(file, symbol_name);
let [symbol] = symbols.as_slice() else {
panic!("symbol {symbol_name} occurs {} times", symbols.len())
};

assert!(symbol.is_definition(), "`{symbol_name}` is not a function");
assert!(symbol.is_global(), "`{symbol_name}` is not marked as global");
}

#[track_caller]
fn weak_function(file: &File, symbol_name: &str) {
let symbols = find_dynamic_symbol(file, symbol_name);
let [symbol] = symbols.as_slice() else {
panic!("symbol {symbol_name} occurs {} times", symbols.len())
};

assert!(symbol.is_definition(), "`{symbol_name}` is not a function");
assert!(symbol.is_weak(), "`{symbol_name}` is not marked as weak");
}

#[track_caller]
fn not_exported(file: &File, symbol_name: &str) {
assert_eq!(find_dynamic_symbol(file, symbol_name).len(), 0)
}

fn find_subsequence(haystack: &[u8], needle: &[u8]) -> bool {
haystack.windows(needle.len()).any(|window| window == needle)
}

fn find_dynamic_symbol<'file, 'data>(
file: &'file File<'data>,
expected: &str,
) -> Vec<Symbol<'data, 'file>> {
file.exports()
.unwrap()
.into_iter()
.filter(|e| find_subsequence(e.name(), expected.as_bytes()))
.filter_map(|e| file.symbol_by_name_bytes(e.name()))
.collect()
}

0 comments on commit 2e45a70

Please sign in to comment.