Stream: t-compiler/const-eval

Topic: Listing env vars


Christian Poveda (Aug 27 2019 at 05:59, on Zulip):

I'm opening this topic to keep my questions about https://github.com/rust-lang/miri/issues/756 in the same place :P

Christian Poveda (Aug 27 2019 at 06:11, on Zulip):

I know that the error when trying to access environ is created here: https://github.com/rust-lang/miri/blob/master/src/machine.rs#L251

Christian Poveda (Aug 27 2019 at 06:12, on Zulip):

But I'm not sure how to actually write to that location before hand.

oli (Aug 27 2019 at 08:46, on Zulip):

you'll need to add more arguments

oli (Aug 27 2019 at 08:47, on Zulip):

where do you want to store the location?

oli (Aug 27 2019 at 08:47, on Zulip):

MemoryExtra?

Christian Poveda (Aug 27 2019 at 13:36, on Zulip):

where do you want to store the location?

"in the correct place", I think memory extra would be ok

oli (Aug 27 2019 at 13:40, on Zulip):

yea, then I guess add an &MemoryExtra arg to the function

Christian Poveda (Aug 27 2019 at 13:49, on Zulip):

Will do

Christian Poveda (Aug 28 2019 at 15:17, on Zulip):

After some hacks here and there I was able to allocate the environment variables and the environ pointer as specified in http://man7.org/linux/man-pages/man7/environ.7.html with the null at the end and everything

Christian Poveda (Aug 28 2019 at 15:18, on Zulip):

However when I try to run this example

fn main() {
    for x in std::env::vars_os() {
        println!("{:?}", x);
    }
}

i get an alignment error here

error[E0080]: Miri evaluation error: tried to access memory with alignment 1, but alignment 8 is required
   --> /home/christian/Workspace/contrib/rust/src/libstd/sys/unix/os.rs:440:41
    |
