Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions compiler/rustc_codegen_llvm/src/back/lto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,10 @@ pub(crate) fn optimize_and_codegen_thin_module(
{
let target = &*module.module_llvm.tm;
let llmod = module.module_llvm.llmod();

let reloc_model = cgcx.relocation_model;
llvm::set_module_pic_and_pie_levels(llmod, reloc_model, &cgcx.crate_types);

@nikic nikic Jun 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused by why this is necessary. Aren't we just loading a previously serialized module here? Shouldn't the PIC/PIE flags have been set at the time the module was created?

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was surprised too. I believe the main contributor is that core & std libs end up w/o the PIE levels set at all, since they aren't being used in an executable. I can't recall if we have other examples or not, but its my vague recollection that we triggered this issue w/o use of core at all when codegen-units>1.

For our kernel specifically, this becomes an issue as we'd really like to use code from core. However, that would be a hard sell if we cannot prevent GOT indirection when accessing globals that would normally be safe to directly access (and have been accessed that way from C++ code for a very long time).

Do you think there's a better way to address this? Maybe somewhere in the lto.rs code?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the problem is that libcore/libstd are compiled as PIC but you want them to be PIE? And you're solving this by adding the PIE metadata during LTO? I don't think that's the right way to fix this.

Wouldn't the correct solution here to use build-std and enable PIE for libstd?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're being compiled into an executable, and our kernel builds core on demand as part of its build with all the same flags as the rest of the kernel. Under codegen-units>1 ThinLTO is always used, but the output isn't necessarily a PIE object, and this causes the kernel build to break due to the presence of GOT indirection (we have a rule preventing this in our linker script), even when we're not trying to use LTO if we don't setcodegen-units=1. This is all pretty surprising to us, as we didn't expect to have issues w/ LTO builds since they've been working forever in C++. FWIW, I'm pretty sure we tried setting all the various relocation-model flags on the core build w/o success, but we can try again.

I also think the test I added should just work w/ direct-access-external-data when compiling and linking against an rlib.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I'll be damned. That did solve the issue. So this does at least unblock me. Thanks for the suggestion. I had wrongly assumed that setting the platform defaults to position independent in the target spec would have amounted to the same thing, especially coupled with the fact that the Rust parts are only slotting into a minor part of the kernel build that's still mostly driven by clang.

I do still think the example in the test should be something that can work out of the box for platforms that default to PIE, though. That said, I'm guessing there may be a better way to spell that, such that we don't need about half of this patch. If you have thoughts about that I'm all ears.

In the meantime, unless there's an objection I'll pare this PR down to just adding the module flag and the minimal set of tests.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect setting it in the target spec to also work, as long as it's not explicitly overwritten on the command line. sess.relocation_model() looks like a pretty simple fallback to the target: self.opts.cg.relocation_model.unwrap_or(self.target.relocation_model).

In the meantime, unless there's an objection I'll pare this PR down to just adding the module flag and the minimal set of tests.

Sounds good to me.


save_temp_bitcode(cgcx, &module, "thin-lto-input");

// Up next comes the per-module local analyses that we do for Thin LTO.
Expand Down
29 changes: 12 additions & 17 deletions compiler/rustc_codegen_llvm/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,11 @@ use rustc_middle::ty::{self, Instance, Ty, TyCtxt};
use rustc_middle::{bug, span_bug};
use rustc_session::Session;
use rustc_session::config::{
BranchProtection, CFGuard, CFProtection, CrateType, DebugInfo, FunctionReturn, PAuthKey, PacRet,
BranchProtection, CFGuard, CFProtection, DebugInfo, FunctionReturn, PAuthKey, PacRet,
};
use rustc_span::{DUMMY_SP, Span, Spanned, Symbol, sym};
use rustc_target::spec::{
Arch, CfgAbi, Env, FramePointer, HasTargetSpec, Os, RelocModel, SmallDataThresholdSupport,
Target, TlsModel,
Arch, CfgAbi, Env, FramePointer, HasTargetSpec, Os, SmallDataThresholdSupport, Target, TlsModel,
};
use smallvec::SmallVec;

Expand Down Expand Up @@ -255,20 +254,7 @@ pub(crate) unsafe fn create_module<'ll>(
}

let reloc_model = sess.relocation_model();
if matches!(reloc_model, RelocModel::Pic | RelocModel::Pie) {
unsafe {
llvm::LLVMRustSetModulePICLevel(llmod);
}
// PIE is potentially more effective than PIC, but can only be used in executables.
// If all our outputs are executables, then we can relax PIC to PIE.
if reloc_model == RelocModel::Pie
|| tcx.crate_types().iter().all(|ty| *ty == CrateType::Executable)
{
unsafe {
llvm::LLVMRustSetModulePIELevel(llmod);
}
}
}
llvm::set_module_pic_and_pie_levels(llmod, reloc_model, tcx.crate_types());

// Linking object files with different code models is undefined behavior
// because the compiler would have to generate additional code (to span
Expand Down Expand Up @@ -517,6 +503,15 @@ pub(crate) unsafe fn create_module<'ll>(
);
}

