Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If/when LLVM switches to deref-at-point semantics, start applying dereferenceable to Box args/returns and loads of dereferenceable types #94133

Open
erikdesjardins opened this issue Feb 18, 2022 · 1 comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-blocked Status: Blocked on something else such as an RFC or other implementation work. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@erikdesjardins
Copy link
Contributor

Latest LLVM issue for context (currently abandoned and may not be revived): https://reviews.llvm.org/D110745

  1. because dereferenceable currently means "dereferenceable everywhere", we avoid applying dereferenceable attribute to Box:

// `Box` (`UniqueBorrowed`) are not necessarily dereferenceable
// for the entire duration of the function as they can be deallocated
// at any time. Set their valid size to 0.
attrs.pointee_size = match kind {
PointerKind::UniqueOwned => Size::ZERO,
_ => pointee.size,
};

// we could apply `dereferenceable_at_point` to the `x` argument
fn foo(x: Box<u64>) -> u64 {
    let value = *x;
    // ...but we can't apply `dereferenceable_globally` to it, because that would allow `x` to be loaded from at this point, after the box is freed
    value + 1
}
  1. we could use the same logic to apply !dereferenceable metadata to loads, e.g. here:

fn scalar_load_metadata<'a, 'll, 'tcx>(
bx: &mut Builder<'a, 'll, 'tcx>,
load: &'ll Value,
scalar: abi::Scalar,
) {
match scalar.value {
abi::Int(..) => {
if !scalar.is_always_valid(bx) {
bx.range_metadata(load, scalar.valid_range);
}
}
abi::Pointer if !scalar.valid_range.contains(0) => {
bx.nonnull_metadata(load);
}
_ => {}
}
}
let val = if let Some(llextra) = place.llextra {
OperandValue::Ref(place.llval, Some(llextra), place.align)
} else if place.layout.is_llvm_immediate() {
let mut const_llval = None;
unsafe {
if let Some(global) = llvm::LLVMIsAGlobalVariable(place.llval) {
if llvm::LLVMIsGlobalConstant(global) == llvm::True {
const_llval = llvm::LLVMGetInitializer(global);
}
}
}
let llval = const_llval.unwrap_or_else(|| {
let load = self.load(place.layout.llvm_type(self), place.llval, place.align);
if let abi::Abi::Scalar(scalar) = place.layout.abi {
scalar_load_metadata(self, load, scalar);
}
load
});
OperandValue::Immediate(self.to_immediate(llval, place.layout))
} else if let abi::Abi::ScalarPair(a, b) = place.layout.abi {
let b_offset = a.value.size(self).align_to(b.value.align(self).abi);
let pair_ty = place.layout.llvm_type(self);
let mut load = |i, scalar: abi::Scalar, align| {
let llptr = self.struct_gep(pair_ty, place.llval, i as u64);
let llty = place.layout.scalar_pair_element_llvm_type(self, i, false);
let load = self.load(llty, llptr, align);
scalar_load_metadata(self, load, scalar);
self.to_immediate_scalar(load, scalar)
};
OperandValue::Pair(
load(0, a, place.align),
load(1, b, place.align.restrict_for_offset(b_offset)),
)
} else {
OperandValue::Ref(place.llval, None, place.align)
};

let x = Box::new(42);
let ref1: &&u64 = &x.as_ref();
// we could apply `!dereferenceable_at_point` to this load of `&u64`
let ref2: &u64 = *ref1;
dbg!(ref2);
drop(x);
// ...but we can't apply `!dereferenceable_globally` to it, because that would allow `ref2` to be loaded from at this point, after the box is freed

@rustbot label A-llvm T-compiler S-blocked

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-blocked Status: Blocked on something else such as an RFC or other implementation work. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 18, 2022
@erikdesjardins
Copy link
Contributor Author

And this should also be done for non-Freeze references (if the following lands), see #98017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-blocked Status: Blocked on something else such as an RFC or other implementation work. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants