Stream: t-compiler/const-eval

Topic: #55223


oli (Oct 22 2018 at 11:52, on Zulip):

@RalfJ what kind of error should we be reporting for https://github.com/rust-lang/rust/issues/55223#issuecomment-431811751 ?

oli (Oct 22 2018 at 11:53, on Zulip):

Dangling pointer deref?

oli (Oct 22 2018 at 11:54, on Zulip):

or should we just error out even earlier by checking whether all AllocIds are global?

RalfJ (Oct 22 2018 at 13:03, on Zulip):

how is it even managing to violate this invariant?

RalfJ (Oct 22 2018 at 13:03, on Zulip):

we should usually have an alignment+size for all allocations, or sth went really wrong

oli (Oct 22 2018 at 13:06, on Zulip):

Not if these allocations were created in a different EvalContext ;)

RalfJ (Oct 22 2018 at 13:07, on Zulip):

uh... what. if they are still around they must have been interned, right?

RalfJ (Oct 22 2018 at 13:08, on Zulip):

eh, interned isnt even the most relevant part; moved to tcx.alloc_map is

oli (Oct 22 2018 at 13:08, on Zulip):

we can't intern a deallocated allocation

oli (Oct 22 2018 at 13:08, on Zulip):

well we could

oli (Oct 22 2018 at 13:08, on Zulip):

do you want me to implement that rather than what I did in the PR?

RalfJ (Oct 22 2018 at 13:13, on Zulip):

I dont follow

RalfJ (Oct 22 2018 at 13:13, on Zulip):

also I dont know what you implemented, GH says the PR doesnt exist^^

RalfJ (Oct 22 2018 at 13:14, on Zulip):

AFAIK we walk the relocations and globalize all things that this refers to, don't we?

oli (Oct 22 2018 at 13:14, on Zulip):

lol. I implemented a check during validation that errors out if const_mode == true and the AllocId is not in tcx.alloc_map.lock()

RalfJ (Oct 22 2018 at 13:14, on Zulip):

(interning = putting into the arena, that just gives us a &'tcx Allocation; globalizing = putting into the global alloc_map which is a separate step. it took me quite a while to figure that out.)

oli (Oct 22 2018 at 13:15, on Zulip):

yes but we only do that if there's an Allocation

RalfJ (Oct 22 2018 at 13:15, on Zulip):

and why wouldnt there be? we error out if there isnt, dont we?

oli (Oct 22 2018 at 13:15, on Zulip):

if the AllocId's memory has been deallocated, then we don't intern anything

oli (Oct 22 2018 at 13:15, on Zulip):

nope, apparently not XD

oli (Oct 22 2018 at 13:15, on Zulip):

lemme check what happens there

RalfJ (Oct 22 2018 at 13:26, on Zulip):

ah, we assume that it it doesnt exist it has already been interned

oli (Oct 22 2018 at 13:26, on Zulip):

oh right

RalfJ (Oct 22 2018 at 13:26, on Zulip):

lol. I implemented a check during validation that errors out if const_mode == true and the AllocId is not in tcx.alloc_map.lock()

I dont think that is enough, there are other operations that query size/alignment of an allocation

oli (Oct 22 2018 at 13:26, on Zulip):

so... we add another variant to AllocType: Deallocated(Size, Align) ?

RalfJ (Oct 22 2018 at 13:27, on Zulip):

what is AllocType again?

RalfJ (Oct 22 2018 at 13:27, on Zulip):

it's not the same as MemoryKind, that much I remember^^

oli (Oct 22 2018 at 13:27, on Zulip):

https://github.com/rust-lang/rust/blob/66910ba686e1e89ff6cfbb46b3e5394bedbe5564/src/librustc/mir/interpret/mod.rs#L431

oli (Oct 22 2018 at 13:27, on Zulip):

I have PR cleaning that up, too, but that's a different story

RalfJ (Oct 22 2018 at 13:28, on Zulip):

oh. so it is actually an Alloc, not just its type

oli (Oct 22 2018 at 13:28, on Zulip):

yes

RalfJ (Oct 22 2018 at 13:28, on Zulip):

how would we even determine its size/alloc though during interning?

RalfJ (Oct 22 2018 at 13:28, on Zulip):

it is already gone

RalfJ (Oct 22 2018 at 13:28, on Zulip):

ah we could look at the dealloc_map I guess

RalfJ (Oct 22 2018 at 13:29, on Zulip):

(or whatever I called it)

oli (Oct 22 2018 at 13:29, on Zulip):

yes

oli (Oct 22 2018 at 13:29, on Zulip):

I'll implement that

oli (Oct 22 2018 at 13:30, on Zulip):

I dont think that is enough, there are other operations that query size/alignment of an allocation

it is, because we error out before anyone can see these values again

RalfJ (Oct 22 2018 at 13:30, on Zulip):

okay. that would maintain the invariant that we know size+alloc of every AllocId. might be too much complexity though?

RalfJ (Oct 22 2018 at 13:30, on Zulip):

also what is that message?^^

oli (Oct 22 2018 at 13:30, on Zulip):

pressed return too early

RalfJ (Oct 22 2018 at 13:32, on Zulip):

it is, because we error out before anyone can see these values again

hm... that could work

RalfJ (Oct 22 2018 at 13:32, on Zulip):

but if that's true we can do something even simpler, maybe?

RalfJ (Oct 22 2018 at 13:33, on Zulip):

cannot we just error out during interning if we see sth that is in the dead_map?

RalfJ (Oct 22 2018 at 13:33, on Zulip):

no extra state in tcx should be needed

oli (Oct 22 2018 at 13:33, on Zulip):

heh

oli (Oct 22 2018 at 13:33, on Zulip):

jap

RalfJ (Oct 22 2018 at 13:33, on Zulip):

or rather, something that is neither in the local nor the global map

oli (Oct 22 2018 at 13:33, on Zulip):

editing PR

RalfJ (Oct 22 2018 at 13:33, on Zulip):

so we have an invariant: all relocations in the global map, refer to AllocId also in the global map

RalfJ (Oct 22 2018 at 13:34, on Zulip):

(might be temporarily violated during interning because cycles, but well)

oli (Oct 22 2018 at 14:36, on Zulip):

done, and github is even showing stuff atm https://github.com/rust-lang/rust/pull/55262

oli (Oct 22 2018 at 14:37, on Zulip):

and not anymore -.-

RalfJ (Oct 22 2018 at 14:41, on Zulip):

;)

Last update: Nov 15 2019 at 21:00UTC