set subsections_via_symbols for ld64 helper sections #139752

usamoi · 2025-04-13T15:56:43Z

rustbot · 2025-04-13T15:56:48Z

rustbot has assigned @Nadrieril.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2025-04-13T15:56:50Z

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

madsmtm

Thanks for the ping!

@rustbot label O-apple O-linkage

madsmtm · 2025-04-13T18:09:03Z

compiler/rustc_codegen_ssa/src/back/metadata.rs

+    if binary_format == BinaryFormat::MachO {
+        file.set_subsections_via_symbols();
+    }


It's not clear to me that this needs to be in create_object_file? Maybe it would be better to only use it in add_linked_symbol_object?

Also, MH_SUBSECTIONS_VIA_SYMBOLS is vastly under-documented, I'd really like to see a comment here explaining why it's safe for us to use.

Object::set_subsections_via_symbols says it should be called before add_section or add_subsection, so I feel it better to put it here (not error-prone).

I don't know whether it's safe or not, either. I searched on the Internet and found that this is the only way for Mach-O to implement GC.

My problem with having it in create_object_file is that it may negatively affect the other object files we create (which again is hard to tell for sure, since the docs around it are so limited).

Ah! I didn't notice that this function has multiple callers.

I will add a parameter to control this behavior.

tests/run-make/executable-no-mangle-strip/main.rs

madsmtm · 2025-04-13T18:13:43Z

tests/run-make/executable-no-mangle-strip/main.rs

+unsafe extern "C" {
+    unsafe static UNDEFINED: usize;
+}
+
+#[unsafe(no_mangle)]
+pub fn used() {
+    println!("UNDEFINED = {}", unsafe { UNDEFINED });
+}


Honestly, I'm kind of surprised that this worked in the past. Could you describe the use-case?

I'd also be interested, does this pattern work on other platforms?

It's used by https://github.com/pgcentralfoundation/pgrx. It's a framework for developing PostgreSQL extensions. This trick is used for writing hybrid (dylib and SQL generation) code in a library. Code about SQL generation could be compiled to an executable, since dylib-related code are GC-ed.

This should be reasonable usage. Please see #95604 and #95363 (comment).

Yes. It works for Linux, FreeBSD, and Windows, too.

Thanks for the answer. Might make sense to leave a comment in the file about this, and perhaps link to #139744, stating that it is a regression test for that?

Also, is the expected behaviour for the symbol to still be present, or do we expect the linker to completely strip it out? E.g. would dlsym(RTLD_DEFAULT, "used") work?

The symbol is present in fhe dynamic library. It's GC-ed iff the final artifact is the executable.

An idea then would perhaps be to add another test (or use UI-test revisions) with #![crate_type = "dylib"] and //@ dont-check-compiler-stderr that expectedly fails?

I think it's alreay tested in https://github.com/rust-lang/rust/tree/master/tests/run-make/c-link-to-rust-dylib.

No, I meant having test that fails when trying to link undefined symbols in a dylib

It is added.

jieyouxu · 2025-04-14T02:24:56Z

cc @bjorn3 @petrochenkov in case this is problematic

madsmtm · 2025-04-14T04:36:10Z

tests/ui/linking/executable-no-mangle-strip.rs

+#[unsafe(no_mangle)]
+pub unsafe fn used() {
+    println!("THIS_SYMBOL_SHOULD_BE_UNDEFINED = {}", unsafe { THIS_SYMBOL_SHOULD_BE_UNDEFINED });
+}


Nit: I know that #[no_mangle] implies it, but could we also add an explicit test for #[used]?

#[used] emits errors, while #[no_mangle] does not, on MacOS.

Don't know why.

Edit: using pr + lld, nightly + lld, stable + lld, stable + ld64, #[used] emits errors, too.

I know why now.

On MacOS, #[used] emits llvm.used. So #[used(compiler)] is needed here. Is it expected behavior?

TIL that #[used] is an alias for #[used(linker)] on some platforms:

rust/compiler/rustc_codegen_ssa/src/codegen_attrs.rs

Lines 195 to 223 in f836ae4

// Unfortunately, unconditionally using `llvm.used` causes

// issues in handling `.init_array` with the gold linker,

// but using `llvm.compiler.used` caused a nontrivial amount

// of unintentional ecosystem breakage -- particularly on

// Mach-O targets.

//

// As a result, we emit `llvm.compiler.used` only on ELF

// targets. This is somewhat ad-hoc, but actually follows

// our pre-LLVM 13 behavior (prior to the ecosystem

// breakage), and seems to match `clang`'s behavior as well

// (both before and after LLVM 13), possibly because they

// have similar compatibility concerns to us. See

// https://github.com/rust-lang/rust/issues/47384#issuecomment-1019080146

// and following comments for some discussion of this, as

// well as the comments in `rustc_codegen_llvm` where these

// flags are handled.

//

// Anyway, to be clear: this is still up in the air

// somewhat, and is subject to change in the future (which

// is a good thing, because this would ideally be a bit

// more firmed up).

let is_like_elf = !(tcx.sess.target.is_like_darwin

|| tcx.sess.target.is_like_windows

|| tcx.sess.target.is_like_wasm);

codegen_fn_attrs.flags |= if is_like_elf {

CodegenFnAttrFlags::USED

} else {

CodegenFnAttrFlags::USED_LINKER

};

