Set branch protection function attributes

[nikic]

Since LLVM 19, it is necessary to set not only module flags, but also function attributes for branch protection on aarch64. See llvm/llvm-project@e15d67c for the relevant LLVM change.

Fixes#127829.

[rustbot]

r? @Nadrieril

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

[Nadrieril]

r? @rust-lang/wg-llvm

[dianqk]

It seems the flags on the module are no longer being used? Can we remove this code?

rust/compiler/rustc_codegen_llvm/src/context.rs

Lines 277 to 312 in a5ee5cb

	ifletSome(BranchProtection{ bti, pac_ret }) = sess.opts.unstable_opts.branch_protection{
	if sess.target.arch == "aarch64"{
	unsafe{
	llvm::LLVMRustAddModuleFlagU32(
	llmod,
	llvm::LLVMModFlagBehavior::Min,
	c"branch-target-enforcement".as_ptr().cast(),
	bti.into(),
	);
	llvm::LLVMRustAddModuleFlagU32(
	llmod,
	llvm::LLVMModFlagBehavior::Min,
	c"sign-return-address".as_ptr().cast(),
	pac_ret.is_some().into(),
	);
	let pac_opts = pac_ret.unwrap_or(PacRet{leaf:false,key:PAuthKey::A});
	llvm::LLVMRustAddModuleFlagU32(
	llmod,
	llvm::LLVMModFlagBehavior::Min,
	c"sign-return-address-all".as_ptr().cast(),
	pac_opts.leaf.into(),
	);
	llvm::LLVMRustAddModuleFlagU32(
	llmod,
	llvm::LLVMModFlagBehavior::Min,
	c"sign-return-address-with-bkey".as_ptr().cast(),
	u32::from(pac_opts.key == PAuthKey::B),
	);
	}
	}else{
	bug!(
	"branch-protection used on non-AArch64 target; \
	this should be checked in rustc_session."
	);
	}
	}

[dianqk]

Should we use let pac_opts = pac_ret.unwrap_or(PacRet { leaf: false, key: PAuthKey::A });? See

rust/compiler/rustc_codegen_llvm/src/context.rs

Line 292 in a5ee5cb

let pac_opts = pac_ret.unwrap_or(PacRet{leaf:false,key:PAuthKey::A});

.

[nikic]

I don't think so. Note that, while that code makes it look like non-leaf a-key is the default, this is not actually the case, as sign-return-address is still set to false in the None case and the overall functionality is disabled. (I think the reason why it's always setting all module flags is to get correct module flag merging behavior during LTO, but not entirely sure on that.)

[nikic]

For reference, this matches the corresponding clang logic for the function attrs: https://github.com/llvm/llvm-project/blob/5bd3aef5e285cce793e3fc6b21299ac9d650a947/clang/lib/CodeGen/TargetInfo.cpp#L219-L222

[nikic]

It seems the flags on the module are no longer being used? Can we remove this code?

No, the module flags are still needed to emit the ELF attributes (and when creating functions with default attrs).

[dianqk]

It seems the flags on the module are no longer being used? Can we remove this code?

No, the module flags are still needed to emit the ELF attributes (and when creating functions with default attrs).

LGTM. :) Comparing with clang's code, this seems largely consistent. But, clang doesn't always add these attributes:

https://github.com/llvm/llvm-project/blob/a347bdb2b828272359e49a8ce9de9d6412950838/clang/lib/CodeGen/CodeGenModule.cpp#L1195-L1202

(Very complex options, especially under LTO.)

[cuviper]

LGTM as well!

@bors r=DianQK,cuviper rollup

[bors]

📌 Commit ea7625f has been approved by DianQK,cuviper

It is now in the queue for this repository.