Allow section override when using patchable-function-entries

[pmur]

View all comments

Sometimes it is necessary to group patchable function entrypoint records in distinct linker sections. This is the case for some bpf functions within the linux kernel which shouldn't be visible to ftrace.

Extend -Zpatchable-function-entry to accept an argument of the form prefix_nops,total_nops,record_section, which places all entry record into a user specified section.

Likewise, extend the patchable_function_entry attribute to accept an optional section="name" option to place a function into a specific section.

This is made possible by llvm attribute patchable-function-entry-section added in llvm 21.

[rustbot]

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

[rustbot]

r? @jackh726

rustbot has assigned @jackh726.
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

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 73 candidates
• Random selection from 19 candidates

[pmur]

I think this is an important feature which we weren't aware of during the initial implementation of #123115.

Adding this should allow Rust to reduce the need to implement mcount or fentry (essentially x86 only) for compatibility within the linux kernel, and likely reduces the need for rust-lang/rfcs#3917.

[pmur]

@rustbot reroll

[chenyukang]

@rustbot reroll

[mejrs]

What are the rules for valid section names? This implementation accepts every string. Is #[patchable_function_entry(section = "")] valid? Can I use names with null bytes in them?

View changes since this review

[mejrs]

Suggested change

	section: Option<String>,
	section: Option<Symbol>,

This will be simpler if you pass around the symbol (not String/&str) everywhere, just use as_str before calling llvm api.

[pmur]

I think this is initialized before string interning is available. Or, so the runtime crash leads me to believe. Is there somewhere to insert a late initialization?

[mejrs]

Ah, I wasn't aware of that, makes sense. I'd leave it as is then.

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[mejrs]

nit: usually we use Option<ErrorGuaranteed> for "has errored" variables. Feel free to implement it or not, at your option.

View changes since the review

[pmur]

Can that be done within the attribute parsing framework? I think I am constrained to the SingleAttributeParser trait.

[mejrs]

Functions like duplicate_key return ErrorGuaranteed for this reason.

[pmur]

What are the rules for valid section names?

I think that gets punted to the assembler, and the targets object file format. For ELF, I think that is any nul terminated string.

[mejrs]

What are the rules for valid section names?

I think that gets punted to the assembler, and the targets object file format. For ELF, I think that is any nul terminated string.

So this would fail at the codegen stage? It makes for a better user experience to reject it earlier, similar to #155817

[pmur]

Updated to handle nul's more gracefully. llvm doesn't handle the inline nul gracefully.

[mejrs]

Updated to handle nul's more gracefully. llvm doesn't handle the inline nul gracefully.

Thanks, this looks good. What about empty strings? IIRC with link_name, llvm would just make up something.

[pmur]

Thanks, this looks good. What about empty strings? IIRC with link_name, llvm would just make up something.

llvm is grumpy and percolates up an error. I think ELF allows an empty section name (whenever that would be useful, I am not sure). I'll add the check.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.