Skip to content

Commit

Permalink
Rollup merge of rust-lang#123994 - chbaker0:fn-declare-visibility, r=…
Browse files Browse the repository at this point in the history
…petrochenkov

Use Default visibility for rustc-generated C symbol declarations

Previously, visibility for these symbols was determined by the `default-hidden-visibility` target option or the presence of `-Zdefault-hidden-visibility`. This leads to issue rust-lang#123427, where use of the flag leads to undefined hidden symbols (i.e., references that can never be resolved to an exported symbol from another shared library) for functions often provided by a platform shared library, such as `memcpy` and `memcmp` from `libc.so`.

References to symbols provided by shared libraries must have default visibility. Hidden visibility is mostly useful for _defined_ symbols.
  • Loading branch information
tgross35 authored Aug 1, 2024
2 parents 8e86c95 + a1c27e3 commit 5eca8b6
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 10 deletions.
16 changes: 6 additions & 10 deletions compiler/rustc_codegen_llvm/src/declare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,20 +83,16 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
unnamed: llvm::UnnamedAddr,
fn_type: &'ll Type,
) -> &'ll Value {
// Declare C ABI functions with the visibility used by C by default.
let visibility = if self.tcx.sess.default_hidden_visibility() {
llvm::Visibility::Hidden
} else {
llvm::Visibility::Default
};

declare_raw_fn(self, name, llvm::CCallConv, unnamed, visibility, fn_type)
// Declare C ABI functions with Default visibility to allow them to link
// dynamically with shared object-provided symbols later on. This is
// needed to link intrinsic-generated calls to e.g. libc.so symbols like
// memcmp.
declare_raw_fn(self, name, llvm::CCallConv, unnamed, llvm::Visibility::Default, fn_type)
}

/// Declare an entry Function
///
/// The ABI of this function can change depending on the target (although for now the same as
/// `declare_cfn`)
/// The ABI of this function can change depending on the target.
///
/// If there’s a value with the same name already declared, the function will
/// update the declaration and return existing Value instead.
Expand Down
15 changes: 15 additions & 0 deletions tests/codegen/default-hidden-visibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,18 @@ pub static tested_symbol: [u8; 6] = *b"foobar";
// DEFAULT: @{{.*}}default_hidden_visibility{{.*}}tested_symbol{{.*}} = constant
// YES: @{{.*}}default_hidden_visibility{{.*}}tested_symbol{{.*}} = hidden constant
// NO: @{{.*}}default_hidden_visibility{{.*}}tested_symbol{{.*}} = constant

pub fn do_memcmp(left: &[u8], right: &[u8]) -> i32 {
left.cmp(right) as i32
}

// CHECK: define {{.*}} @{{.*}}do_memcmp{{.*}} {
// CHECK: }

// `do_memcmp` should invoke core::intrinsic::compare_bytes which emits a call
// to the C symbol `memcmp` (at least on x86_64-unknown-linux-gnu). This symbol
// should *not* be `declare hidden`.

// DEFAULT: declare i32 @memcmp
// YES: declare i32 @memcmp
// NO: declare i32 @memcmp
8 changes: 8 additions & 0 deletions tests/ui/intrinsics/default-hidden-visibility-intrinsic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//@ build-pass
//@ compile-flags: -Zdefault-hidden-visibility=yes

#![crate_type = "dylib"]

pub fn do_memcmp(left: &[u8], right: &[u8]) -> i32 {
left.cmp(right) as i32
}

0 comments on commit 5eca8b6

Please sign in to comment.