-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Promoteds can contain raw pointers, but these must still only point to immutable allocations #67603
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
Changes from all commits
10f439a
658c27b
1ea4466
eadfd63
b2c43dc
a81784a
7bd01ed
69ffe7b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -268,19 +268,27 @@ impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx | |
} | ||
} | ||
|
||
pub enum InternKind { | ||
/// The `mutability` of the static, ignoring the type which may have interior mutability. | ||
Static(hir::Mutability), | ||
Constant, | ||
Promoted, | ||
ConstProp, | ||
} | ||
|
||
pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>( | ||
ecx: &mut InterpCx<'mir, 'tcx, M>, | ||
// The `mutability` of the place, ignoring the type. | ||
place_mut: Option<hir::Mutability>, | ||
intern_kind: InternKind, | ||
ret: MPlaceTy<'tcx>, | ||
ignore_interior_mut_in_const_validation: bool, | ||
) -> InterpResult<'tcx> { | ||
let tcx = ecx.tcx; | ||
let (base_mutability, base_intern_mode) = match place_mut { | ||
let (base_mutability, base_intern_mode) = match intern_kind { | ||
// `static mut` doesn't care about interior mutability, it's mutable anyway | ||
Some(mutbl) => (mutbl, InternMode::Static), | ||
// consts, promoteds. FIXME: what about array lengths, array initializers? | ||
None => (Mutability::Not, InternMode::ConstBase), | ||
InternKind::Static(mutbl) => (mutbl, InternMode::Static), | ||
// FIXME: what about array lengths, array initializers? | ||
InternKind::Constant | InternKind::ConstProp => (Mutability::Not, InternMode::ConstBase), | ||
InternKind::Promoted => (Mutability::Not, InternMode::ConstBase), | ||
}; | ||
|
||
// Type based interning. | ||
|
@@ -338,10 +346,24 @@ pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>( | |
// We can't call the `intern_shallow` method here, as its logic is tailored to safe | ||
// references and a `leftover_allocations` set (where we only have a todo-list here). | ||
// So we hand-roll the interning logic here again. | ||
match base_intern_mode { | ||
InternMode::Static => {} | ||
InternMode::Const | InternMode::ConstBase => { | ||
// If it's not a static, it *must* be immutable. | ||
match intern_kind { | ||
// Statics may contain mutable allocations even behind relocations. | ||
// Even for immutable statics it would be ok to have mutable allocations behind | ||
// raw pointers, e.g. for `static FOO: *const AtomicUsize = &AtomicUsize::new(42)`. | ||
InternKind::Static(_) => {} | ||
// Raw pointers in promoteds may only point to immutable things so we mark | ||
// everything as immutable. | ||
// It is UB to mutate through a raw pointer obtained via an immutable reference. | ||
// Since all references and pointers inside a promoted must by their very definition | ||
// be created from an immutable reference (and promotion also excludes interior | ||
// mutability), mutating through them would be UB. | ||
// There's no way we can check whether the user is using raw pointers correctly, | ||
// so all we can do is mark this as immutable here. | ||
InternKind::Promoted => { | ||
alloc.mutability = Mutability::Not; | ||
oli-obk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
InternKind::Constant | InternKind::ConstProp => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, I was thinking about this branch here when asking about nested allocations in constants. Isn't this unreachable then? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is reachable, but I'm entirely unclean on why it is reachable const RAW_TRAIT_OBJ_CONTENT_INVALID: *const dyn Trait = &unsafe { BoolTransmute { val: 3 }.bl } as *const _; hits the See the MIR on https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=3bc6c8b165b614bbe6cf5566d30752de there's no I'm not really sure what's going on there. Technically, since we're inside a constant, shouldn't this trigger promotion and promote the transmuted value? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Under unleash it's also reached in const MUTABLE_BEHIND_RAW: *mut i32 = &UnsafeCell::new(42) as *const _ as *mut _; but that triggers the delay_span_bug as expected. Also under unleash, but without an ICE (just triggering the dynamic checks in interp): const MUTABLE_BEHIND_RAW: *mut i32 = &UnsafeCell::new(42) as *const _ as *mut _;
//~^ WARN: skipping const checks
const MUTATING_BEHIND_RAW: () = {
// Test that `MUTABLE_BEHIND_RAW` is actually immutable, by doing this at const time.
unsafe {
*MUTABLE_BEHIND_RAW = 99 //~ ERROR any use of this value will cause an error
}
}; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ah, this must be the "not-promotion" also described in this document (Ctrl-F "looks like"). That makes sense. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What I don't understand though is why There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (The PR doesn't change anything here so this doesn't block landing, but I'd really like to understand this and then see it documented.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would find documentation here immensely helpful. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't, which is why my answers are so confusing. They are just a brain dump of me discovering what is going on, not me understanding it entirely There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These issues are preexisting, so after this PR is merged I'll open a new one to write docs and actually figure out what is going on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Fair enough. :) |
||
// If it's a constant, it *must* be immutable. | ||
// We cannot have mutable memory inside a constant. | ||
// We use `delay_span_bug` here, because this can be reached in the presence | ||
// of fancy transmutes. | ||
|
@@ -364,6 +386,8 @@ pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>( | |
// dangling pointer | ||
throw_unsup!(ValidationFailure("encountered dangling pointer in final constant".into())) | ||
} else if ecx.tcx.alloc_map.lock().get(alloc_id).is_none() { | ||
// We have hit an `AllocId` that is neither in local or global memory and isn't marked | ||
// as dangling by local memory. | ||
span_bug!(ecx.tcx.span, "encountered unknown alloc id {:?}", alloc_id); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
// check-pass | ||
|
||
pub const FOO: &'static *const i32 = &(&0 as _); | ||
|
||
fn main() {} |
Uh oh!
There was an error while loading. Please reload this page.