if let Some(direct_access_external_data) = sess.direct_access_external_data() {
llvm::add_module_flag_u32(
llmod,
llvm::ModuleFlagMergeBehavior::Max,
"direct-access-external-data",
direct_access_external_data as u32,
);
}

match (sess.opts.unstable_opts.small_data_threshold, sess.target.small_data_threshold_support())
{
// Set up the small-data optimization limit for architectures that use
Expand Down
25 changes: 25 additions & 0 deletions compiler/rustc_codegen_llvm/src/llvm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::string::FromUtf8Error;
use libc::c_uint;
use rustc_abi::{AddressSpace, Align, Size, WrappingRange};
use rustc_llvm::RustString;
use rustc_session::config::CrateType;

pub(crate) use self::CallConv::*;
pub(crate) use self::CodeGenOptSize::*;
Expand Down Expand Up @@ -491,3 +492,27 @@ pub(crate) fn const_ptr_auth<'ll>(
&*result
}
}

/// Safe wrapper for setting the PIC and PIE levels via
/// `LLVMRustSetModulePICLevel` and `LLVMRustSetModulePIELevel`.
pub(crate) fn set_module_pic_and_pie_levels(
llmod: &Module,
reloc_model: rustc_target::spec::RelocModel,
crate_types: &[CrateType],
) {
if matches!(
reloc_model,
rustc_target::spec::RelocModel::Pic | rustc_target::spec::RelocModel::Pie
) {
unsafe {
LLVMRustSetModulePICLevel(llmod);
}
if reloc_model == rustc_target::spec::RelocModel::Pie
|| crate_types.iter().all(|ty| *ty == CrateType::Executable)
{
unsafe {
LLVMRustSetModulePIELevel(llmod);
}
}
}
}
4 changes: 3 additions & 1 deletion compiler/rustc_codegen_ssa/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use rustc_session::config::{
};
use rustc_span::source_map::SourceMap;
use rustc_span::{FileName, InnerSpan, Span, SpanData};
use rustc_target::spec::{MergeFunctions, SanitizerSet};
use rustc_target::spec::{MergeFunctions, RelocModel, SanitizerSet};
use tracing::debug;

use crate::back::link::ensure_removed;
Expand Down Expand Up @@ -343,6 +343,7 @@ pub struct CodegenContext {
pub split_debuginfo: rustc_target::spec::SplitDebuginfo,
pub split_dwarf_kind: rustc_session::config::SplitDwarfKind,
pub pointer_size: Size,
pub relocation_model: RelocModel,

/// LLVM optimizations for which we want to print remarks.
pub remark: Passes,
Expand Down Expand Up @@ -1288,6 +1289,7 @@ fn start_executing_work<B: WriteBackendMethods>(
split_dwarf_kind: tcx.sess.opts.unstable_opts.split_dwarf_kind,
parallel: backend.supports_parallel() && !sess.opts.unstable_opts.no_parallel_backend,
pointer_size: tcx.data_layout.pointer_size(),
relocation_model: sess.relocation_model(),
};

// This is the "main loop" of parallel work happening for parallel codegen.
Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1134,11 +1134,17 @@ LLVMRustSetDataLayoutFromTargetMachine(LLVMModuleRef Module,
}

extern "C" void LLVMRustSetModulePICLevel(LLVMModuleRef M) {
unwrap(M)->setPICLevel(PICLevel::Level::BigPIC);
Module *Mod = unwrap(M);
if (!Mod->getModuleFlag("PIC Level")) {
Mod->setPICLevel(PICLevel::Level::BigPIC);
}
}

extern "C" void LLVMRustSetModulePIELevel(LLVMModuleRef M) {
unwrap(M)->setPIELevel(PIELevel::Level::Large);
Module *Mod = unwrap(M);
if (!Mod->getModuleFlag("PIE Level")) {
Mod->setPIELevel(PIELevel::Level::Large);
}
}
Comment on lines 1136 to 1148

@ilovepi ilovepi Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not totally convinced this is 100% the right approach. Alternatively, we could set the merge strategy and do it that way, but I figured its probably better to only replace if it doesn't exist. Happy to amend the strategy here.

View changes since the review


extern "C" void LLVMRustSetModuleCodeModel(LLVMModuleRef M,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,7 @@ crate::target_spec_enum! {
}

crate::target_spec_enum! {
#[derive(Encodable, Decodable)]
pub enum RelocModel {
Static = "static",
Pic = "pic",
Expand Down
7 changes: 7 additions & 0 deletions tests/codegen-llvm/direct-access-external-data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,10 @@ pub fn refer() {
core::hint::black_box(EXTERNAL);
core::hint::black_box(WEAK);
}

// DIRECT: !{{[0-9]+}} = !{i32 7, !"direct-access-external-data", i32 1}

// INDIRECT: !{{[0-9]+}} = !{i32 7, !"direct-access-external-data", i32 0}

// DEFAULT-NOT: direct-access-external-data
// PIE-NOT: direct-access-external-data
25 changes: 25 additions & 0 deletions tests/run-make/lto-direct-access-external-data/dep.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#![feature(no_core, lang_items)]
#![no_std]
#![no_core]
#![crate_type = "lib"]

#[lang = "pointee_sized"]
trait PointeeSized {}
#[lang = "meta_sized"]
trait MetaSized: PointeeSized {}
#[lang = "sized"]
trait Sized: MetaSized {}

#[lang = "copy"]
pub trait Copy {}

impl Copy for i32 {}

unsafe extern "C" {
pub safe static VAR: i32;
}

#[no_mangle]
pub fn refer_dep() -> i32 {
unsafe { VAR }
}
65 changes: 65 additions & 0 deletions tests/run-make/lto-direct-access-external-data/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#![feature(no_core, lang_items)]
#![no_std]
#![no_core]

extern crate dep;

unsafe extern "C" {
pub safe static VAR: i32;
}

pub mod mod_0 {
use super::VAR;
#[no_mangle]
pub fn refer_0() -> i32 {
VAR
}
}

pub mod mod_1 {
use super::VAR;
#[no_mangle]
pub fn refer_1() -> i32 {
VAR
}
}

#[no_mangle]
pub fn call_dep() -> i32 {
dep::refer_dep()
}

// DEFAULT: @VAR = {{.*}}dso_local{{.*}}global i32
// DEFAULT-NOT: direct-access-external-data
// DEFAULT-NOT: PIE Level

// PIE: @VAR = external
// PIE-NOT: dso_local
// PIE-SAME: global i32
// PIE-NOT: direct-access-external-data
// PIE: !{{[0-9]+}} = !{i32 7, !"PIE Level", i32 2}

// DIRECT: @VAR = {{.*}}dso_local{{.*}}global i32
// DIRECT-DAG: !{{[0-9]+}} = !{i32 7, !"direct-access-external-data", i32 1}
// DIRECT-DAG: !{{[0-9]+}} = !{i32 7, !"PIE Level", i32 2}

// INDIRECT: @VAR = external
// INDIRECT-NOT: dso_local
// INDIRECT-SAME: global i32
// INDIRECT: !{{[0-9]+}} = !{i32 7, !"direct-access-external-data", i32 0}
// INDIRECT-NOT: PIE Level

// DEP-PIE: !{{[0-9]+}} = !{i32 7, !"PIE Level", i32 2}

// For Full & Thin LTO we expect direct PC-relative access.
// DIRECT-RELOC-NOT: R_X86_64_GOTPCREL{{.*}}VAR
// DIRECT-RELOC: R_X86_64_PC32{{.*}}VAR

// For indirect cases, we always expect GOT indirection.
// INDIRECT-RELOC: R_X86_64_GOTPCREL{{.*}}VAR
// INDIRECT-RELOC-NOT: R_X86_64_PC32{{.*}}VAR

// Default PIE without direct-access enabled should use GOT indirection.
// This is correct and is not changed by setting PIE Level consistently.
// PIE-RELOC: R_X86_64_GOTPCREL{{.*}}VAR
// PIE-RELOC-NOT: R_X86_64_PC32{{.*}}VAR
Loading
Loading