440 |         while environ != ptr::null() && *environ != ptr::null() {
    |                                         ^^^^^^^^ Miri evaluation error: tried to access memory with alignment 1, but alignment 8 is required
    |
    = note: inside call to `std::sys::unix::os::env` at /home/christian/Workspace/contrib/rust/src/libstd/env.rs:143:21
oli (Aug 28 2019 at 15:21, on Zulip):

well... environ() yields *mut *const *const c_char: https://github.com/rust-lang/rust/blob/ac21131f7859836cd3fcb39231c0162fd892d960/src/libstd/sys/unix/os.rs#L421

oli (Aug 28 2019 at 15:21, on Zulip):

the outer allocation containing the pointers needs to be pointer aligned

Christian Poveda (Aug 28 2019 at 15:21, on Zulip):

wait that's three pointers?

oli (Aug 28 2019 at 15:22, on Zulip):

the first one points to the environ static

Christian Poveda (Aug 28 2019 at 15:22, on Zulip):

like ***char?!?

oli (Aug 28 2019 at 15:22, on Zulip):

essentially

Christian Poveda (Aug 28 2019 at 15:22, on Zulip):

oh shit

Christian Poveda (Aug 28 2019 at 15:22, on Zulip):

I stopped at two pointers

Christian Poveda (Aug 28 2019 at 15:22, on Zulip):

ty I'll fix it

oli (Aug 28 2019 at 15:22, on Zulip):

the environ static is a Vec<String> essentially

oli (Aug 28 2019 at 15:22, on Zulip):

uh

oli (Aug 28 2019 at 15:22, on Zulip):

wiat

Christian Poveda (Aug 28 2019 at 15:22, on Zulip):

yes

oli (Aug 28 2019 at 15:22, on Zulip):

environ's type is just **char

Christian Poveda (Aug 28 2019 at 15:23, on Zulip):

yes that's what I thought

oli (Aug 28 2019 at 15:23, on Zulip):

but the alignment needs to be pointer-alignment

oli (Aug 28 2019 at 15:23, on Zulip):

because the environ var contains pointers

oli (Aug 28 2019 at 15:24, on Zulip):

only what is being pointed at may be u8 aligned

Christian Poveda (Aug 28 2019 at 15:24, on Zulip):

are you taking into account that before the line that errors they already did let environ = *environ?

Christian Poveda (Aug 28 2019 at 15:25, on Zulip):

so environ should be *char and *environ should be char which has 1 byte alignment i think

Christian Poveda (Aug 28 2019 at 15:26, on Zulip):

https://github.com/rust-lang/rust/blob/master/src/libstd/sys/unix/os.rs#L438

Christian Poveda (Aug 28 2019 at 15:27, on Zulip):

well... environ() yields *mut *const *const c_char: https://github.com/rust-lang/rust/blob/ac21131f7859836cd3fcb39231c0162fd892d960/src/libstd/sys/unix/os.rs#L421

but this would mean I have to alloc a pointer to my current environ that is just a **char, is that right?

oli (Aug 28 2019 at 15:52, on Zulip):

uh

oli (Aug 28 2019 at 15:52, on Zulip):

I'm confused

oli (Aug 28 2019 at 15:54, on Zulip):

so environ is an allocation containing an array of pointers pointing to allocations of null terminated strings

Christian Poveda (Aug 28 2019 at 15:55, on Zulip):

yes

Christian Poveda (Aug 28 2019 at 16:00, on Zulip):

and that array of pointers is also null terminated

oli (Aug 28 2019 at 16:00, on Zulip):

yes but it's the wrong alignment

Christian Poveda (Aug 28 2019 at 16:00, on Zulip):

I made it work

Christian Poveda (Aug 28 2019 at 16:00, on Zulip):

you were right

oli (Aug 28 2019 at 16:00, on Zulip):

XD

Christian Poveda (Aug 28 2019 at 16:00, on Zulip):

I need a third pointer

Christian Poveda (Aug 28 2019 at 16:00, on Zulip):

:P

Christian Poveda (Aug 28 2019 at 16:00, on Zulip):

needed*

oli (Aug 28 2019 at 16:00, on Zulip):

uh

Christian Poveda (Aug 28 2019 at 16:01, on Zulip):

why? IDK

Christian Poveda (Aug 28 2019 at 16:01, on Zulip):

but it worked

Christian Poveda (Aug 28 2019 at 16:03, on Zulip):

but in fact that makes things easier

Christian Poveda (Aug 28 2019 at 16:04, on Zulip):

The good thing is that now I know what other changes I need to write it properly

Christian Poveda (Aug 28 2019 at 16:05, on Zulip):

Screenshot-from-2019-08-28-11-04-47.png

Christian Poveda (Aug 28 2019 at 16:09, on Zulip):

I had to disable stacked borrows to make it work because find_foreign_static returns an Allocation<(),()>

oli (Aug 28 2019 at 16:15, on Zulip):

can you add more details? Not sure why stacked borrows needs to be disabled for this

oli (Aug 28 2019 at 16:15, on Zulip):

oh, it's hardcoded to those generic params?

oli (Aug 28 2019 at 16:16, on Zulip):

I guess you need to fiddle through the appropriate params

Christian Poveda (Aug 28 2019 at 16:16, on Zulip):

yes

Christian Poveda (Aug 28 2019 at 16:16, on Zulip):

well not harcoded

Christian Poveda (Aug 28 2019 at 16:16, on Zulip):

those are the default for Allocation

oli (Aug 28 2019 at 16:16, on Zulip):

yea

oli (Aug 28 2019 at 16:16, on Zulip):

but like you need to change rustc

Christian Poveda (Aug 28 2019 at 16:16, on Zulip):

Allocation<Tag = (), Extra =()>

Christian Poveda (Aug 28 2019 at 16:16, on Zulip):

Yes yes

Christian Poveda (Aug 28 2019 at 16:17, on Zulip):

No way I'm going to push this code, Ralf would kill me, it is horrible hahahah

oli (Aug 28 2019 at 17:10, on Zulip):

XD

oli (Aug 28 2019 at 17:11, on Zulip):

Ralf driven development

Christian Poveda (Aug 28 2019 at 18:48, on Zulip):

Let me show you some of my magic:

                let alloc = memory_extra.env_alloc.as_ref().unwrap().clone();
                let alloc = Allocation {
                        bytes: alloc.bytes,
                        relocations: Relocations::from_presorted(alloc.relocations.iter().map(|(s, (_, i))| (*s, ((), *i))).collect::<Vec<_>>()),
                        undef_mask: alloc.undef_mask,
                        align: alloc.align,
                        mutability: alloc.mutability,
                        extra: ()
                    };
                alloc
Christian Poveda (Aug 28 2019 at 18:48, on Zulip):

Untaging Done Wrong™

oli (Aug 28 2019 at 18:53, on Zulip):

:no_entry:

Christian Poveda (Aug 28 2019 at 18:55, on Zulip):

Solving this tagging thing is driving me nuts.

Christian Poveda (Aug 28 2019 at 18:58, on Zulip):

So, the output of Machine::find_foreign_static is tagged in Memory::get_static_alloc. I changed get_static_alloc inside rust so it doesn't tag the output of find_foreign_static. But now I need to tag this allocation inside miri. Any ideas?

oli (Aug 28 2019 at 19:22, on Zulip):

Uh... call tag_allocation ?

oli (Aug 28 2019 at 19:22, on Zulip):

Not sure if all info is available to do so

Christian Poveda (Aug 28 2019 at 19:26, on Zulip):

Nope I dont have an alloc_id

Christian Poveda (Aug 28 2019 at 19:26, on Zulip):

But I think I can leave it untagged, let me see

Christian Poveda (Aug 28 2019 at 19:26, on Zulip):

I mean, tagged with Tag::Untagged

Christian Poveda (Aug 28 2019 at 19:38, on Zulip):

Oh lol, the problem is not the tagging, is the extra

oli (Aug 28 2019 at 19:50, on Zulip):

Ah, that impls Default I think

oli (Aug 28 2019 at 19:50, on Zulip):

So just call Default::default()

Christian Poveda (Aug 28 2019 at 19:53, on Zulip):

At the end the thing is that I have an Allocation<Tag, ()> and I need an Allocation<Tag, AllocExtra> but I wanted to add the alloc_extra with a method instead of destructuring the allocation itself D:

Christian Poveda (Aug 28 2019 at 19:57, on Zulip):

I could do that by changing the Allocation impl

oli (Aug 28 2019 at 20:15, on Zulip):

Add a map_extra method

Christian Poveda (Aug 28 2019 at 20:16, on Zulip):

(deleted)

Christian Poveda (Aug 28 2019 at 20:17, on Zulip):

I was thinking about changing https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc/mir/interpret/allocation.rs.html#84 for

impl<Tag, Extra: Default> Allocation<Tag, Extra> { .. }
Christian Poveda (Aug 28 2019 at 20:17, on Zulip):

but if you think the method is better I will

oli (Aug 28 2019 at 20:22, on Zulip):

Oh heh

oli (Aug 28 2019 at 20:22, on Zulip):

Go right ahead

Christian Poveda (Aug 28 2019 at 20:26, on Zulip):

Uff...

Christian Poveda (Aug 28 2019 at 20:26, on Zulip):

it seems I broke something with all the tagging

Christian Poveda (Aug 28 2019 at 20:27, on Zulip):
error[E0080]: Miri evaluation error: no item granting write access to tag <17> found in borrow stack
   --> /home/christian/Workspace/contrib/rust/src/libstd/sys/unix/thread.rs:307:9
    |
307 |         PAGE_SIZE = os::page_size();
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Miri evaluation error: no item granting write access to tag <17> found in borrow stack
    |
    = note: inside call to `std::sys::unix::thread::guard::init` at /home/christian/Workspace/contrib/rust/src/libstd/rt.rs:33:26
    = note: inside call to `std::rt::lang_start_internal` at /home/christian/Workspace/contrib/rust/src/libstd/rt.rs:64:5
    = note: inside call to `std::rt::lang_start::<()>`
Christian Poveda (Aug 28 2019 at 20:27, on Zulip):

:D

Christian Poveda (Aug 28 2019 at 20:29, on Zulip):

It seems I need to properly tag the statics in miri

RalfJ (Aug 29 2019 at 06:38, on Zulip):

so environ is an allocation containing an array of pointers pointing to allocations of null terminated strings

no, environ is a ptr-sized allocation containing a pointer to an allocation containing a null-terminated array of pointers to null-terminated strings.

Christian Poveda (Aug 29 2019 at 06:38, on Zulip):

and that array of pointers is also null-terminated

RalfJ (Aug 29 2019 at 06:39, on Zulip):

indeed

RalfJ (Aug 29 2019 at 06:40, on Zulip):

also see https://github.com/rust-lang/rust/pull/63951#issuecomment-525921663

RalfJ (Aug 29 2019 at 06:41, on Zulip):

I just don't like the laziness aspect of that very much... the static should always exist, not just when it is used the first time

RalfJ (Aug 29 2019 at 06:41, on Zulip):

the trouble is that unlike the other statics we handle lazily, this one actually changes^^

oli (Aug 29 2019 at 07:01, on Zulip):

Too many pointers...

oli (Aug 29 2019 at 07:02, on Zulip):

Maybe we need a Serializer impl for Memory so we can use serde to just make rust datastructures become const memory

RalfJ (Aug 29 2019 at 07:18, on Zulip):

lol

Christian Poveda (Aug 29 2019 at 08:35, on Zulip):

Well idk I made it work but I had to disable like half of the stacked borrows check to do so

Christian Poveda (Aug 29 2019 at 08:37, on Zulip):

@RalfJ, your suggestion of adding the AllocId to find_foreign_staticshould be enough to solve the stacked borrows conflicts?

Christian Poveda (Aug 29 2019 at 16:53, on Zulip):

it worked :party_ball: :party_ball:

RalfJ (Aug 29 2019 at 16:54, on Zulip):

it worked :party_ball: :party_ball:

awesome :D

Christian Poveda (Aug 29 2019 at 16:54, on Zulip):

let me fix the docs

Christian Poveda (Aug 29 2019 at 16:58, on Zulip):

Oh wait, it actually didnt :(

Christian Poveda (Aug 29 2019 at 16:58, on Zulip):

I ran the wrong file with miri D:

Christian Poveda (Aug 29 2019 at 17:00, on Zulip):
error[E0080]: Miri evaluation error: trying to reborrow for Unique, but parent tag <1363> does not have an appropriate item in the borrow stack
   --> /home/christian/Workspace/contrib/rust/src/libstd/sys/unix/os.rs:423:5
    |
423 |     &mut environ
    |     ^^^^^^^^^^^^ Miri evaluation error: trying to reborrow for Unique, but parent tag <1363> does not have an appropriate item in the borrow stack
    |
    = note: inside call to `std::sys::unix::os::environ` at /home/christian/Workspace/contrib/rust/src/libstd/sys/unix/os.rs:438:28
    = note: inside call to `std::sys::unix::os::env` at /home/christian/Workspace/contrib/rust/src/libstd/env.rs:143:21
Christian Poveda (Aug 29 2019 at 17:06, on Zulip):

I think I'm going to read @RalfJ's blog before trying to change more stuff

Christian Poveda (Aug 29 2019 at 17:07, on Zulip):

Should I commit the changes adding the AllocId to find_foreign_static?

RalfJ (Aug 29 2019 at 18:12, on Zulip):

I think I'm going to read RalfJ's blog before trying to change more stuff

I dont think that'll help

RalfJ (Aug 29 2019 at 18:12, on Zulip):

you are not doing crazy aliasing here, you are just not tracking the pointer povenance properly somewhere

Christian Poveda (Aug 29 2019 at 18:23, on Zulip):

I'm doing the allocations in the same way as for argv but using MemoryKind::Env instead

Christian Poveda (Aug 29 2019 at 19:29, on Zulip):

Any Ideas on how can I debug this in a better way?

oli (Aug 29 2019 at 20:02, on Zulip):

Can you upload the code? Hard to make guesses in a blackbox manner

Christian Poveda (Aug 29 2019 at 20:02, on Zulip):

Sure

Christian Poveda (Aug 29 2019 at 20:04, on Zulip):

https://github.com/christianpoveda/miri/commit/529af9b6333b7c0403cc1fb88bc3772729fc09f1

Last update: Nov 15 2019 at 20:00UTC