Eventually #[used] should be changed to #[used(linker)] unconditionally anyway. Gold is deprecated upstream and broken with current rustc versions anyway: #139425

Nadrieril · 2025-04-16T18:27:02Z

r? codegen

usamoi · 2025-04-22T15:58:12Z

I hope this gets a beta backport to prevent pgrx from being broken on macOS.

@rustbot label +beta-nominated

rustbot assigned Nadrieril Apr 13, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 13, 2025

usamoi force-pushed the macos-used branch from 73a7a59 to 782d196 Compare April 13, 2025 16:22

rustbot added the A-run-make Area: port run-make Makefiles to rmake.rs label Apr 13, 2025

This comment was marked as outdated.

Sign in to view

madsmtm suggested changes Apr 13, 2025

View reviewed changes

madsmtm reviewed Apr 13, 2025

View reviewed changes

rustbot added the O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) label Apr 13, 2025

usamoi force-pushed the macos-used branch from 782d196 to 7a7548a Compare April 14, 2025 04:21

madsmtm reviewed Apr 14, 2025

View reviewed changes

usamoi force-pushed the macos-used branch 2 times, most recently from 6e175fc to 7d10cf7 Compare April 14, 2025 05:34

jieyouxu added the A-linkage Area: linking into static, shared libraries and binaries label Apr 14, 2025

usamoi force-pushed the macos-used branch from 7d10cf7 to e1088d5 Compare April 14, 2025 10:17

set subsections_via_symbols for ld64 helper sections

3d43603

jieyouxu removed the A-run-make Area: port run-make Makefiles to rmake.rs label Apr 14, 2025

usamoi force-pushed the macos-used branch from e1088d5 to 3d43603 Compare April 14, 2025 14:07

rustbot assigned saethlin and unassigned Nadrieril Apr 16, 2025

rustbot added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Apr 22, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

set subsections_via_symbols for ld64 helper sections #139752

set subsections_via_symbols for ld64 helper sections #139752

usamoi commented Apr 13, 2025

rustbot commented Apr 13, 2025

rustbot commented Apr 13, 2025

This comment was marked as outdated.

madsmtm left a comment •

edited

Loading

madsmtm Apr 13, 2025

usamoi Apr 13, 2025

madsmtm Apr 13, 2025

usamoi Apr 14, 2025

madsmtm Apr 13, 2025

usamoi Apr 13, 2025 •

edited

Loading

madsmtm Apr 13, 2025

madsmtm Apr 13, 2025

usamoi Apr 14, 2025

madsmtm Apr 14, 2025

usamoi Apr 14, 2025

madsmtm Apr 14, 2025

usamoi Apr 15, 2025

jieyouxu commented Apr 14, 2025

madsmtm Apr 14, 2025

usamoi Apr 14, 2025 •

edited

Loading

usamoi Apr 14, 2025 •

edited

Loading

bjorn3 Apr 14, 2025 •

edited

Loading

Nadrieril commented Apr 16, 2025

usamoi commented Apr 22, 2025

	// Unfortunately, unconditionally using `llvm.used` causes
	// issues in handling `.init_array` with the gold linker,
	// but using `llvm.compiler.used` caused a nontrivial amount
	// of unintentional ecosystem breakage -- particularly on
	// Mach-O targets.
	//
	// As a result, we emit `llvm.compiler.used` only on ELF
	// targets. This is somewhat ad-hoc, but actually follows
	// our pre-LLVM 13 behavior (prior to the ecosystem
	// breakage), and seems to match `clang`'s behavior as well
	// (both before and after LLVM 13), possibly because they
	// have similar compatibility concerns to us. See
	// https://github.com/rust-lang/rust/issues/47384#issuecomment-1019080146
	// and following comments for some discussion of this, as
	// well as the comments in `rustc_codegen_llvm` where these
	// flags are handled.
	//
	// Anyway, to be clear: this is still up in the air
	// somewhat, and is subject to change in the future (which
	// is a good thing, because this would ideally be a bit
	// more firmed up).
	let is_like_elf = !(tcx.sess.target.is_like_darwin
	\|\| tcx.sess.target.is_like_windows
	\|\| tcx.sess.target.is_like_wasm);
	codegen_fn_attrs.flags \|= if is_like_elf {
	CodegenFnAttrFlags::USED
	} else {
	CodegenFnAttrFlags::USED_LINKER
	};

set subsections_via_symbols for ld64 helper sections #139752

Are you sure you want to change the base?

set subsections_via_symbols for ld64 helper sections #139752

Conversation

usamoi commented Apr 13, 2025

rustbot commented Apr 13, 2025

rustbot commented Apr 13, 2025

This comment was marked as outdated.

madsmtm left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

usamoi Apr 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jieyouxu commented Apr 14, 2025

Choose a reason for hiding this comment

usamoi Apr 14, 2025 • edited Loading

Choose a reason for hiding this comment

usamoi Apr 14, 2025 • edited Loading

Choose a reason for hiding this comment

bjorn3 Apr 14, 2025 • edited Loading

Choose a reason for hiding this comment

Nadrieril commented Apr 16, 2025

usamoi commented Apr 22, 2025

madsmtm left a comment •

edited

Loading

usamoi Apr 13, 2025 •

edited

Loading

usamoi Apr 14, 2025 •

edited

Loading

usamoi Apr 14, 2025 •

edited

Loading

bjorn3 Apr 14, 2025 •

edited

Loading