Stream: t-compiler/const-eval

Topic: intptrcast model


Christian Poveda (Jun 05 2019 at 21:32, on Zulip):

This is just for disccussing https://github.com/rust-lang/miri/issues/224

Christian Poveda (Jun 05 2019 at 21:47, on Zulip):

So based on @oli instructions, the int_to_ptr method in Machine should be something like this:

Christian Poveda (Jun 05 2019 at 21:48, on Zulip):
    fn int_to_ptr(scalar: Scalar<Self::PointerTag>, extra: &Self::MemoryExtra) -> EvalResult<'tcx, Pointer<Self::PointerTag>> {
        match scalar {
            Scalar::Ptr(ptr) => Ok(ptr),
            _ => Err(EvalError::from(InterpError::ReadBytesAsPointer))
        }
    }
Christian Poveda (Jun 05 2019 at 21:50, on Zulip):

But then, based on my recent discussion with @RalfJ , this method should take an u64. So what should happen here? Because following the same logic would always return an error, i.e., if i use Scalar::from_uint to generate a scalar, it will be a Scalar::Raw.

Christian Poveda (Jun 05 2019 at 21:51, on Zulip):

and then, what should happen with ptr_to_int, should it return a Scalar or an u64?

oli (Jun 06 2019 at 06:47, on Zulip):

I can't find the discussion on int_to_ptr taking u64. I would have thought the above to be the correct way for const eval to implement int_to_ptr and I don't see how we could do this otherwise without duplicating the above match in a lot of places

RalfJ (Jun 06 2019 at 07:56, on Zulip):

@oli so my plan was to have operations force_bits and force_ptr in the InterpCtx that have type Scalar -> u128 and Scalar -> Pointer, respectively. They would be used instead of to_bits and to_ptr

RalfJ (Jun 06 2019 at 07:57, on Zulip):

and those are implemented using new machine hooks int_to_ptr and ptr_to_int with types u64 -> Pointer and Pointer -> u64

RalfJ (Jun 06 2019 at 07:57, on Zulip):

an operation called int_to_ptr IMO should have type int-to-ptr, or else something is off ;)

RalfJ (Jun 06 2019 at 07:58, on Zulip):

and your suggested ptr_to_int had return type Scalar which is annoying because it means every caller still needs to do to_bits

RalfJ (Jun 06 2019 at 07:58, on Zulip):

and moreover every caller would have to duplicate the match because really the operation we want is Scalar -> u128

oli (Jun 06 2019 at 10:29, on Zulip):

Right, that makes sense. So the machine hook for const eval would just ignore all its arguments and return the error

RalfJ (Jun 06 2019 at 10:55, on Zulip):

both of them, yes

Christian Poveda (Jun 06 2019 at 13:52, on Zulip):

Ohh OK, I'll do that then

Christian Poveda (Jun 06 2019 at 14:03, on Zulip):

I'll try to do the same replacements I had before and I'll write you if I get stuck

oli (Jun 06 2019 at 14:04, on Zulip):

as a first step you could just move the to_bits and to_ptr functions to InterpCx without touching Machine at all

oli (Jun 06 2019 at 14:04, on Zulip):

the diffs should get much simpler afterwards ^^

Christian Poveda (Jun 06 2019 at 14:39, on Zulip):

I decided to reset the branch :P

Christian Poveda (Jun 06 2019 at 14:39, on Zulip):

i decided to reset the branch (I opened a new topic by accident)

RalfJ (Jun 06 2019 at 14:55, on Zulip):

wow that seems drastic^^

Christian Poveda (Jun 06 2019 at 14:56, on Zulip):

hahaha maybe it is

Christian Poveda (Jun 06 2019 at 15:05, on Zulip):

So, force_bits would take a Scalar, how can I get an u128 from it using the new methods from Machine?, just call int_to_ptr and then ptr_to_int?

RalfJ (Jun 06 2019 at 15:12, on Zulip):

I am confused

RalfJ (Jun 06 2019 at 15:13, on Zulip):

int_to_ptr should take a u64 or u128 so that won't help^^

Christian Poveda (Jun 06 2019 at 15:13, on Zulip):

so. We have int_to_ptr and ptr_to_int

Christian Poveda (Jun 06 2019 at 15:13, on Zulip):

those are in Machine

RalfJ (Jun 06 2019 at 15:13, on Zulip):

you have this: https://github.com/rust-lang/rust/blob/8b36867093fb774bcbd9f787cbc470a5f44c1310/src/librustc/mir/interpret/value.rs#L305

RalfJ (Jun 06 2019 at 15:14, on Zulip):

then you can match on the result and either you already got a u128 or you call ptr_to_int

Christian Poveda (Jun 06 2019 at 15:14, on Zulip):

ohhh

Christian Poveda (Jun 06 2019 at 15:14, on Zulip):

thats evil

Christian Poveda (Jun 06 2019 at 15:14, on Zulip):

ok

RalfJ (Jun 06 2019 at 15:14, on Zulip):

force_bits calling int_to_ptr makes no sense, does it?

RalfJ (Jun 06 2019 at 15:14, on Zulip):

why evil?^^

Christian Poveda (Jun 06 2019 at 15:14, on Zulip):

and i guess i should cast the u64 to u128

Christian Poveda (Jun 06 2019 at 15:15, on Zulip):

why evil?^^

I was biased to not use the methods from Scalar directly for some reason :P

RalfJ (Jun 06 2019 at 15:18, on Zulip):

and i guess i should cast the u64 to u128

yes

RalfJ (Jun 06 2019 at 15:18, on Zulip):

hm actually there is a good question here

RalfJ (Jun 06 2019 at 15:18, on Zulip):

one testcase should do something like ... as *const i32 as usize as u8 as usize and make sure the value got truncated

RalfJ (Jun 06 2019 at 15:19, on Zulip):

I haven't looked at the cast code yet so I am not sure if it does the right thing there

RalfJ (Jun 06 2019 at 15:25, on Zulip):

we currently error in a cast of a pointer value to u8, so that's good

Christian Poveda (Jun 06 2019 at 15:35, on Zulip):

but *const i32 as usize shouldn't truncate anything i guess, just the as u8 should

Christian Poveda (Jun 06 2019 at 15:42, on Zulip):

ok it is done

Christian Poveda (Jun 06 2019 at 15:42, on Zulip):

https://github.com/rust-lang/rust/compare/master...christianpoveda:intptrcast-model

Christian Poveda (Jun 06 2019 at 15:42, on Zulip):

what do you think?

Christian Poveda (Jun 06 2019 at 16:10, on Zulip):

I moved the force methods to Memory but added hooks in InterpretCx

Christian Poveda (Jun 06 2019 at 20:06, on Zulip):

I've been replacing to_ptr by force_ptr and most of the time it causes UB or ends up with the following error when compiling rustc:

 a raw memory access tried to access part of a pointer value as raw bytes
RalfJ (Jun 07 2019 at 08:22, on Zulip):

please leave a comment on GH asking for review, I cant look at it right now and Zulip notifications are to ephemeral

RalfJ (Jun 07 2019 at 08:23, on Zulip):

emails are much better, I can actually organize them in a way that I can remember to get back to you ;)

Christian Poveda (Jun 07 2019 at 12:19, on Zulip):

Sure!

RalfJ (Jun 07 2019 at 17:09, on Zulip):

@Christian Poveda I apologize in advance for the conflicts https://github.com/rust-lang/rust/pull/61625 is going to cause you :(

Christian Poveda (Jun 07 2019 at 17:58, on Zulip):

Don't worry, I knew I had to resolve merge conflicts on this one eventually :P

Jake Goulding (Jun 10 2019 at 01:34, on Zulip):

Just passing through, but I recently ran into some issues casting pointers (https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/function.20pointers.20and.20address.20spaces) which may or may not be interesting to y'all

Christian Poveda (Jun 12 2019 at 14:10, on Zulip):

Should I start working on the miri changes?

oli (Jun 12 2019 at 14:14, on Zulip):

as commented on the PR I do not believe that miri will be able to pass its test suite after the current state of your PR

Christian Poveda (Jun 12 2019 at 14:18, on Zulip):

I just got your review

Christian Poveda (Jun 12 2019 at 14:19, on Zulip):

I'm gonna undo the operator changes and fix the size problem

Christian Poveda (Jun 12 2019 at 14:26, on Zulip):

compiling :counterclockwise:

Christian Poveda (Jun 12 2019 at 14:32, on Zulip):

man i gotta get one of those ryzen cpus, this is taking ages D:

Christian Poveda (Jun 12 2019 at 14:48, on Zulip):

where can I get the context for the to_usize call?

Christian Poveda (Jun 12 2019 at 14:48, on Zulip):

it is inside Memory

Christian Poveda (Jun 12 2019 at 14:49, on Zulip):

ohhh memory implements hasDataLayout

Christian Poveda (Jun 12 2019 at 14:49, on Zulip):

great

Christian Poveda (Jun 12 2019 at 14:49, on Zulip):

xD

oli (Jun 12 2019 at 14:50, on Zulip):

yea, just pass self :D

Christian Poveda (Jun 12 2019 at 14:52, on Zulip):

hahahaha yep i'm running tests after the rebase :clock:

Christian Poveda (Jun 12 2019 at 15:40, on Zulip):

i hope that git will believe me that this can be merged

oli (Jun 12 2019 at 15:40, on Zulip):

I'll have a look tomorrow, going offline for the day

Christian Poveda (Jun 12 2019 at 15:41, on Zulip):

sure, guten Abend :D (goddamn Tz)

Christian Poveda (Jun 13 2019 at 16:44, on Zulip):

I'm going to need a lecture about that last comment

Christian Poveda (Jun 13 2019 at 16:45, on Zulip):

i assume pointer sized types should have the same size as usize, right?

RalfJ (Jun 13 2019 at 16:51, on Zulip):

which comment?

Christian Poveda (Jun 13 2019 at 16:52, on Zulip):

IOW, the run-tme dispatch on the value here is just as ugly as the similar one in binops. We should dispatch only based on types, not on values... but we cannot yet due to pointers at integer types.

But what we can do is first dispatch based on whether the type is ptr-sized, and if yes proceed like now, and if not do basically self.cast_from_int(self.force_bits(data)).

RalfJ (Jun 13 2019 at 16:56, on Zulip):

that's for later, let's not do these casts now

Christian Poveda (Jun 13 2019 at 16:58, on Zulip):

yep i'm just gonna move the ptr size as a parameter in force_bits and replace to_bits in the places you mentioned

RalfJ (Jun 13 2019 at 16:59, on Zulip):

:+1:

RalfJ (Jun 13 2019 at 17:00, on Zulip):

what's this quote-only comment?^^ https://github.com/rust-lang/rust/pull/61781#discussion_r293481602

Christian Poveda (Jun 13 2019 at 17:01, on Zulip):

hahaha, my comment mixed with the quote, fixed it

Christian Poveda (Jun 13 2019 at 17:01, on Zulip):

this shouldnt take a lot of time brb

Christian Poveda (Jun 13 2019 at 17:07, on Zulip):

I added the ptr_size as an argument inside ecx also

RalfJ (Jun 13 2019 at 17:09, on Zulip):

yes

RalfJ (Jun 13 2019 at 17:09, on Zulip):

also the argument is the size, not the ptr size -- the entire point is that it might not be the ptr size ;)

RalfJ (Jun 13 2019 at 17:10, on Zulip):

the ptr size is always fixed for every execution, it's either 4 or 8 bytes (currently); the size is the size of this particular value

Christian Poveda (Jun 13 2019 at 17:11, on Zulip):

lol

Christian Poveda (Jun 13 2019 at 17:12, on Zulip):

if it compiles ill change the name before doing the commit

RalfJ (Jun 13 2019 at 17:32, on Zulip):

looks great :)

Christian Poveda (Jun 13 2019 at 18:34, on Zulip):

yay

Christian Poveda (Jun 13 2019 at 19:00, on Zulip):

tests are running :dancing:

RalfJ (Jun 13 2019 at 19:06, on Zulip):

time to write some testcases and then do the "real" work on the Miri side? :D

Christian Poveda (Jun 13 2019 at 19:19, on Zulip):

we'll see

Christian Poveda (Jun 13 2019 at 19:19, on Zulip):

where do we start?

Christian Poveda (Jun 14 2019 at 14:39, on Zulip):

time to write some testcases and then do the "real" work on the Miri side? :D

Should I follow @oli steps for the miri side?

oli (Jun 14 2019 at 14:40, on Zulip):

/me made steps?

oli (Jun 14 2019 at 14:40, on Zulip):

oh, right, I remember now

oli (Jun 14 2019 at 14:40, on Zulip):

I think Ralf wants to see tests first and implementation later

Christian Poveda (Jun 14 2019 at 14:42, on Zulip):

/me made steps?

maybe it was your inner Mr. Robot or something

Christian Poveda (Jun 14 2019 at 14:42, on Zulip):

ok so tests

Christian Poveda (Jun 14 2019 at 14:43, on Zulip):

I suck at writing tests

oli (Jun 14 2019 at 14:43, on Zulip):

Like let x: usize = &42 as *const i32; let y = x & 1; let z = x >> 1; let a = (z << 1) + y; let b = a as *const i32; let c = unsafe { *b }; assert_eq!(c, 42);

oli (Jun 14 2019 at 14:43, on Zulip):

I mean the extreme test case would be println!("{:p}", &42);

Christian Poveda (Jun 14 2019 at 14:43, on Zulip):

where do miri tests go?

oli (Jun 14 2019 at 14:43, on Zulip):

in the miri repo

Christian Poveda (Jun 14 2019 at 14:44, on Zulip):

oh the test directory structure is plain, great

Christian Poveda (Jun 14 2019 at 14:45, on Zulip):

I'm going to fork the repo and start playing with it a little bit

oli (Jun 14 2019 at 14:47, on Zulip):

ah

oli (Jun 14 2019 at 14:47, on Zulip):

you can do that inside your rustc checkout

oli (Jun 14 2019 at 14:48, on Zulip):

./x.py test src/tools/miri

oli (Jun 14 2019 at 14:48, on Zulip):

there's a full miri checkout in there

oli (Jun 14 2019 at 14:48, on Zulip):

or maybe use your custom checkout and follow the instructions in the miri repo

oli (Jun 14 2019 at 14:49, on Zulip):

you can create a rustup toolchain for your locally built rustc

Christian Poveda (Jun 14 2019 at 14:50, on Zulip):

ok this is new ground

Christian Poveda (Jun 14 2019 at 14:51, on Zulip):

mmm... So i can run the miri tests from x.py

Christian Poveda (Jun 14 2019 at 14:52, on Zulip):

but my changes to the miri repo shouldnt be in my fork and then do a PR against the rust-lang/miri repo?

oli (Jun 14 2019 at 14:54, on Zulip):

no

oli (Jun 14 2019 at 14:54, on Zulip):

in your fork against the miri repo

Christian Poveda (Jun 14 2019 at 14:54, on Zulip):

in your fork against the miri repo

oh yeah sorry i meant rust-lang/miri repo

oli (Jun 14 2019 at 14:55, on Zulip):

right, a branch in your fork, and PR against rust-lang/miri

Christian Poveda (Jun 14 2019 at 14:55, on Zulip):

but then how can I replace the miri repo inside the rustc repo by my fork?

oli (Jun 14 2019 at 14:55, on Zulip):

aaah

oli (Jun 14 2019 at 14:56, on Zulip):

heh, no need, just git remote add mine your_addr_here

oli (Jun 14 2019 at 14:56, on Zulip):

and then git push mine branch_name

Christian Poveda (Jun 14 2019 at 14:56, on Zulip):

ohh of course

Christian Poveda (Jun 14 2019 at 14:58, on Zulip):

I'm gonna do that, but as you can see my git-fu is not great: https://github.com/rust-lang/rust/pull/61668

oli (Jun 14 2019 at 15:01, on Zulip):

heh, just ask when you screw up, it's how I learned

oli (Jun 14 2019 at 15:01, on Zulip):

usually everything is salvageable

RalfJ (Jun 14 2019 at 16:15, on Zulip):

alternatively you can clone miri and work there locally, the miri README has instructions for that. which style you prefer is a matter of taste I guess^^

Christian Poveda (Jun 14 2019 at 16:18, on Zulip):

Ok I added the remotes and nothing blew up

RalfJ (Jun 14 2019 at 16:18, on Zulip):

:)

Christian Poveda (Jun 14 2019 at 16:18, on Zulip):

:D

Christian Poveda (Jun 14 2019 at 17:08, on Zulip):

So, rustc is compiling and its going to take a while. Tomorrow I'll tell you what happened to decide which should be good tests

Christian Poveda (Jun 14 2019 at 18:03, on Zulip):

It seems like miri is broken due to the Eval -> Interp changes, I'll try to pull when that gets fixed

RalfJ (Jun 14 2019 at 18:07, on Zulip):

uh, that got fixed long ago...

RalfJ (Jun 14 2019 at 18:07, on Zulip):

miri builds fine with current rustc master

RalfJ (Jun 14 2019 at 18:08, on Zulip):

just the miri in rustc hasnt received those updates yet, which might be your problem

Christian Poveda (Jun 14 2019 at 18:08, on Zulip):

ohh damn

RalfJ (Jun 14 2019 at 18:08, on Zulip):

TBH I feel it will be easier to work in a separate clone^^

RalfJ (Jun 14 2019 at 18:09, on Zulip):

submodules are annoying as hell to deal with

Christian Poveda (Jun 14 2019 at 18:10, on Zulip):

yeah I'll try to follow the instructions then

Christian Poveda (Jun 14 2019 at 18:21, on Zulip):

I tried to build miri with my locally built rustc and got an error:

Christian Poveda (Jun 14 2019 at 18:21, on Zulip):
error[E0107]: wrong number of lifetime arguments: expected 3, found 2
  --> src/lib.rs:78:25
   |
78 | ) -> InterpResult<'tcx, InterpretCx<'mir, 'tcx, Evaluator<'tcx>>> {
   |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected 3 lifetime arguments
Christian Poveda (Jun 14 2019 at 18:21, on Zulip):

well... a bunch of them tbh

Christian Poveda (Jun 14 2019 at 18:22, on Zulip):

maybe should I wait for my rustc changes to get merged and then try to build it

RalfJ (Jun 14 2019 at 18:22, on Zulip):

what does rustc --version say?

RalfJ (Jun 14 2019 at 18:22, on Zulip):

doesnt look like you are actually using latest master

Christian Poveda (Jun 14 2019 at 18:23, on Zulip):

rustc 1.37.0-dev

RalfJ (Jun 14 2019 at 18:23, on Zulip):

which commit

RalfJ (Jun 14 2019 at 18:23, on Zulip):

please dont cut off the most important part of the version number :)

Christian Poveda (Jun 14 2019 at 18:23, on Zulip):

there is no other part

RalfJ (Jun 14 2019 at 18:23, on Zulip):

oh dang it doesnt show that

RalfJ (Jun 14 2019 at 18:23, on Zulip):

I thought it would, sorry

RalfJ (Jun 14 2019 at 18:24, on Zulip):

so which commit did you build then?

Christian Poveda (Jun 14 2019 at 18:24, on Zulip):

on the last commit in my forked rustc

RalfJ (Jun 14 2019 at 18:25, on Zulip):

well then I can only guess^^

RalfJ (Jun 14 2019 at 18:25, on Zulip):

try git reset --hard 118274f300d60d8a450cdbc16a72101efde23b12 in your miri clone

Christian Poveda (Jun 14 2019 at 18:25, on Zulip):

no the horror

RalfJ (Jun 14 2019 at 18:26, on Zulip):

well welcome to the world of closely interlinked projects with unstable APIs ;)

RalfJ (Jun 14 2019 at 18:26, on Zulip):

but TBH I am wodnering why your PR didnt need rebasing for this...?

Christian Poveda (Jun 14 2019 at 18:27, on Zulip):

I'm not sure to be honest

RalfJ (Jun 14 2019 at 18:27, on Zulip):

well can you please at least tell me the exact git commit on which you are building rustc?^^

Christian Poveda (Jun 14 2019 at 18:27, on Zulip):

maybe it was because I synced my fork with master after my last fiasco

RalfJ (Jun 14 2019 at 18:27, on Zulip):

I am not clairvoyant^^

Christian Poveda (Jun 14 2019 at 18:27, on Zulip):

212f233b7d548c54ddec73142dfce1ee96a0c5c9

RalfJ (Jun 14 2019 at 18:28, on Zulip):

okay so its your PR. interesting that that did not need a rebase.

RalfJ (Jun 14 2019 at 18:28, on Zulip):

the issue is if you start working with that old miri you are going to get more conflicts

Christian Poveda (Jun 14 2019 at 18:29, on Zulip):

yeah I haven't done anything yet, I wanted to be sure that the build was successful

RalfJ (Jun 14 2019 at 18:29, on Zulip):

so you can either rebase your branch now (you dont have to push that, just do it locally), and then rebuild rustc; or you can move to an older miri like I suggested above but then you'll need more rebasing when you update your miri

Christian Poveda (Jun 14 2019 at 18:30, on Zulip):

I think I'm going to wait a little bit, right now I'm on my work machine and it is slow as hell, when I get home I'll try to rebase my branch or something

Christian Poveda (Jun 16 2019 at 06:54, on Zulip):

We'll have to step back a bit. For some reason the PR is not passing the tests when being rolled up. I rebased on local and runned the tests

Christian Poveda (Jun 16 2019 at 06:54, on Zulip):

got the following error

---- [run-make] run-make-fulldeps/linker-output-non-utf8 stdout ----

error: make failed
status: exit code: 2
command: "make"
stdout:
------------------------------------------
LD_LIBRARY_PATH="/home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8:/home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/stage2/lib:/home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools/x86_64-unknown-linux-gnu/release/deps:/home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/stage0/lib" '/home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/stage2/bin/rustc' --out-dir /home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8 -L /home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8  library.rs
mkdir /home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8/zzz$'\xff'
mv /home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8/liblibrary.a /home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8/zzz$'\xff'
LD_LIBRARY_PATH="/home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8:/home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/stage2/lib:/home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools/x86_64-unknown-linux-gnu/release/deps:/home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/stage0/lib" '/home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/stage2/bin/rustc' --out-dir /home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8 -L /home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8  -L /home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8/zzz$'\xff' exec.rs 2>&1 | "/home/christian/Workspace/contrib/rust/src/etc/cat-and-grep.sh" this_symbol_not_defined
[[[ begin stdout ]]]
error: Argument 6 is not valid Unicode: "/home/christian/Workspace/contrib/rust/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/linker-output-non-utf8/linker-output-non-utf8/zzz\xFF"


[[[ end stdout ]]]
Error: cannot match: this_symbol_not_defined
Christian Poveda (Jun 16 2019 at 06:56, on Zulip):

I'm not sure how my changes are affecting this

RalfJ (Jun 16 2019 at 08:00, on Zulip):

that looks spurious or like a platform-dependent thing

RalfJ (Jun 16 2019 at 08:00, on Zulip):

TBH I never run the fulldepds tests^^

RalfJ (Jun 16 2019 at 08:01, on Zulip):

if I want to be really thorough I do ./x.py test --stage 1 src/test/{run-pass,ui}

RalfJ (Jun 16 2019 at 08:02, on Zulip):

but also, CI in your branch passed

RalfJ (Jun 16 2019 at 08:02, on Zulip):

so why do you think it is your PR causing the failure in the tollrup?

RalfJ (Jun 16 2019 at 08:02, on Zulip):

one thing you could do is rebase and push that, to make sure CI in your branch actually runs against latest master

RalfJ (Jun 16 2019 at 08:03, on Zulip):

ah, I see... yeah https://github.com/rust-lang/rust/pull/61880#issuecomment-502417943 looks a lot like it might be your fault. but that's in Miri's test suite.

RalfJ (Jun 16 2019 at 08:03, on Zulip):

so what you could do is first rebase to get a working Miri again (that finally landed :D ), and then ./x.py test src/tools/miri

Christian Poveda (Jun 16 2019 at 08:04, on Zulip):

I rebased but I havent pushed it

Christian Poveda (Jun 16 2019 at 08:04, on Zulip):

going to run miri tests then

RalfJ (Jun 16 2019 at 08:05, on Zulip):

make sure you rebase to something that includes https://github.com/rust-lang/rust/pull/61832

RalfJ (Jun 16 2019 at 08:05, on Zulip):

which landed just 30min ago

Christian Poveda (Jun 16 2019 at 08:06, on Zulip):

yep I rebased like 5 mins ago

RalfJ (Jun 16 2019 at 08:08, on Zulip):

@Christian Poveda I think I found the bug: https://github.com/rust-lang/rust/pull/61781/files#r294072800

Christian Poveda (Jun 16 2019 at 08:10, on Zulip):

yay

Christian Poveda (Jun 16 2019 at 08:14, on Zulip):

I did the change but I'll run tests before pushing

Christian Poveda (Jun 16 2019 at 08:44, on Zulip):

I pushed my changes, got some trouble with the compilation pipeline but I think they'll be fine

RalfJ (Jun 16 2019 at 08:53, on Zulip):

So did ./x.py test src/tools/miri pass?

Christian Poveda (Jun 16 2019 at 08:54, on Zulip):

nope, apparently my rustc_llvm version is too old so I'll have to recompile to be able to build it

Edit: I'm building it on local but it will take a while

Christian Poveda (Jun 16 2019 at 08:54, on Zulip):

should I always test miri in this kind of changes?

RalfJ (Jun 16 2019 at 09:13, on Zulip):

well if you dont you might break miri which would make me sad ;)

Christian Poveda (Jun 16 2019 at 09:13, on Zulip):

Oh well, sure you can handle it

Christian Poveda (Jun 16 2019 at 09:13, on Zulip):

:P

RalfJ (Jun 16 2019 at 09:13, on Zulip):

;)

RalfJ (Jun 16 2019 at 09:14, on Zulip):

I usually test them with my separate miri clone, but that does not make much of a difference

RalfJ (Jun 16 2019 at 09:14, on Zulip):

you do have test-miri = true set in your config.toml, right?

Christian Poveda (Jun 16 2019 at 09:14, on Zulip):

ummm

Christian Poveda (Jun 16 2019 at 09:14, on Zulip):

well, given that this happened you already know the answer

RalfJ (Jun 16 2019 at 09:15, on Zulip):

ah no that doesnt mean miri gets automatically tested or something

RalfJ (Jun 16 2019 at 09:15, on Zulip):

that's just needed or else test src/tools/miri will fail

Christian Poveda (Jun 16 2019 at 09:15, on Zulip):

oh no I don't

Christian Poveda (Jun 16 2019 at 09:15, on Zulip):

I believe I have that on my other machine

Christian Poveda (Jun 16 2019 at 09:15, on Zulip):

but I'll change that here

Christian Poveda (Jun 16 2019 at 09:17, on Zulip):

Well LLVM is compiling but it will take a while

RalfJ (Jun 16 2019 at 09:20, on Zulip):

yeah those builds are sooooo slooooooooooo....

Christian Poveda (Jun 16 2019 at 09:22, on Zulip):

Well, I'll have breakfast and sleep a little, its like 4 am here

Christian Poveda (Jun 16 2019 at 09:23, on Zulip):

yeah those builds are sooooo slooooooooooo....

I know, I'm thinking on saving money for a Ryzen just to compile rustc faster

Christian Poveda (Jun 16 2019 at 10:03, on Zulip):

hmmm

Christian Poveda (Jun 16 2019 at 10:03, on Zulip):

Got a different error on rustc-llvm

error[E0523]: found two different crates with name `rustc_demangle` that are not distinguished by differing `-C metadata`. This will result in symbol conflicts between the two.
RalfJ (Jun 16 2019 at 10:12, on Zulip):

:(

RalfJ (Jun 16 2019 at 10:12, on Zulip):

my best bet is to randomly delete files called librustc_demangle* in build/

RalfJ (Jun 16 2019 at 10:13, on Zulip):

or do ./x.py clean (which will not delete your LLVM build) and then ./x.py test src/tools/miri and then go to sleep ;)

Christian Poveda (Jun 16 2019 at 13:45, on Zulip):

The thing is that I deleted the whole build directory for that build

Christian Poveda (Jun 16 2019 at 15:54, on Zulip):

Ok I don't know why or how but the rebuild worked :P all tests passed

RalfJ (Jun 16 2019 at 20:56, on Zulip):

nice!

Christian Poveda (Jun 17 2019 at 15:31, on Zulip):

Im building rustc to start doing the miri changes

Christian Poveda (Jun 17 2019 at 15:54, on Zulip):
error[E0107]: wrong number of lifetime arguments: expected 3, found 2
  --> src/lib.rs:78:25
   |
78 | ) -> InterpResult<'tcx, InterpretCx<'mir, 'tcx, Evaluator<'tcx>>> {
   |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected 3 lifetime arguments

After rebasing my rustc this keeps happening

RalfJ (Jun 17 2019 at 16:38, on Zulip):

what's your build invocation?

RalfJ (Jun 17 2019 at 16:38, on Zulip):

miri master works with rustc master as of right now

RalfJ (Jun 17 2019 at 16:38, on Zulip):

this looks like you are using a too-old-miri (again)

RalfJ (Jun 17 2019 at 16:38, on Zulip):

ah no. you are using a too-old-rustc.

Christian Poveda (Jun 17 2019 at 16:39, on Zulip):

cargo build

Christian Poveda (Jun 17 2019 at 16:40, on Zulip):

but it has the override to run my rustc

RalfJ (Jun 17 2019 at 16:42, on Zulip):

well it's not using a new enough one I'm afraid

RalfJ (Jun 17 2019 at 16:44, on Zulip):

did you build all the way to stage 2? is rustc --version plausible?

Christian Poveda (Jun 17 2019 at 16:56, on Zulip):

yes i followed the miri README instructions

RalfJ (Jun 17 2019 at 16:57, on Zulip):

well what can I say, the rustc you compiled/use is clearly not the one you think it should be, and without SSH access to your system I'm afraid I can't help you more than write instructions that, to the best of my knowledge, work when followed exactly.

RalfJ (Jun 17 2019 at 16:58, on Zulip):

looks like you built a too-old-rustc then. or didn't rebuild after updating it. or you are using the wrong linked toolchain.

Christian Poveda (Jun 17 2019 at 17:49, on Zulip):

I'm going to redo it and see what happens

Christian Poveda (Jun 17 2019 at 19:00, on Zulip):

nope, still ran ./miri test and it returned the same error

Christian Poveda (Jun 17 2019 at 19:26, on Zulip):

I deleted my local branch and pulled from my fork to see what happens

Christian Poveda (Jun 17 2019 at 19:31, on Zulip):

is 1e388703c07c6f693d5974b9f8520403a4fc57c0 recent enough?

RalfJ (Jun 17 2019 at 19:41, on Zulip):

does it include 24ddd1615419be89828fb5628e3c14af86c08b01 ?

RalfJ (Jun 17 2019 at 19:41, on Zulip):

and 9606f6fa64926a84d82e3c62dbdc57f5c10f756d ?

Christian Poveda (Jun 17 2019 at 19:45, on Zulip):

well, git show shows both of them

Christian Poveda (Jun 17 2019 at 20:05, on Zulip):

but I'm not sure which one happened before

Christian Poveda (Jun 17 2019 at 20:09, on Zulip):

I rebased and I'm building again

RalfJ (Jun 17 2019 at 20:49, on Zulip):

git show $SHA showing them just means that they are somewhere in your local clone, but does not say anything about them being in your branch

RalfJ (Jun 17 2019 at 20:50, on Zulip):

git log without extra arguments would have to show them

RalfJ (Jun 17 2019 at 20:50, on Zulip):

(that's pager output, so you can search it by typing / and then what you are searching for)

Christian Poveda (Jun 17 2019 at 20:50, on Zulip):

i found the first but not the second

Christian Poveda (Jun 17 2019 at 20:51, on Zulip):

after rebasing both are there

Christian Poveda (Jun 17 2019 at 20:56, on Zulip):

yay I'ts merged: https://github.com/rust-lang/rust/pull/61781

Christian Poveda (Jun 17 2019 at 22:24, on Zulip):

ok after all of this I think I was able to make this work

Christian Poveda (Jun 17 2019 at 22:24, on Zulip):

so... tests

Christian Poveda (Jun 18 2019 at 00:13, on Zulip):

one testcase should do something like ... as *const i32 as usize as u8 as usize and make sure the value got truncated

I did this in a test but its still failing when doing the as u8 cast, is that ok?

Christian Poveda (Jun 18 2019 at 01:37, on Zulip):

Given that there is no implementation inside mir for the new Machine methods i suppose the behaviour should be the same, right?

Christian Poveda (Jun 18 2019 at 04:45, on Zulip):

mir -> miri*

oli (Jun 18 2019 at 10:21, on Zulip):

yes, that is expected

Christian Poveda (Jun 18 2019 at 12:46, on Zulip):

So what's the next step?

oli (Jun 18 2019 at 12:51, on Zulip):

overwrite the force* methods on the machine and start experimenting with converting pointers to ints and back in the way I described on the issue

oli (Jun 18 2019 at 12:51, on Zulip):

Though the conversion should be skipped if we don't have a random seed

oli (Jun 18 2019 at 12:52, on Zulip):

without a random seed, we are in deterministic mode and should just bail out like we do right now

Christian Poveda (Jun 18 2019 at 12:56, on Zulip):

How does the miri project handle tests that should fail in deterministic mode only?

oli (Jun 18 2019 at 12:58, on Zulip):

I think we have an example test using rand

oli (Jun 18 2019 at 12:58, on Zulip):

it should have a flag specified somewhere

Christian Poveda (Jun 18 2019 at 12:58, on Zulip):

Ok I'll look it up

oli (Jun 18 2019 at 13:03, on Zulip):

if there are no tests, we should create some XD

Christian Poveda (Jun 18 2019 at 13:13, on Zulip):

I'll check those in an hour, gotta get ready for work :P

RalfJ (Jun 18 2019 at 13:13, on Zulip):

@Christian Poveda the new tests should have // compile-flags: -Zmiri-seed=4242

RalfJ (Jun 18 2019 at 13:13, on Zulip):

that will enable nondeterministic mode

RalfJ (Jun 18 2019 at 13:13, on Zulip):

the existing tests dont have that flag and hence will still fail

Jake Goulding (Jun 18 2019 at 14:00, on Zulip):

Heh, deterministic nondeterminism

RalfJ (Jun 18 2019 at 14:01, on Zulip):

the best kind :)

oli (Jun 18 2019 at 14:01, on Zulip):

the best kind of nondeterminism

oli (Jun 18 2019 at 14:01, on Zulip):

lol

Mahmut Bulut (Jun 18 2019 at 14:13, on Zulip):

Edward Lorenz kind of determinism.

Christian Poveda (Jun 18 2019 at 16:33, on Zulip):

I'm doing ptr_to_int

Christian Poveda (Jun 18 2019 at 16:35, on Zulip):

when there is no entry in the hashmap for the ptr, what u64 should I use? in other words, what is name_of_the_u64 in @oli instructions?

Christian Poveda (Jun 18 2019 at 16:43, on Zulip):

also in memory.get(id).bytes.len(), do you mean memory: Memory or the hashmap? because memory: Memory is not accesible from ptr_to_int, just MemoryExtra.

Christian Poveda (Jun 18 2019 at 18:43, on Zulip):

when there is no entry in the hashmap for the ptr, what u64 should I use? in other words, what is name_of_the_u64 in oli instructions?

should It be the u64 inside AllocId?

eddyb (Jun 18 2019 at 18:46, on Zulip):

just saw this and I have to say, ptr_to_int and int_to_ptr seem quite confusing :P

eddyb (Jun 18 2019 at 18:49, on Zulip):

but then again I'm not sure I have any better suggestions :(

eddyb (Jun 18 2019 at 18:49, on Zulip):

(but they seem ambiguous with language-level usize <-> *mut T casts)

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

just saw this and I have to say, ptr_to_int and int_to_ptr seem quite confusing :P

I believe @RalfJ has stronger naming opinions than me. I'm happy with whatever name you want to use :P

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

an operation called int_to_ptr IMO should have type int-to-ptr, or else something is off ;)

This :P

RalfJ (Jun 18 2019 at 19:30, on Zulip):

well it has type u128 -> Pointer, does it not?

RalfJ (Jun 18 2019 at 19:30, on Zulip):

oh you were quoting me?^^

RalfJ (Jun 18 2019 at 19:30, on Zulip):

@eddyb well in some sense this is about the casts

eddyb (Jun 18 2019 at 19:31, on Zulip):

right but "int" in the sense of "raw bits" not "surface language integer types"

eddyb (Jun 18 2019 at 19:31, on Zulip):

similar with "ptr" - miri's Pointer, not merely a pointer type

Christian Poveda (Jun 18 2019 at 19:34, on Zulip):

if you want to discuss the implementation also I`ll be happy to join that discussion :P

RalfJ (Jun 18 2019 at 19:36, on Zulip):

well the part about int/ptr values vs type is confusing everywhere. not sure what to do about it.

oli (Jun 19 2019 at 06:38, on Zulip):

@Christian Poveda I think essentially we're going to need a Vec<(u64, AllocId)> for the int -> ptr direction. What that Vec would contain would be the base addresses of the AllocId that went through a ptr -> int transformation. You'll need to do a binary search on that for finding the u64 in the Vec that is the largest one that is smaller or equal to the u64 that you are searching for

oli (Jun 19 2019 at 06:39, on Zulip):

the problem is that e.g. a user could do a ptr -> int transformation for a pointer to the 5th element of an array

Christian Poveda (Jun 19 2019 at 06:39, on Zulip):

yeah I got that part

oli (Jun 19 2019 at 06:39, on Zulip):

oh

oli (Jun 19 2019 at 06:39, on Zulip):

you mean ptr to int

Christian Poveda (Jun 19 2019 at 06:39, on Zulip):

the real question is what u64 should be

oli (Jun 19 2019 at 06:39, on Zulip):

sorry

oli (Jun 19 2019 at 06:40, on Zulip):

so as a start, create a u64 field in the ExtraData of the InterpCx that denotes how many bytes have ever been part of an ptr -> int transformation

oli (Jun 19 2019 at 06:41, on Zulip):

so the first time anything is transformed, that value is 2^16 or sth (don't start with 0) and thus the u64 is 2^16. Then you add the Allocation's byte size to the ExtraData field

Christian Poveda (Jun 19 2019 at 06:41, on Zulip):

so as a start, create a u64 field in the ExtraData of the InterpCx that denotes how many bytes have ever been part of an ptr -> int transformation

I think that should go in MemoryExtra, given that none of the methods can access InterpCx directrly

oli (Jun 19 2019 at 06:41, on Zulip):

the next transformation will thus have its allocation directly next to it

oli (Jun 19 2019 at 06:42, on Zulip):

right

oli (Jun 19 2019 at 06:42, on Zulip):

MemoryExtra is the right thing

Christian Poveda (Jun 19 2019 at 06:42, on Zulip):

ohhh now I get it

Christian Poveda (Jun 19 2019 at 06:42, on Zulip):

so this u64 just keeps growing simulating the effect of allocating more and more stuff in memory

oli (Jun 19 2019 at 06:43, on Zulip):

jop

oli (Jun 19 2019 at 06:43, on Zulip):

in the future we can handle deallocations, but for now, let's do the easy thing

Christian Poveda (Jun 19 2019 at 06:43, on Zulip):

maybe this is a little too far, but what should happen when the u64 overflows?

oli (Jun 19 2019 at 06:44, on Zulip):

use checked_add and report an Unimplemented error xD

Christian Poveda (Jun 19 2019 at 06:44, on Zulip):

hahaha

Christian Poveda (Jun 19 2019 at 06:45, on Zulip):

well I believe that's enough to get me started tomorrow (now + 8h)

oli (Jun 19 2019 at 06:45, on Zulip):

cool!

Christian Poveda (Jun 19 2019 at 06:45, on Zulip):

see you at 17:00 then :P

oli (Jun 19 2019 at 06:45, on Zulip):

jup :D

RalfJ (Jun 19 2019 at 10:06, on Zulip):

so the first time anything is transformed, that value is 2^16 or sth (don't start with 0) and thus the u64 is 2^16. Then you add the Allocation's byte size to the ExtraData field

I don't get the point of this?

RalfJ (Jun 19 2019 at 10:07, on Zulip):

is this for picking a base address? those should be non-deterministic, that's why this depends on the seed

RalfJ (Jun 19 2019 at 10:07, on Zulip):

so IMO you should ask the RNG for a random base address until you find one that works

RalfJ (Jun 19 2019 at 10:07, on Zulip):

and give up after 100 attempts or so^^

RalfJ (Jun 19 2019 at 10:08, on Zulip):

if we pick a fixed base address like 2^16, we might as well do this in all executions as everything is deterministic^^

oli (Jun 19 2019 at 10:59, on Zulip):

we can improve this algorithm at any time, I just chose the simplest one to start with

oli (Jun 19 2019 at 11:00, on Zulip):

We need to do something like you suggested anyway in order to be able to handle deallocations

oli (Jun 19 2019 at 11:00, on Zulip):

but for now we can skip any complex logic and just count up deterministically

RalfJ (Jun 19 2019 at 11:20, on Zulip):

seems funny that we make a deterministic part of the execution depend on the "nondeterminism" flag, but whatever ;)

RalfJ (Jun 19 2019 at 11:20, on Zulip):

fine for me

oli (Jun 19 2019 at 11:36, on Zulip):

hmm

oli (Jun 19 2019 at 11:36, on Zulip):

like we could also choose a random default for the nondeterminism flag and thus be deterministic

oli (Jun 19 2019 at 11:37, on Zulip):

I thought that the nondeterminsm flag is for things that aren't deterministic on hardware

RalfJ (Jun 19 2019 at 12:17, on Zulip):

miri has nothing to do with hardware :P

RalfJ (Jun 19 2019 at 12:17, on Zulip):

the goal is to implement the MIR spec. there is no MIR spec but most of the time we can imagine what it would be. and for allocation it'd certainly be non-deterministic.

oli (Jun 19 2019 at 12:37, on Zulip):

exactly, and that's why I'd want it behind the flag even if the actual implementation isn't non-deterministic yet

RalfJ (Jun 19 2019 at 12:40, on Zulip):

oh absolutely. my surprise it not about the flag but about the lack of actual non-determinism. ;)

oli (Jun 19 2019 at 12:43, on Zulip):

yea, I should have communicated better that this is the first step implementation. I just didn't want the PR to get bigger than it will already get

Christian Poveda (Jun 19 2019 at 15:58, on Zulip):

I'll leave a comment stating the fact that we need to pick the starting address randomly in the future.

Christian Poveda (Jun 19 2019 at 18:57, on Zulip):

I have an small issue, in order to shift the u64, I need to get the size of the allocation, however I'm not sure how to do it because inside int_to_ptr and ptr_to_int I have no access to Memory itself, just to MemoryExtra

Christian Poveda (Jun 19 2019 at 20:08, on Zulip):

I have an small issue, in order to shift the u64, I need to get the size of the allocation, however I'm not sure how to do it because inside int_to_ptr and ptr_to_int I have no access to Memory itself, just to MemoryExtra

Should I change the machine trait to take Memory or perhaps InterpCx instead of MemoryExtra?

RalfJ (Jun 19 2019 at 20:45, on Zulip):

InterpCx won't be possible because you need to call this in Memory methods... but you can change it to Memory

RalfJ (Jun 19 2019 at 20:45, on Zulip):

then please also change the other machine hooks that take MemoryExtra, for consisteny

Christian Poveda (Jun 19 2019 at 21:12, on Zulip):

InterpCx won't be possible because you need to call this in Memory methods... but you can change it to Memory

oh of course, i forget about the Memory ones. I'm going to do the changes in my local rustc to see if my miri changes work. I assume I should wrap the new MemoryExtra in miri like the stacked_borrows: Rc<RefCell<_>>

RalfJ (Jun 19 2019 at 21:14, on Zulip):

well see if you can pass &mut Memory to int_to_ptr/ptr_to_int

RalfJ (Jun 19 2019 at 21:14, on Zulip):

if yes you need no interior mutability

RalfJ (Jun 19 2019 at 21:14, on Zulip):

I need that for Stacked Borrows because some of the relevant machine hooks can only get &Memory/&MemoryExtra

Christian Poveda (Jun 19 2019 at 21:14, on Zulip):

but the thing is that would force the usages of force_* to have a mutable pointer of Memory

RalfJ (Jun 19 2019 at 21:15, on Zulip):

yes

RalfJ (Jun 19 2019 at 21:15, on Zulip):

so if they dont have a mutable memory you cant do that

RalfJ (Jun 19 2019 at 21:15, on Zulip):

thats why I said see if you can do it :)

Christian Poveda (Jun 19 2019 at 21:16, on Zulip):

yeah I tried and saw that, but I think I'll go for the Rc

RalfJ (Jun 19 2019 at 21:17, on Zulip):

you should need Rc even then

RalfJ (Jun 19 2019 at 21:17, on Zulip):

just RefCell

RalfJ (Jun 19 2019 at 21:17, on Zulip):

I need Rc because I also need all allocations to share this pointer

RalfJ (Jun 19 2019 at 21:17, on Zulip):

but you wont need that

Christian Poveda (Jun 19 2019 at 21:17, on Zulip):

Ohh ok

Christian Poveda (Jun 19 2019 at 21:57, on Zulip):

I believe ptr_to_int is working, I'm going to do int_to_ptr now

Christian Poveda (Jun 19 2019 at 22:23, on Zulip):

Christian Poveda I think essentially we're going to need a Vec<(u64, AllocId)> for the int -> ptr direction. What that Vec would contain would be the base addresses of the AllocId that went through a ptr -> int transformation. You'll need to do a binary search on that for finding the u64 in the Vec that is the largest one that is smaller or equal to the u64 that you are searching for

So this means that the binary search would give me the "greater lower bound" for the one I'm searching for, right?. What should be the offset for the pointer here? And what should happen when the greater lower bound is not equal to the u64 I'm looking for? What should be the AllocId in this case?

RalfJ (Jun 20 2019 at 07:18, on Zulip):

if you get the int 64 and find a "greatest lower bound" of 60, then you create a pointer with offset 4

RalfJ (Jun 20 2019 at 07:18, on Zulip):

except you should make sure that this is still "inbounds" the allocation

Christian Poveda (Jun 20 2019 at 12:51, on Zulip):

I suppose that means to check that the Int is smaller than the latest address used

RalfJ (Jun 20 2019 at 13:24, on Zulip):

well once you got the base address you compute the offset

RalfJ (Jun 20 2019 at 13:25, on Zulip):

and then you check offset <= alloc.size

RalfJ (Jun 20 2019 at 13:25, on Zulip):

note the =! this is the "one-past-the-end" rule.

Christian Poveda (Jun 20 2019 at 13:45, on Zulip):

At least in our current state of affairs

RalfJ (Jun 20 2019 at 13:57, on Zulip):

no that should be the final state, this is how int-ptr-casts work

RalfJ (Jun 20 2019 at 13:57, on Zulip):

which part of this int_to_ptr do you think is temporary?

Christian Poveda (Jun 20 2019 at 14:00, on Zulip):

what do you mean?

RalfJ (Jun 20 2019 at 14:05, on Zulip):

you said "At least in our current state of affairs". what did you refer to and what did you mean?

Christian Poveda (Jun 20 2019 at 14:10, on Zulip):

we can improve this algorithm at any time, I just chose the simplest one to start with

Well I assume that simulating the memory allocations could eventually have a more sophisticated model

Christian Poveda (Jun 20 2019 at 14:13, on Zulip):

and then you check offset <= alloc.size

So basically its a: "This thing fits here"

Christian Poveda (Jun 20 2019 at 14:15, on Zulip):

no that should be the final state, this is how int-ptr-casts work

I shouldn't have skimmed the paper

RalfJ (Jun 20 2019 at 14:18, on Zulip):

Well I assume that simulating the memory allocations could eventually have a more sophisticated model

I think the u64-AllocId mapping, plus a base_address field in AllocationExtra for the other direction, (do you have that yet?) is a reasonable representation. and on the int-to-ptr side, there is not much choice, so whatever you are doing now (and what I described above) is the final version.

RalfJ (Jun 20 2019 at 14:18, on Zulip):

the one place where we are choosing a preliminary option now is for how to allocate the offsets in ptr-to-int.

Christian Poveda (Jun 20 2019 at 14:25, on Zulip):

I think the u64-AllocId mapping, plus a base_address field in AllocationExtra for the other direction, (do you have that yet?) is a reasonable representation.

Yes I wanted to do a single commit with both sides, I added the base_address as a field in MemoryExtra, should I move it to AllocationExtra?

Christian Poveda (Jun 20 2019 at 14:25, on Zulip):

so whatever you are doing now (and what I described above) is the final version.

Damn, there is no salvation from the git blame, this time

oli (Jun 20 2019 at 14:26, on Zulip):

yea, git should totally mention the reviewer in git blame, too ;)

RalfJ (Jun 20 2019 at 14:27, on Zulip):

well you have a per-allocation base_address, so AllocExtra makes more sense IMO

Christian Poveda (Jun 20 2019 at 14:32, on Zulip):

just to be sure, the base_address is the integer that gets modified when doing ptr_to_int, i.e. Oliver's 2^16, right?

oli (Jun 20 2019 at 14:33, on Zulip):

no

oli (Jun 20 2019 at 14:33, on Zulip):

@RalfJ wants to move the ptr -> int mapping into the Allocation

oli (Jun 20 2019 at 14:33, on Zulip):

which I support

oli (Jun 20 2019 at 14:34, on Zulip):

so we have one HashMap less

Christian Poveda (Jun 20 2019 at 14:34, on Zulip):

Wow wow, wait

oli (Jun 20 2019 at 14:34, on Zulip):

if you want to map Pointer -> int, you fetch the int via memory.get(ptr.alloc_id)?.base_address + ptr.offset

oli (Jun 20 2019 at 14:35, on Zulip):

At least that's what I understood?

Christian Poveda (Jun 20 2019 at 14:37, on Zulip):

if you want to map Pointer -> int, you fetch the int via memory.get(ptr.alloc_id)?.base_address + ptr.offset

then wouldnt we need the Vec for the int_to_ptr only and forget the HashMap? Is that what you mean by having one HashMap less?

oli (Jun 20 2019 at 14:39, on Zulip):

yes

Christian Poveda (Jun 20 2019 at 14:41, on Zulip):

Well If @RalfJ agrees I'll do the change to ptr_to_int after fixing int_to_ptr

RalfJ (Jun 20 2019 at 14:42, on Zulip):

yes think we'll end up with a (sorted!) Vec, and an entry in AllocExtra, and no HashMap

RalfJ (Jun 20 2019 at 14:43, on Zulip):

the Vec is only queried in int-to-ptr, but of course ptr-to-int has to fill the Vec with the right information to make it match the base address stored in AllocExtra

Christian Poveda (Jun 20 2019 at 14:48, on Zulip):

yep, understood. I'll come back in a few minutes with a bunch of errors

Christian Poveda (Jun 20 2019 at 14:52, on Zulip):

Taking a few steps back

if you get the int 64 and find a "greatest lower bound" of 60, then you create a pointer with offset 4

If we had a perfect match the offset would be zero following this idea. am I correctly assuming that this is the case when we have ZSTs?

oli (Jun 20 2019 at 14:54, on Zulip):

pointer offset is unrelated to the actual types that are pointed at

oli (Jun 20 2019 at 14:54, on Zulip):

basically a perfect match means your pointer points into the beginning of an Allocation

Christian Poveda (Jun 20 2019 at 14:54, on Zulip):

oh so the offset is just the offset from inside the pointed value

oli (Jun 20 2019 at 14:55, on Zulip):

if you get 64 from Pointer(AllocId(something), 4), then Pointer(AllocId(something), 0) gives you 60

oli (Jun 20 2019 at 14:55, on Zulip):

yes

Christian Poveda (Jun 20 2019 at 14:55, on Zulip):

which would be zero for most basic types, just contiguous stuff like arrays, structs, tuples and so on would actually require to offset to access them

oli (Jun 20 2019 at 14:56, on Zulip):

yes, but you may have a pointer to a basic type, and that pointer may be into a field of a struct

oli (Jun 20 2019 at 14:57, on Zulip):

or the user may have allocated random bytes on the heap and placed the value somewhere in there

oli (Jun 20 2019 at 14:57, on Zulip):

e.g. Rc<u32> would never give you a zero offset to the u32

oli (Jun 20 2019 at 14:57, on Zulip):

because of the leading reference count

Christian Poveda (Jun 20 2019 at 14:58, on Zulip):

I'm slowly understanding whats going on, thank you

oli (Jun 20 2019 at 15:00, on Zulip):

it's a confusing system to get into. I remember the madness at the beginning. We even had special ZST allocations and some other tricks like that

Christian Poveda (Jun 20 2019 at 15:00, on Zulip):

heheh

oli (Jun 20 2019 at 15:00, on Zulip):

but it's very consistent now, just not obvious from the start I guess

Christian Poveda (Jun 20 2019 at 15:03, on Zulip):

ok now I'm in the part where there is no perfect match after doing a binary search. Checking the docs, I have the position (lets call it pos) where the integer would be inserted into the Vec to keep it sorted. If i understand correctly, vec[pos - 1] would be our greatest lower bound, but what should happen when the Vec is empty? Should this be an error given that we are trying to transform an integer to a pointer that doesn't even exists?

oli (Jun 20 2019 at 15:04, on Zulip):

yea, that sounds reasonable

RalfJ (Jun 20 2019 at 15:07, on Zulip):

alternatively we could make the Vec never empty

RalfJ (Jun 20 2019 at 15:07, on Zulip):

if we fill it initially with 2^16 we'd fall back to that

RalfJ (Jun 20 2019 at 15:07, on Zulip):

but, hm, we'd have to map it to an AllocId...

RalfJ (Jun 20 2019 at 15:08, on Zulip):

also you'll have to handle pos == 0 anyway

Christian Poveda (Jun 20 2019 at 15:15, on Zulip):

what about the tag? I assume these pointers should be untagged given that we are doing crazy raw pointer shenaningans anyway

RalfJ (Jun 20 2019 at 15:17, on Zulip):

yes. in fact int-to-ptr-tags is exactly what Untagged exists for :)

Christian Poveda (Jun 20 2019 at 15:17, on Zulip):

Man, two hits in a row, I'm on fire this morning

RalfJ (Jun 20 2019 at 15:17, on Zulip):

:fire:

RalfJ (Jun 20 2019 at 15:17, on Zulip):

or more like :fireworks: ?

Christian Poveda (Jun 20 2019 at 15:22, on Zulip):

we'll see, if travis doesn't get caught in the fire we'll decide

Christian Poveda (Jun 20 2019 at 15:23, on Zulip):

ok time to fix the other way

Christian Poveda (Jun 20 2019 at 15:32, on Zulip):

and then you check offset <= alloc.size

does this alloc come from Memory?

RalfJ (Jun 20 2019 at 15:35, on Zulip):

yeah that#s where we typically have our allocations :)

Christian Poveda (Jun 20 2019 at 15:36, on Zulip):

3 in a row, this is going to fail in CI

Christian Poveda (Jun 20 2019 at 15:36, on Zulip):

ok now it is time to ditch the HashMap

Christian Poveda (Jun 20 2019 at 15:43, on Zulip):

I think the u64-AllocId mapping, plus a base_address field in AllocationExtra for the other direction, (do you have that yet?) is a reasonable representation. and on the int-to-ptr side, there is not much choice, so whatever you are doing now (and what I described above) is the final version.

So the AllocationExtra here is stacked_borrows::Stacks, should I put it there?

Christian Poveda (Jun 20 2019 at 15:47, on Zulip):

or given that Stacks has the GlobalState, should I I put it there instead?
Edit: This doesn't make sense, GlobalState is global after all

Christian Poveda (Jun 20 2019 at 16:04, on Zulip):

Ok the fire is over.

Christian Poveda (Jun 20 2019 at 16:06, on Zulip):

When does the base_address field acquires its value?

Christian Poveda (Jun 20 2019 at 16:15, on Zulip):

When does the base_address field acquires its value?

Moreover, this means that not only the HashMap gets removed, the u64 inside MemoryExtra (the one with the initial 2^16 value) should be removed too.

Christian Poveda (Jun 20 2019 at 16:46, on Zulip):

Ohhhh I think I got it. So when Stacks is created, we get the base_address using the u64 inside MemoryExtra with all the acrobatics of adding the size and stuff. Then inside ptr_to_int we just recover it

Christian Poveda (Jun 20 2019 at 17:48, on Zulip):

Ohhhh I think I got it. So when Stacks is created, we get the base_address using the u64 inside MemoryExtra with all the acrobatics of adding the size and stuff. Then inside ptr_to_int we just recover it

If this is right I believe I'm done with the implementation inside miri

Christian Poveda (Jun 20 2019 at 17:50, on Zulip):

Probably is not because the *std::ptr::null()test is behaving unexpectedly
Edit: Ok, the problem is that the tests are returning a different error, I can fix that later

Christian Poveda (Jun 20 2019 at 17:58, on Zulip):

Ok none of the tests broke (after fixing the null ptr ones). So, now what about new tests?

Christian Poveda (Jun 20 2019 at 18:12, on Zulip):

I believe there are some cases that I'm handling wrong because &42 as *const i32 as usize as u8 as usize; still fails

Christian Poveda (Jun 20 2019 at 20:47, on Zulip):

At least in our current state of affairs

oli (Jun 21 2019 at 07:19, on Zulip):

@Christian Poveda I'm not sure if you are still unsure about anything :D (except that tests are still failing)

oli (Jun 21 2019 at 07:20, on Zulip):

you can try MIRI_BACKTRACE=1 to see where the error is reported from in the code

Christian Poveda (Jun 21 2019 at 12:34, on Zulip):

At least in our current state of affairs

Christian Poveda (Jun 21 2019 at 12:35, on Zulip):

Well I have to land the other PRs before submitting this one but I'll check it

oli (Jun 21 2019 at 12:36, on Zulip):

we landed a bunch of your stuff a few hours ago

Christian Poveda (Jun 21 2019 at 12:53, on Zulip):

I just saw it hehehe

Christian Poveda (Jun 21 2019 at 12:54, on Zulip):

Is everything OK with the python scripts?

oli (Jun 21 2019 at 12:54, on Zulip):

just some issues not opening autmatically

oli (Jun 21 2019 at 12:54, on Zulip):

I think I saw a PR where @RalfJ was on it

RalfJ (Jun 21 2019 at 12:55, on Zulip):

well, kindof. I found the error message. but I still have no idea why it is failing.

RalfJ (Jun 21 2019 at 12:56, on Zulip):

@Christian Poveda you don't have to worry about this though, that's me screwing up an unrelated PR. ;)

Christian Poveda (Jun 21 2019 at 12:59, on Zulip):

Mama, just lost a branch. Did git rebase against its head. I pressed enter, now it’s dead

oli (Jun 21 2019 at 13:03, on Zulip):

@Christian Poveda need any help recovering it?

Christian Poveda (Jun 21 2019 at 13:21, on Zulip):

Nah no problems I was remembering the first "preparing for intptrcast" PR

oli (Jun 21 2019 at 13:22, on Zulip):

heh

oli (Jun 21 2019 at 13:22, on Zulip):

ususally you can recover everything

RalfJ (Jun 21 2019 at 14:07, on Zulip):

if the rebase is still ongoing, do git rebase --abort

RalfJ (Jun 21 2019 at 14:07, on Zulip):

else git reflog should at least show every commit you ever created with git commit, and then you can use the SHA1 you get there to recover

RalfJ (Jun 21 2019 at 14:08, on Zulip):

git rebase is very easy to get wrong, I've been there several times as well :/

oli (Jun 21 2019 at 14:11, on Zulip):

the most common failure I've seen is to choose the wrong thing to rebase over XD So often it's that one rebases over the own fork's master instead of the upstream master

Christian Poveda (Jun 21 2019 at 14:22, on Zulip):

my usual problem is that i squash the wrong commits D:

RalfJ (Jun 21 2019 at 14:23, on Zulip):

my usual problem is that I do git commit --amend when I should have done git add . && git rebase --continue

RalfJ (Jun 21 2019 at 14:24, on Zulip):

as in, I accidentally amend the previous commit after fixing conflicts during a rebase

Christian Poveda (Jun 21 2019 at 14:24, on Zulip):

Bad git practices: The const-eval definitive guide

Christian Poveda (Jun 21 2019 at 14:24, on Zulip):

I'm amazed how easily things can go wrong with git

RalfJ (Jun 21 2019 at 14:25, on Zulip):

it doesn't help that when doing rebase with some commtis marked to edit, you have to kind of remember if you are currently in "conflict resolution" or "editing" state, and hence have to use git add . && git rebase --coninue or git commit -a --amend :/

Christian Poveda (Jun 21 2019 at 14:25, on Zulip):

I don't know, maybe one can change the $PS1 to reflect that state or something

RalfJ (Jun 21 2019 at 14:25, on Zulip):

I love the git data model. but much of its UI... ugh

Christian Poveda (Jun 21 2019 at 14:27, on Zulip):

hehehe

Christian Poveda (Jun 21 2019 at 14:27, on Zulip):

Well I'd like to discuss some errors with you

Christian Poveda (Jun 21 2019 at 14:27, on Zulip):

do you remember that we are checking if offset <= alloc_size during int_to_ptr?

Christian Poveda (Jun 21 2019 at 14:29, on Zulip):

What should be the appropriate error when the offset is larger than the size of the allocation?

oli (Jun 21 2019 at 14:31, on Zulip):

that can't happen unless it's the last allocation

oli (Jun 21 2019 at 14:32, on Zulip):

and in that case it doesn't matter, because you'll just have an invalid pointer into that allocation and thus will get an OOB error when you try to access it

oli (Jun 21 2019 at 14:32, on Zulip):

oh

oli (Jun 21 2019 at 14:33, on Zulip):

but that would be problematic if someone comes along later and creates a ptr_to_int that actually overlaps that region

oli (Jun 21 2019 at 14:33, on Zulip):

so the pointer would suddenly be legal

Christian Poveda (Jun 21 2019 at 14:35, on Zulip):

I was thinking something like PointerOutOfBounds or InvalidMemoryAccess

oli (Jun 21 2019 at 14:36, on Zulip):

maybe just Unimplemented for now, I'm not sure about the expected semantics

RalfJ (Jun 21 2019 at 14:37, on Zulip):

sounds like a new error to me

RalfJ (Jun 21 2019 at 14:37, on Zulip):

invalid int-to-ptr-cast

oli (Jun 21 2019 at 14:37, on Zulip):

the question is whether it should be an error?

RalfJ (Jun 21 2019 at 14:37, on Zulip):

for an address that was never returned by a ptr-to-int-cast

RalfJ (Jun 21 2019 at 14:37, on Zulip):

yes!

RalfJ (Jun 21 2019 at 14:37, on Zulip):

hm

oli (Jun 21 2019 at 14:37, on Zulip):

ah :D

RalfJ (Jun 21 2019 at 14:37, on Zulip):

yes

oli (Jun 21 2019 at 14:37, on Zulip):

ok, well then, new error it is

RalfJ (Jun 21 2019 at 14:37, on Zulip):

we only do force_ptr if we really need a ptr

RalfJ (Jun 21 2019 at 14:37, on Zulip):

so if we cant do the conversion, we have a dangling pointer

oli (Jun 21 2019 at 14:37, on Zulip):

right

RalfJ (Jun 21 2019 at 14:38, on Zulip):

and it should be errored as such

Christian Poveda (Jun 21 2019 at 14:40, on Zulip):

That would apply for every error in both methods?

Christian Poveda (Jun 21 2019 at 14:41, on Zulip):

because in int_to_ptr there is also the problem when the index returned by the binary search is zero. Meaning that there is no address smaller than the current integer

oli (Jun 21 2019 at 14:41, on Zulip):

what other errors do you have?

oli (Jun 21 2019 at 14:42, on Zulip):

yes, that's the same error as being OOB of the last allocation

Christian Poveda (Jun 21 2019 at 14:43, on Zulip):

the last error is in ptr_to_int when adding the alloc_id to our vec if there is a base_address with the same alloc_id already stored

Christian Poveda (Jun 21 2019 at 14:44, on Zulip):

I believe that one is not an error, because it would happen everytime you call ptr_to_int with the same int I think

RalfJ (Jun 21 2019 at 14:45, on Zulip):

you should only do the allocation thing in ptr_to_int if there is no base address stored in the AllocExtra yet

RalfJ (Jun 21 2019 at 14:45, on Zulip):

otherwise just re-use what is already in the table(s)

Christian Poveda (Jun 21 2019 at 14:46, on Zulip):

you should only do the allocation thing in ptr_to_int if there is no base address stored in the AllocExtra yet

That could happen? I'm initializing the AllocExtra with a base address

oli (Jun 21 2019 at 14:47, on Zulip):

hold on

Christian Poveda (Jun 21 2019 at 14:47, on Zulip):

o.o

oli (Jun 21 2019 at 14:47, on Zulip):

can you push your PR :D I think we're diverging our mental models

oli (Jun 21 2019 at 14:47, on Zulip):

not by much, but by a relevant point

Christian Poveda (Jun 21 2019 at 14:48, on Zulip):

we probably are, I'm going to let the errors as unimplemented and PR then

oli (Jun 21 2019 at 14:48, on Zulip):

I assumed there was an Cell<Option<u64>> in the AllocExtra

Christian Poveda (Jun 21 2019 at 14:53, on Zulip):

https://github.com/rust-lang/miri/pull/779

Christian Poveda (Jun 21 2019 at 14:55, on Zulip):

I assumed there was an Cell<Option<u64>> in the AllocExtra

I thought about Option<u64> but then believed it was better to set the value during Stacks::new

Christian Poveda (Jun 21 2019 at 14:59, on Zulip):

What should be the difference here?

oli (Jun 21 2019 at 15:00, on Zulip):

it should not be done eagerly

oli (Jun 21 2019 at 15:00, on Zulip):

you are doing it eagerly now

oli (Jun 21 2019 at 15:00, on Zulip):

The idea was to do it in ptr_to_int

Christian Poveda (Jun 21 2019 at 15:00, on Zulip):

Oh of course, I remember @RalfJ 's insistence on the lazy part

Christian Poveda (Jun 21 2019 at 15:00, on Zulip):

going to fix it

oli (Jun 21 2019 at 15:00, on Zulip):

hehe

oli (Jun 21 2019 at 15:00, on Zulip):

wait a sec

oli (Jun 21 2019 at 15:00, on Zulip):

you also kinda infected stacked borrows

oli (Jun 21 2019 at 15:01, on Zulip):

I have to read the source for a sec

oli (Jun 21 2019 at 15:01, on Zulip):

I haven't touched miri in too long

Christian Poveda (Jun 21 2019 at 15:01, on Zulip):

well I had to, but after removing the initalization of the base address that should be reverted

oli (Jun 21 2019 at 15:02, on Zulip):

I think you should do what you did for the MemoryState and create a new struct that holds what you need and whatever stacked borrows need

Christian Poveda (Jun 21 2019 at 15:02, on Zulip):

what do you mean?

oli (Jun 21 2019 at 15:03, on Zulip):

you can make https://github.com/rust-lang/miri/blob/285e9a65cd61520a4da1f912d1b41e523cd3a5e4/src/lib.rs#L390 point to other types

Christian Poveda (Jun 21 2019 at 15:03, on Zulip):

like adding an struct that contains Stacks and the base_addr?

oli (Jun 21 2019 at 15:03, on Zulip):

yes

Christian Poveda (Jun 21 2019 at 15:03, on Zulip):

Will do it then

Christian Poveda (Jun 21 2019 at 15:09, on Zulip):

Did you see the Default implementation inside intptrcast.rs?

Christian Poveda (Jun 21 2019 at 15:10, on Zulip):

I'm forcing the u64 to be 2.pow(16) there, should I also use Option<u64> and do that somewhere else?

oli (Jun 21 2019 at 15:11, on Zulip):

no that's perfectly alright

oli (Jun 21 2019 at 15:11, on Zulip):

that's the starting value

Christian Poveda (Jun 21 2019 at 15:11, on Zulip):

ok

oli (Jun 21 2019 at 15:12, on Zulip):

the reason I wanted Cell<Option<u64>> inside Allocations is that we can start with None and only fill it in in ptr_to_int

oli (Jun 21 2019 at 15:12, on Zulip):

so when you ptr_to_int an Allocation with None in its base_address field, you do the logic you did eagerly in Stacks

Christian Poveda (Jun 21 2019 at 15:13, on Zulip):

Ja, but I was thinking about @RalfJ 's comments about changing the starting value randomly

Christian Poveda (Jun 21 2019 at 15:13, on Zulip):

oh I remembered something else

Christian Poveda (Jun 21 2019 at 15:14, on Zulip):

how can I access the random seed from Memory? because int_to_ptr and ptr_to_int are not instance methods

oli (Jun 21 2019 at 15:16, on Zulip):

oh, the starting value won't randomly change, it will be initialized with a random value once when ptr_to_int needs to set that value

oli (Jun 21 2019 at 15:16, on Zulip):

oh, we may have to move the random seed to the MemoryExtra struct that already contains Stacks and base_addr

oli (Jun 21 2019 at 15:17, on Zulip):

(also maybe name base_addr as next_base_addr to reduce confusion with the base_addr field in AllocExtra)

Christian Poveda (Jun 21 2019 at 15:17, on Zulip):

maybe i should derive the Default and just set the 2.pow(16) value inside the intptrcast methods when needed?
Edit: nah forget it

oli (Jun 21 2019 at 15:17, on Zulip):

no, the custom Default impl is a good idea

Christian Poveda (Jun 21 2019 at 15:18, on Zulip):

well going to do all these small changes

oli (Jun 21 2019 at 15:18, on Zulip):

yea sorry about not being really available the last few days

Christian Poveda (Jun 21 2019 at 15:18, on Zulip):

oh, we may have to move the random seed to the MemoryExtra struct that already contains Stacks and base_addr

wait wait

Christian Poveda (Jun 21 2019 at 15:19, on Zulip):

Stacks and base_addr would be in the new AllocExtra and those are allocation specific, do you mean MemoryState in intptrcast.rs?

oli (Jun 21 2019 at 15:19, on Zulip):

oh

oli (Jun 21 2019 at 15:19, on Zulip):

wait

oli (Jun 21 2019 at 15:20, on Zulip):

sorry yes, MemoryState

Christian Poveda (Jun 21 2019 at 15:20, on Zulip):

I'm happy to see that small details :D

oli (Jun 21 2019 at 15:21, on Zulip):

yes, four eyes are great

Christian Poveda (Jun 21 2019 at 15:22, on Zulip):

Well it means that I'm starting to understand some parts better

Christian Poveda (Jun 21 2019 at 15:46, on Zulip):

I finished disinfencting stacked_borrows.rs :P

RalfJ (Jun 21 2019 at 17:09, on Zulip):

I finished disinfencting stacked_borrows.rs :P

Thanks :D

Christian Poveda (Jun 21 2019 at 17:21, on Zulip):

But now we have to move the AllocationExtra implementation outside stacked_borrows.rs

RalfJ (Jun 21 2019 at 17:25, on Zulip):

yes, both MemoryExtra and AllocationExtra should be structs with two fields -- one from stacked_borrows and one from intptrcast.

Christian Poveda (Jun 21 2019 at 17:30, on Zulip):

Yep will do that

Christian Poveda (Jun 21 2019 at 19:06, on Zulip):

Ok I did the changes

Christian Poveda (Jun 21 2019 at 19:06, on Zulip):

I'm not completely sold on the idea of using the Dangling Pointer error but I changed the tests accordingly

RalfJ (Jun 21 2019 at 19:07, on Zulip):

what error would you have used?

RalfJ (Jun 21 2019 at 19:07, on Zulip):

and do you know what kind of code can trigger this error?

Christian Poveda (Jun 21 2019 at 19:07, on Zulip):

there are several cases

Christian Poveda (Jun 21 2019 at 19:08, on Zulip):
    let x: i32 = unsafe { *std::ptr::null() }; //~ ERROR dangling pointer was dereferenced
Christian Poveda (Jun 21 2019 at 19:08, on Zulip):
    let g = unsafe {
        std::mem::transmute::<usize, fn(i32)>(42)
    };

    g(42) //~ ERROR dangling pointer was dereferenced
Christian Poveda (Jun 21 2019 at 19:08, on Zulip):

I mean probably for the first we could add an special case when the ptr is zero as in the default machine impl

Christian Poveda (Jun 21 2019 at 19:09, on Zulip):

but the second one, i don't know, that is not even a proper dangling pointer, it was created by doing an "invalid" cast

Christian Poveda (Jun 21 2019 at 19:10, on Zulip):

But yeah those are rough edges at the end, we can fix that next week

Christian Poveda (Jun 21 2019 at 19:12, on Zulip):

I'm more interested in knowing why

&42 as *const i32 as usize as u8 as usize;

fails

Christian Poveda (Jun 21 2019 at 19:12, on Zulip):

shouldn't that be accepted by miri now?

RalfJ (Jun 21 2019 at 19:16, on Zulip):

but the second one, i don't know, that is not even a proper dangling pointer, it was created by doing an "invalid" cast

"proper dangling pointer"? you mean it's too dangling to be put into one category with the other dangling pointers? :joy:

RalfJ (Jun 21 2019 at 19:17, on Zulip):

a pointer is dangling if it doesnt point to valid memory. whether it was obtained by casting from a pad int, out-of-bounds arithmetic or use-after-free doesn't matter.

RalfJ (Jun 21 2019 at 19:17, on Zulip):

shouldn't that be accepted by miri now?

no, we specifically did not use force_bits for the casting stuff

RalfJ (Jun 21 2019 at 19:17, on Zulip):

because that code is messy

Christian Poveda (Jun 21 2019 at 19:19, on Zulip):

"proper dangling pointer"? you mean it's too dangling to be put into one category with the other dangling pointers? :joy:

Well meine Wenigkeit believes that dangling pointers are created when you free memory and leave pointers to that memory segment.

So when I saw the "dangling pointer" error it was more like: I'm actually creating a wild pointer than a dangling one

Christian Poveda (Jun 21 2019 at 19:19, on Zulip):

shouldn't that be accepted by miri now?

no, we specifically did not use force_bits for the casting stuff

Should we add anymore tests to check that the force_* methods are working as expected?

RalfJ (Jun 21 2019 at 19:21, on Zulip):

so something like Box::into_raw(Box::new(0u32)).wrapping_offset(8) is also not "dangling"?

RalfJ (Jun 21 2019 at 19:22, on Zulip):

Should we add anymore tests to check that the force_* methods are working as expected?

well you added force_bits to slice indexing

RalfJ (Jun 21 2019 at 19:22, on Zulip):

so a weird thing you could so is to cast a ptr to an int and use that as a slice index

RalfJ (Jun 21 2019 at 19:22, on Zulip):

but uh there'll be a bounds check first

RalfJ (Jun 21 2019 at 19:23, on Zulip):

ah but you could make it a huge ZST slice :D

RalfJ (Jun 21 2019 at 19:23, on Zulip):

also you could add in miri some force_bits to ptr_op, then stuff like (&mut 4 as *mut _ as usize) * 2 should work

Christian Poveda (Jun 21 2019 at 19:24, on Zulip):

so something like Box::into_raw(Box::new(0u32)).wrapping_offset(8) is also not "dangling"?

Well I would say that it isn't, because it was not a valid pointer that didn't get invalidated after freeing memory, just an invalid pointer created from a valid one. But again I don't have any proper arguments to say that something is or it is not dangling

RalfJ (Jun 21 2019 at 19:24, on Zulip):

you are just using a different definition for the same term, it's okay

RalfJ (Jun 21 2019 at 19:24, on Zulip):

I can't say you are wrong

RalfJ (Jun 21 2019 at 19:24, on Zulip):

but in Miri we have used "dangling" to mean "pointer that cannot be dereferenced"

RalfJ (Jun 21 2019 at 19:25, on Zulip):

so 42 is certainly dangling

Christian Poveda (Jun 21 2019 at 19:25, on Zulip):

oh great

Christian Poveda (Jun 21 2019 at 19:25, on Zulip):

and null is dangling too i suppose

RalfJ (Jun 21 2019 at 19:25, on Zulip):

NULL always got a special error

RalfJ (Jun 21 2019 at 19:25, on Zulip):

don't ask me why^^

Christian Poveda (Jun 21 2019 at 19:25, on Zulip):

Should we keep that special error?

RalfJ (Jun 21 2019 at 19:25, on Zulip):

maybe keep that for now

RalfJ (Jun 21 2019 at 19:25, on Zulip):

I have an error refactoring on my todo list anyway

RalfJ (Jun 21 2019 at 19:25, on Zulip):

then it'll go :P

Christian Poveda (Jun 21 2019 at 19:26, on Zulip):

ok I'll change the method to return the NullPtr error instead when the int is zero

Christian Poveda (Jun 21 2019 at 19:26, on Zulip):

i'm still a little bit confused about what should be a good test here

RalfJ (Jun 21 2019 at 19:27, on Zulip):

I think we already have some dangling ptr tests, including casting a random integer to a ptr and using it

RalfJ (Jun 21 2019 at 19:28, on Zulip):

you could duplicate those to make sure they still error with -Zmiri-seed=... (i.e., with intrptrcast enabled)

Christian Poveda (Jun 21 2019 at 19:28, on Zulip):

oh I forgot about the seed, I have to fix that

RalfJ (Jun 21 2019 at 19:29, on Zulip):

:D

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

now I'm just letting intptrcast enabled by default hahaha

RalfJ (Jun 21 2019 at 19:29, on Zulip):

not sure if there is much to do in terms of compile-fail tests otherwise TBH

Christian Poveda (Jun 21 2019 at 19:30, on Zulip):

where are the non-determinist tests in miri?

RalfJ (Jun 21 2019 at 19:30, on Zulip):

well I think we might even want to do that at some point, both to kill some awful code and to avoid having to duplicate our entire test suite... but not yet

RalfJ (Jun 21 2019 at 19:30, on Zulip):

there aren't many^^

Christian Poveda (Jun 21 2019 at 19:30, on Zulip):

or how do you handle that?

RalfJ (Jun 21 2019 at 19:30, on Zulip):

handle?

RalfJ (Jun 21 2019 at 19:31, on Zulip):

// compile-flags: -Zmiri-seed=4242

Christian Poveda (Jun 21 2019 at 19:31, on Zulip):

ohhh ok

RalfJ (Jun 21 2019 at 19:31, on Zulip):

didn't I already tell you that? anyway^^

Christian Poveda (Jun 21 2019 at 19:31, on Zulip):

maybe, I have the memory of a fly

Christian Poveda (Jun 21 2019 at 19:43, on Zulip):

How can I make the seed to get all the way down to MemoryExtra?

Christian Poveda (Jun 21 2019 at 19:44, on Zulip):

oh i know how :P

Christian Poveda (Jun 21 2019 at 19:44, on Zulip):

nvm

Christian Poveda (Jun 21 2019 at 20:02, on Zulip):

I think we already have some dangling ptr tests, including casting a random integer to a ptr and using it
you could duplicate those to make sure they still error with -Zmiri-seed=... (i.e., with intrptrcast enabled)

what about:
- cast_int_to_fn_ptr.rs
- dangling_pointer_deref.rs
- null_pointer_deref.rs
are they good candidates?

RalfJ (Jun 21 2019 at 20:29, on Zulip):

the fn ptr thing is weird, but sure^^ reminds me that we should have run-pass tests that roudntrip a fn ptr through an int

Christian Poveda (Jun 21 2019 at 20:30, on Zulip):

Should I add other similar tests? like the null pointer write? or just those are fine?

RalfJ (Jun 21 2019 at 20:30, on Zulip):

dangling_pointer_deref looks good, though you'll have to extend it such that ptr_to_int even gets called

RalfJ (Jun 21 2019 at 20:31, on Zulip):

wild_pointer_deref.rs was the one I originally thought of

Christian Poveda (Jun 21 2019 at 20:32, on Zulip):

oh ok I'll do wild pointer then

Christian Poveda (Jun 21 2019 at 20:32, on Zulip):

other than that, I think we are ready to go

oli (Jun 21 2019 at 20:52, on Zulip):

You can do the as usize test by using transmute instead

oli (Jun 21 2019 at 20:52, on Zulip):

That way you can even assert a specific int address for an address of a local

Christian Poveda (Jun 21 2019 at 20:52, on Zulip):

this one &42 as *const i32 as usize as u8 as usize;?

Christian Poveda (Jun 21 2019 at 20:54, on Zulip):

or which one do you mean?

oli (Jun 21 2019 at 21:05, on Zulip):

I guess the first cast to usize

RalfJ (Jun 21 2019 at 21:05, on Zulip):

I think that would require some more patches in allocation.rs

oli (Jun 21 2019 at 21:05, on Zulip):

Then maybe do *2 or sth yo trigger the transform

RalfJ (Jun 21 2019 at 21:05, on Zulip):

to allow reading parts of the bytes of an allocation

RalfJ (Jun 21 2019 at 21:06, on Zulip):

yeah * 2 should work. but not transmute.

Christian Poveda (Jun 21 2019 at 21:07, on Zulip):

yeah * 2 should work. but not transmute.

I'm confused, where is the *2 supposed to go?

RalfJ (Jun 21 2019 at 21:09, on Zulip):

(&32 as *const _ as usize) * 2

RalfJ (Jun 21 2019 at 21:09, on Zulip):

do you understand why that is needed?

Christian Poveda (Jun 21 2019 at 21:10, on Zulip):

not really

RalfJ (Jun 21 2019 at 21:10, on Zulip):

remember the thing about int/ptr values vs types

Christian Poveda (Jun 21 2019 at 21:10, on Zulip):

yep

RalfJ (Jun 21 2019 at 21:10, on Zulip):

(&32 as *const _ as usize) is a pointer value at int type

RalfJ (Jun 21 2019 at 21:11, on Zulip):

the casting code doesn't to ptr_to_int

RalfJ (Jun 21 2019 at 21:11, on Zulip):

whether it should I am not sure

RalfJ (Jun 21 2019 at 21:11, on Zulip):

but even if it did, mem::transmute::<usize, _>(&32) would certainly not call it

RalfJ (Jun 21 2019 at 21:12, on Zulip):

and thus we anyway have to be able to handle ptr values at int type

RalfJ (Jun 21 2019 at 21:12, on Zulip):

so by doing * 2 we end up in the bin_op code in operand.rs, and because one argument has ptr value, we dispatch to ptr_op in the machine

RalfJ (Jun 21 2019 at 21:12, on Zulip):

and that's a good place for Miri to do force_bits

Christian Poveda (Jun 21 2019 at 21:12, on Zulip):

ohhh

RalfJ (Jun 21 2019 at 21:13, on Zulip):

and then we finally run ptr_to_int

Christian Poveda (Jun 21 2019 at 21:13, on Zulip):

ok I understand

Christian Poveda (Jun 21 2019 at 21:13, on Zulip):

buuuut even then

Christian Poveda (Jun 21 2019 at 21:14, on Zulip):
    let x = &42 as *const i32 as usize;
    let y = x * 2;

fails with: attempted to do invalid arithmetic on pointers that would leak base addresses,

RalfJ (Jun 21 2019 at 21:14, on Zulip):

probably because you didn't add the force_bits in Miri's implementation of ptr_op?

Christian Poveda (Jun 21 2019 at 21:14, on Zulip):

should I??

RalfJ (Jun 21 2019 at 21:14, on Zulip):

yes :)

Christian Poveda (Jun 21 2019 at 21:14, on Zulip):

oh ok thats a new one

RalfJ (Jun 21 2019 at 21:15, on Zulip):

basically right here: https://github.com/rust-lang/miri/blob/beba8c4d8a100824867c1ec36dbaa817056e5547/src/operator.rs#L45

RalfJ (Jun 21 2019 at 21:15, on Zulip):

we talked about this a while ago on he rustc side, and then agreed to do this on the miri side

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

ohhh yeah now I remember the conversation

RalfJ (Jun 21 2019 at 21:15, on Zulip):

what that code should do is, check if we run with intrptrcast, and if yes, if the operator is not Offset, call force_bits on both operands and dispatch back to rustc

RalfJ (Jun 21 2019 at 21:17, on Zulip):

all operators except for Offset should be implemented by looking at the integer values.
at least that's my position. this includes pointer comparisons so @oli might disagree. ;)

RalfJ (Jun 21 2019 at 21:17, on Zulip):

it's certainly a reasonable start though, we can always tweak later

RalfJ (Jun 21 2019 at 21:18, on Zulip):

anyway it's getting late here, I'm out. good night!

Christian Poveda (Jun 21 2019 at 21:18, on Zulip):

Ok Ralf, thanks for the explanation

Christian Poveda (Jun 21 2019 at 21:19, on Zulip):

it is clear for me except on the "dispatch back to rustc", to which method?

RalfJ (Jun 21 2019 at 21:21, on Zulip):

binop-something

Christian Poveda (Jun 21 2019 at 21:21, on Zulip):

ok

RalfJ (Jun 21 2019 at 21:21, on Zulip):

they are all in operand.rs

RalfJ (Jun 21 2019 at 21:21, on Zulip):

eh

RalfJ (Jun 21 2019 at 21:21, on Zulip):

operator.rs

Christian Poveda (Jun 21 2019 at 21:30, on Zulip):

I tried to dispatch to https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_mir/interpret/operator.rs.html#108

Christian Poveda (Jun 21 2019 at 21:31, on Zulip):

but it is private D:

RalfJ (Jun 22 2019 at 08:42, on Zulip):

try binary_op

RalfJ (Jun 22 2019 at 08:43, on Zulip):

or should we make it public? not sure what I prefer

Christian Poveda (Jun 22 2019 at 13:01, on Zulip):

What layout should I use to build the ImmTy?

Christian Poveda (Jun 22 2019 at 14:24, on Zulip):

What layout should I use to build the ImmTy?

Solved :P

Christian Poveda (Jun 22 2019 at 14:29, on Zulip):

I broke the hashmap test, I'm trying to see why

Christian Poveda (Jun 22 2019 at 19:57, on Zulip):

Apparently it only happens in local, CI is fine

Christian Poveda (Jun 24 2019 at 13:42, on Zulip):

Hi Oliver, Ralf

oli (Jun 24 2019 at 13:43, on Zulip):

aloha

Christian Poveda (Jun 24 2019 at 13:43, on Zulip):

I'm going to do the corrections Oliver stated on GH

Christian Poveda (Jun 24 2019 at 13:43, on Zulip):

After that is there something else to be done?

oli (Jun 24 2019 at 13:44, on Zulip):

I think it's ready implementation wise, so we can merge after that

Christian Poveda (Jun 24 2019 at 13:44, on Zulip):

Great!

oli (Jun 24 2019 at 13:44, on Zulip):

You can notice that something is ready if the reviewer only has naming and other convention complaints left to give off ;)

Christian Poveda (Jun 24 2019 at 13:44, on Zulip):

:P

Christian Poveda (Jun 24 2019 at 13:48, on Zulip):

I was thinking about the push vs binary_search. On one had it is nice to be completely sure that elements would be added in order and avoid the overhead of searching. On the other hand, modifications in the future could break this (although Ralf suggested that this is unlikely to change in the future)

oli (Jun 24 2019 at 14:33, on Zulip):

I just think it's a lot of effort

oli (Jun 24 2019 at 14:33, on Zulip):

especially if the list grows

Christian Poveda (Jun 24 2019 at 14:34, on Zulip):

I'll use push and do a comment

Christian Poveda (Jun 24 2019 at 14:35, on Zulip):

could you explain the page size FIXME?

oli (Jun 24 2019 at 14:36, on Zulip):

the 2^16 is an arbitrary pick by me

oli (Jun 24 2019 at 14:36, on Zulip):

I think most operating systems use a single memory page

Christian Poveda (Jun 24 2019 at 14:36, on Zulip):

So you want to initialize the base_addr in exactly the page size?

oli (Jun 24 2019 at 14:37, on Zulip):

yea

Christian Poveda (Jun 24 2019 at 14:38, on Zulip):

Why is that important?

Christian Poveda (Jun 24 2019 at 14:38, on Zulip):

I mean

oli (Jun 24 2019 at 14:38, on Zulip):

oh it isn't very important, just represents the OS behaviour better

Christian Poveda (Jun 24 2019 at 14:38, on Zulip):

I understand it is important to start with an address that in fact exists

Christian Poveda (Jun 24 2019 at 14:38, on Zulip):

oh ok

Christian Poveda (Jun 24 2019 at 15:03, on Zulip):

we're good to go

RalfJ (Jun 24 2019 at 19:45, on Zulip):

I just left you a bunch of comments :)

Christian Poveda (Jun 24 2019 at 19:48, on Zulip):

Yayyy

Christian Poveda (Jun 24 2019 at 19:48, on Zulip):

thank you Ralf

RalfJ (Jun 24 2019 at 19:48, on Zulip):

sorry it took me so long. wanted to do it yesterday but then I did a bike trip instead.^^

Christian Poveda (Jun 24 2019 at 19:48, on Zulip):

Don't worry, bike riding is rad :P

Christian Poveda (Jun 24 2019 at 19:49, on Zulip):

completely understandable

Christian Poveda (Jun 24 2019 at 19:55, on Zulip):

To initialize MemoryExtra with the rng, we would need to modify InterpretCx and Memory constructors to receive the rng. That seems to be to specific to be done there. Do you have any other route in mind?

RalfJ (Jun 24 2019 at 20:00, on Zulip):

seems like a TODO for later... what I think should happen is that the InterpretCx constructor takes the initial MemoryExtra as argument

Christian Poveda (Jun 24 2019 at 20:00, on Zulip):

sounds good, I'll do it later this week

Christian Poveda (Jun 24 2019 at 20:11, on Zulip):

@RalfJ what kind of bounds do you mean? https://github.com/rust-lang/miri/pull/779/files/2861ceb2fa1acc2c642abd4f2efb96c713a47e29#r296879361

RalfJ (Jun 24 2019 at 20:12, on Zulip):

oh this is the case where we have an exact hit so the offset is 0?

Christian Poveda (Jun 24 2019 at 20:12, on Zulip):

that pos is smaller than the size of the array?

Christian Poveda (Jun 24 2019 at 20:12, on Zulip):

oh this is the case where we have an exact hit so the offset is 0?

yes

RalfJ (Jun 24 2019 at 20:12, on Zulip):

then please add a comment saying so :)

Christian Poveda (Jun 25 2019 at 12:26, on Zulip):

we'll need to compute the next address here that is divisble by the alignment, but not by the next higher alignment.

is the alignment std::mem::align_of::<usize>()?

Christian Poveda (Jun 25 2019 at 12:40, on Zulip):

The next immediate address that is aligned with alignment should be something like:

global_state.next_base_addr + global_state.next_base_addr % alignment;

this should be exactly divisible by alignment. Is that enough?

Christian Poveda (Jun 25 2019 at 12:52, on Zulip):

Also, shouldn't be better to keep next_base_addr always aligned? I mean, instead of aligning when needed, shouldn't be better to align beforehand to keep next_base_addr alignment as an invariant?

oli (Jun 25 2019 at 13:07, on Zulip):

isn't it global_state.next_base_addr + alignment - global_state.next_base_addr % alignment assuming global_state.next_base_addr % alignment isn't 0?

Christian Poveda (Jun 25 2019 at 13:29, on Zulip):

Oh yeah it is, my bad. Are my other assumptions correct?

Christian Poveda (Jun 25 2019 at 14:52, on Zulip):

Also I tried to reproduce your misalignment but it fails at the assertion. I understood the assert should succeed

Christian Poveda (Jun 25 2019 at 14:55, on Zulip):
let x = (&5u8 as *const u8 as usize) * 2;
let y = (&6u16 as *const u16 as usize) * 2 / 2;
assert!(y % 2 == 1);

this one

Christian Poveda (Jun 25 2019 at 15:36, on Zulip):

Also, shouldn't be better to keep next_base_addr always aligned? I mean, instead of aligning when needed, shouldn't be better to align beforehand to keep next_base_addr alignment as an invariant?

dis doesnt make sense, i just understood

Christian Poveda (Jun 25 2019 at 15:40, on Zulip):

is the alignment std::mem::align_of::<usize>()?

i just discovered alloc.align

oli (Jun 25 2019 at 15:46, on Zulip):

The assert should fail, buy I think it's succeeding atm?

Christian Poveda (Jun 25 2019 at 16:01, on Zulip):

it is failing at the moment

Christian Poveda (Jun 25 2019 at 16:02, on Zulip):

and after doing the alignment tweaks it also fails

Christian Poveda (Jun 25 2019 at 16:02, on Zulip):
                let align = alloc.align.bytes();
                let mut base_addr = global_state.next_base_addr;

                // Align the address with the allocation
                base_addr += align - base_addr % align;
                global_state.next_base_addr = base_addr + alloc.bytes.len() as u64;
                alloc.extra.intptrcast.base_addr.set(Some(base_addr));

                let elem = (base_addr, ptr.alloc_id);

                // Given that `next_base_addr` increases in each allocation, pushing the
                // corresponding tuple keeps `int_to_ptr_map` sorted
                global_state.int_to_ptr_map.push(elem);

                base_addr

these are the changes

Christian Poveda (Jun 25 2019 at 16:04, on Zulip):

I'd like to have a test to avoid breaking the alignment like the one you suggested but i'm not sure why it does not break without fixing the alignment.

RalfJ (Jun 25 2019 at 16:13, on Zulip):

please factor that "rounding up the integer to the next multiple" into a separate method, for readability

RalfJ (Jun 25 2019 at 16:13, on Zulip):

I'm surprised libcore doesn't already have that?

Christian Poveda (Jun 25 2019 at 16:13, on Zulip):

maybe it does and I haven't discovered it yet :P

RalfJ (Jun 25 2019 at 16:14, on Zulip):

can't find one either though^^

oli (Jun 25 2019 at 16:15, on Zulip):

Yea, there's only next_power_of_2 or sth like that

oli (Jun 25 2019 at 16:16, on Zulip):

Why is it failing atm. ..

oli (Jun 25 2019 at 16:16, on Zulip):

Oh maybe there are preceding allocations that do ptr to iny

oli (Jun 25 2019 at 16:17, on Zulip):

Did you run with the seed flag? And it's the assert that fails?

oli (Jun 25 2019 at 16:17, on Zulip):

Try doing the same assert for the first pointer

oli (Jun 25 2019 at 16:17, on Zulip):

So on x

Christian Poveda (Jun 25 2019 at 16:20, on Zulip):

Did you run with the seed flag? And it's the assert that fails?

yes

Christian Poveda (Jun 25 2019 at 16:21, on Zulip):

Try doing the same assert for the first pointer

Loading :clock:

Christian Poveda (Jun 25 2019 at 16:23, on Zulip):

please factor that "rounding up the integer to the next multiple" into a separate method, for readability

Done :D

Christian Poveda (Jun 25 2019 at 16:26, on Zulip):

assert!(x % 2 == 1); also fails with and without doing the alignment

Christian Poveda (Jun 25 2019 at 20:12, on Zulip):

Other than deciding what to do with the alignment tests we are ready to go

oli (Jun 25 2019 at 20:17, on Zulip):

I have no clue what's going on

Christian Poveda (Jun 25 2019 at 20:17, on Zulip):

Oh well

Christian Poveda (Jun 25 2019 at 20:18, on Zulip):

// FIXME: Here be misaligned dragons

oli (Jun 25 2019 at 20:19, on Zulip):

Heh

oli (Jun 25 2019 at 20:19, on Zulip):

Open an issue pls

oli (Jun 25 2019 at 20:19, on Zulip):

Then r=me

Christian Poveda (Jun 25 2019 at 20:19, on Zulip):

what does r= does?

oli (Jun 25 2019 at 21:20, on Zulip):

Oh that just means I'll merge or someone can merge in my name

Christian Poveda (Jun 25 2019 at 21:45, on Zulip):

I opened the ticket and did the corrections that @RalfJ suggested

Christian Poveda (Jun 25 2019 at 21:48, on Zulip):

I made a fool of myself by doing r=oli-obk. Bors slapped me with a nice "This incident will be reported" :P

RalfJ (Jun 26 2019 at 07:21, on Zulip):

@Christian Poveda https://xkcd.com/838/ ;)

RalfJ (Jun 26 2019 at 07:26, on Zulip):

@Christian Poveda addr + align - addr % align will return addr + align if addr is divisible by align. How is that "rounding up to the next multiple (that's >= addr)"?

Christian Poveda (Jun 26 2019 at 13:47, on Zulip):

Oh damn, it's just strictly larger

Christian Poveda (Jun 26 2019 at 13:48, on Zulip):

Sorry yesterday I had trouble sleeping and I was more clumsy than usual

Christian Poveda (Jun 26 2019 at 13:57, on Zulip):

So @RalfJ @oli, is there anything else after fixing that?

RalfJ (Jun 26 2019 at 20:51, on Zulip):

@Christian Poveda let's continue here, this is off-topic in the other topic ;)

RalfJ (Jun 26 2019 at 20:51, on Zulip):

casts are in librustc_mir/interpret/cast.rs

Christian Poveda (Jun 26 2019 at 20:51, on Zulip):

yep sorry haha

RalfJ (Jun 26 2019 at 20:52, on Zulip):

for ptr-to-int casts (of any size) you'll have to do something like "try force_bits but if that fails continue with the existing code path"

RalfJ (Jun 26 2019 at 20:52, on Zulip):

like, match force_bits(...) { Ok(...) => ..., Err(...) => ... }

Christian Poveda (Jun 26 2019 at 20:52, on Zulip):

ok

Christian Poveda (Jun 26 2019 at 21:17, on Zulip):

I'm inside cast.rs, I suppose that the castmethod is the entrypoint here. Should we use force_bits in for every PointerCast variant?

RalfJ (Jun 26 2019 at 21:22, on Zulip):

your entry point is cast_scalar

RalfJ (Jun 26 2019 at 21:22, on Zulip):

(I'd like to clean up cast at some point but not now^^)

RalfJ (Jun 26 2019 at 21:23, on Zulip):

in cast_from_int, there's a case where we cast to RawPtr, that's... interesting but I am not sure what we want to do there so let's ignore it for now

RalfJ (Jun 26 2019 at 21:24, on Zulip):

so cast_from_ptr is where you want to do your thing

Christian Poveda (Jun 26 2019 at 21:28, on Zulip):

Oh ok that seems pretty straightforward.

Christian Poveda (Jun 26 2019 at 21:48, on Zulip):

How should I get the size of the target type? I thought I could use tcx to get the layout but it seems a little complicated

RalfJ (Jun 26 2019 at 21:53, on Zulip):

https://doc.rust-lang.org/nightly/nightly-rustc/syntax/ast/enum.IntTy.html#method.bit_width

RalfJ (Jun 26 2019 at 21:53, on Zulip):

and https://doc.rust-lang.org/nightly/nightly-rustc/syntax/ast/enum.UintTy.html#method.bit_width

Christian Poveda (Jun 26 2019 at 21:53, on Zulip):

oh great

RalfJ (Jun 26 2019 at 21:53, on Zulip):

which are methods on the i in Int(i) and Uint(i)

RalfJ (Jun 26 2019 at 21:53, on Zulip):

(where we have _ or Isize/Usize so far)

Christian Poveda (Jun 26 2019 at 21:58, on Zulip):

yeah ok

Christian Poveda (Jun 26 2019 at 21:59, on Zulip):

I think bors needs a little bit of rest: https://github.com/rust-lang/rust/pull/62158

Christian Poveda (Jun 26 2019 at 22:03, on Zulip):

which are methods on the i in Int(i) and Uint(i)

and what about isize and usize, should I use 64?

RalfJ (Jun 26 2019 at 22:04, on Zulip):

no you should use self.memory.pointer_size()

Christian Poveda (Jun 26 2019 at 22:14, on Zulip):

Hahhahaha

Christian Poveda (Jun 26 2019 at 22:14, on Zulip):

santa is not comming here this year https://github.com/rust-lang/rust/pull/62158#issuecomment-506064431

RalfJ (Jun 26 2019 at 22:18, on Zulip):

bors is very over-eager and any time you mention his name he thinks it's a command, even in code blocks or quotes. ;)

Christian Poveda (Jun 26 2019 at 22:19, on Zulip):

yeah.. "Today I learned"

RalfJ (Jun 26 2019 at 22:19, on Zulip):

however, it seems my "delegate" failed because it was in a review approval, not a comment...

Christian Poveda (Jun 26 2019 at 22:19, on Zulip):

great, now you owe me a christmas gift

RalfJ (Jun 26 2019 at 22:30, on Zulip):

:gift:

Christian Poveda (Jun 26 2019 at 22:31, on Zulip):

I did the change to cast_from_ptr. I'm running tests to check if something broke

Christian Poveda (Jun 26 2019 at 22:46, on Zulip):

So the following tests broke:

    [ui] ui/consts/const-eval/const_raw_ptr_ops.rs
    [ui] ui/consts/const-eval/issue-52442.rs
    [ui] ui/consts/const-eval/match-test-ptr-null.rs
    [ui] ui/issues/issue-52023-array-size-pointer-cast.rs

I think most of them are ok because the errors when doing ptr as usize changed to PointerAsBytes. However, the error when doing &loop { break } changed from it is undefined behavior to use this value to evaluation of constant value failed inside issue-52442.rs

RalfJ (Jun 26 2019 at 22:47, on Zulip):

remember that Miri might run with or without ptr-to-int support

RalfJ (Jun 26 2019 at 22:47, on Zulip):

that's why I said you need to catch the Err

RalfJ (Jun 26 2019 at 22:47, on Zulip):

which it seems you did not?

Christian Poveda (Jun 26 2019 at 22:55, on Zulip):

let me find a way of doing it without being redundant

Christian Poveda (Jun 26 2019 at 23:16, on Zulip):

so basically ptr as usize and ptr as isize will try to use force_bits and if it fails, then they will return ptr.into()

Christian Poveda (Jun 26 2019 at 23:17, on Zulip):

to keep the old behaviour

Christian Poveda (Jun 26 2019 at 23:45, on Zulip):

ok now to_bits_or_ptr inside librustc\mir\interpret\value.rs panicked when running run-pass/cast-rfc0401.rs. I'm going to push this code so you can see what's going on

Christian Poveda (Jun 26 2019 at 23:50, on Zulip):

https://github.com/christianpoveda/rust/commit/e1e95a83861271e144f65903c652ab9fa9bd64b8

RalfJ (Jun 28 2019 at 06:37, on Zulip):

@Christian Poveda wait in https://github.com/rust-lang/rust/pull/62158#issuecomment-506620125 you linked to the PR itself?

RalfJ (Jun 28 2019 at 06:37, on Zulip):

did you mean the corresponding Miri PR?

Christian Poveda (Jun 28 2019 at 06:37, on Zulip):

Oh lol

Christian Poveda (Jun 28 2019 at 06:38, on Zulip):

I fixed the comment, dont tell anyone

Christian Poveda (Jun 28 2019 at 07:28, on Zulip):

@oli https://github.com/rust-lang/miri/issues/224#issuecomment-506631948 you mean like being able to just enable intptrcast in one direction?

RalfJ (Jun 28 2019 at 07:45, on Zulip):

https://github.com/christianpoveda/rust/commit/e1e95a83861271e144f65903c652ab9fa9bd64b8

I think this does what we want but we can make it a bit less messy but better preserving the original structure of the match

Christian Poveda (Jun 28 2019 at 07:46, on Zulip):

It does not, when the size of the types is different it fails

RalfJ (Jun 28 2019 at 07:46, on Zulip):

please define a helper method that does something like

fn int_size(ty) -> Option<...> {
match ty {  Int(i) => i.bit_width(),
            Uint(i) => i.bit_width(),
_ => bug!("This is not an int"),
}
RalfJ (Jun 28 2019 at 07:46, on Zulip):

then you can, I think, stick to an outermost match and only have code inside the branches, instead of after the match

Christian Poveda (Jun 28 2019 at 07:46, on Zulip):

I though about that but didn't feel that comfortable creating new methods here and there, I'm still shy on that. Will do :)

RalfJ (Jun 28 2019 at 07:47, on Zulip):

you can just do that locally

RalfJ (Jun 28 2019 at 07:47, on Zulip):

make it a helper method defined inside cast_from_ptr

Christian Poveda (Jun 28 2019 at 07:47, on Zulip):

Ok I will fix that, however I'm still worried about the test that this broke

RalfJ (Jun 28 2019 at 07:48, on Zulip):

it's a trade-off between matching twice and overall code structure, and I agree it's not clear-cut. I think matching twice, if once is done ina helper method, is nicer though.

RalfJ (Jun 28 2019 at 07:48, on Zulip):

this broke a test?

Christian Poveda (Jun 28 2019 at 07:48, on Zulip):

yep

Christian Poveda (Jun 28 2019 at 07:48, on Zulip):

ok now to_bits_or_ptr inside librustc\mir\interpret\value.rs panicked when running run-pass/cast-rfc0401.rs. I'm going to push this code so you can see what's going on

here

Christian Poveda (Jun 28 2019 at 07:50, on Zulip):

the thing is that cast-rfc0401 has a lot of casts :P so I was not sure about which one panicked. IIRC the problem was that the sizes did not match, one was 2 and the other one 4, so probably something from u16 to u32

Christian Poveda (Jun 28 2019 at 07:56, on Zulip):

I'll do the change to make the code more readable for the moment

RalfJ (Jun 28 2019 at 08:01, on Zulip):

do you have panic message + stacktrace?

Christian Poveda (Jun 28 2019 at 08:01, on Zulip):

Not right now, let me reproduce it

RalfJ (Jun 28 2019 at 08:02, on Zulip):

also if you run rustc with RUSTC_LOG=rustc_mir::interpret=info you should get a better clue of where in the interpreted program it failed

Christian Poveda (Jun 28 2019 at 08:02, on Zulip):

ohhh nice trick

Christian Poveda (Jun 28 2019 at 08:02, on Zulip):

didn't know that

Christian Poveda (Jun 28 2019 at 08:41, on Zulip):

Ok, found the error, time to get the traces

Christian Poveda (Jun 28 2019 at 08:47, on Zulip):

I'm overwhelmed by the ammount of lines in this :P. I have the gut feeling that this:

 INFO 2019-06-28T08:41:50Z: rustc_mir::interpret::step: _1 = (const "/home/christian/Workspace/contrib/rust/src/test/run-pass/cast-rfc0401.rs", const 147u32, const 5u32)

Is telling me the line that is being executed but I'm not sure why

Christian Poveda (Jun 28 2019 at 08:53, on Zulip):

Ok yep, here is the problem (or at least that's what my nonexistent experience reading this tells me)

Christian Poveda (Jun 28 2019 at 08:53, on Zulip):
    let x = [1,2,3];
    let first : *const u32 = &x[0];

    assert_eq!(first, &x as *const _);
    assert_eq!(first, &x as *const u32);
Christian Poveda (Jun 28 2019 at 08:56, on Zulip):

And the actual error is:

thread 'rustc' panicked at 'assertion failed: `(left == right)`
  left: `Size { raw: 2 }`,
 right: `Size { raw: 8 }`', src/librustc/mir/interpret/value.rs:367:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
Christian Poveda (Jun 28 2019 at 14:03, on Zulip):

Probably I'm wrong and the error is not there because the sizes do not make sense to me

RalfJ (Jun 28 2019 at 16:24, on Zulip):

that looks like you are asking for a ptr-sized value and got one of size 2

RalfJ (Jun 28 2019 at 16:25, on Zulip):

a rustc stacktrace would still be useful

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

let met get it

RalfJ (Jun 28 2019 at 16:25, on Zulip):

the RUST_BACKTRACE=1 thing

RalfJ (Jun 28 2019 at 16:25, on Zulip):

as for the long log, yes it's huge ;)

RalfJ (Jun 28 2019 at 16:25, on Zulip):

can you take the last 50 lines or so and paste them in a gist or so?

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

Sorry, I'm in other machine ATM

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

so I'll have to compile/run tests and so on so it will take a while

RalfJ (Jun 28 2019 at 16:56, on Zulip):

dont worry, I need to have dinner first anyway ;)

RalfJ (Jun 28 2019 at 16:56, on Zulip):

and then move my stuff down to the basement where temperatures are survivable

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

Lecker! :)

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

and then move my stuff down to the basement where temperatures are survivable

I got a mail at work telling me that one of the clusters in spain went down because the oil refrigeration is failing because of the heat wave

RalfJ (Jun 28 2019 at 17:02, on Zulip):

oO

RalfJ (Jun 28 2019 at 17:03, on Zulip):

on Sunday there's some chance we might see a new all-time-ever record for the highest temperature in Germany

RalfJ (Jun 28 2019 at 17:03, on Zulip):

(well, ever = for the last 100-150 years or so, where we have records)

Christian Poveda (Jun 28 2019 at 17:05, on Zulip):

I think is safe to assume that even without data, there is no way such high temperatures could be reached since a long time ago

RalfJ (Jun 28 2019 at 17:05, on Zulip):

as usual, up to a small probability. ;)

RalfJ (Jun 28 2019 at 17:06, on Zulip):

statistics and probability distributions are great

RalfJ (Jun 28 2019 at 17:06, on Zulip):

but I agree. it's a very small probability.

Christian Poveda (Jun 28 2019 at 18:08, on Zulip):

Ok I have the logs now

Christian Poveda (Jun 28 2019 at 18:10, on Zulip):

here is the rustc backtrace

Christian Poveda (Jun 28 2019 at 18:10, on Zulip):

https://gist.github.com/christianpoveda/4287862a77a35bf8ca84a6c7129d6295

Christian Poveda (Jun 28 2019 at 18:11, on Zulip):

and here the miri logs
https://gist.github.com/christianpoveda/815b11e267d1b705e52d4ee3a1ad8b12

RalfJ (Jun 28 2019 at 18:32, on Zulip):

oh it's in const_prop :(

RalfJ (Jun 28 2019 at 18:33, on Zulip):

that's always bad news, that pass is a mess

RalfJ (Jun 28 2019 at 18:33, on Zulip):

btw you seem to be building without debug info so there are no line numbers in that backtrace

Christian Poveda (Jun 28 2019 at 18:33, on Zulip):

how should I enable debug info?

RalfJ (Jun 28 2019 at 18:33, on Zulip):

debuginfo-level = 1 is a good thing to have in your config.toml

Christian Poveda (Jun 28 2019 at 18:34, on Zulip):

ok nice

Christian Poveda (Jun 28 2019 at 18:34, on Zulip):

let me do it

RalfJ (Jun 28 2019 at 18:42, on Zulip):

btw, just added the most amazing intrpcast test so far :D https://github.com/rust-lang/miri/pull/798

RalfJ (Jun 28 2019 at 18:42, on Zulip):

this is something people have been running into all the time

RalfJ (Jun 28 2019 at 18:43, on Zulip):

so it's great that soon this will work in Miri out-of-the-box :)

Christian Poveda (Jun 28 2019 at 18:43, on Zulip):

:party_ball:

Christian Poveda (Jun 28 2019 at 18:45, on Zulip):

debuginfo-level = 1 is a good thing to have in your config.toml

I did this but I dont see a difference

Christian Poveda (Jun 28 2019 at 18:46, on Zulip):

so it's great that soon this will work in Miri out-of-the-box :)

So, now miri can execute the code needed to print a pointer, right?

RalfJ (Jun 28 2019 at 18:47, on Zulip):

debuginfo-level = 1 is a good thing to have in your config.toml

I did this but I dont see a difference

don't worry about it, the stacktrace is good enough for us for now

Christian Poveda (Jun 28 2019 at 18:47, on Zulip):

ok

RalfJ (Jun 28 2019 at 18:47, on Zulip):

you'll have to rebuild everything to see a difference

RalfJ (Jun 28 2019 at 18:47, on Zulip):

so it's great that soon this will work in Miri out-of-the-box :)

So, now miri can execute the code needed to print a pointer, right?

yes :D

Christian Poveda (Jun 28 2019 at 18:47, on Zulip):

debuginfo-level = 1 is a good thing to have in your config.toml

I did this but I dont see a difference

don't worry about it, the stacktrace is good enough for us for now

ok not today

RalfJ (Jun 28 2019 at 18:48, on Zulip):

did you run the miri test suite with your cast-patched compiler?

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

i did not, let me try

RalfJ (Jun 28 2019 at 18:49, on Zulip):

oh d'oh I see the problem

RalfJ (Jun 28 2019 at 18:49, on Zulip):

you are using the size of the target type to extract the source value

Christian Poveda (Jun 28 2019 at 18:49, on Zulip):

tell me

Christian Poveda (Jun 28 2019 at 18:50, on Zulip):

you are using the size of the target type to extract the source value

Loading...

Christian Poveda (Jun 28 2019 at 18:50, on Zulip):

Oh crap

RalfJ (Jun 28 2019 at 18:50, on Zulip):

so first of all try to unify the Isize/Usize branch with the other one

RalfJ (Jun 28 2019 at 18:50, on Zulip):

I think we talked about that, with thr helper function

RalfJ (Jun 28 2019 at 18:51, on Zulip):

then, in that branch, you need always do self.force_bits(Scalar::Ptr(ptr), self.memory.pointer_size())

RalfJ (Jun 28 2019 at 18:51, on Zulip):

if that gives Err, old stuff happens

RalfJ (Jun 28 2019 at 18:52, on Zulip):

but if that gives Ok... I think the best bet is to forward to cast_from_int

RalfJ (Jun 28 2019 at 18:52, on Zulip):

then you dont even need to do all the size matching, I think that function already does ot

Christian Poveda (Jun 28 2019 at 18:52, on Zulip):

Ok let me try

RalfJ (Jun 28 2019 at 18:53, on Zulip):

though you'll have to get rid of the src_layout parameter

RalfJ (Jun 28 2019 at 18:53, on Zulip):

replace it by a src_signed: bool and src_size: Size or so

Christian Poveda (Jun 28 2019 at 18:57, on Zulip):

There is no way to build the layout to avoid doing that?

RalfJ (Jun 28 2019 at 18:58, on Zulip):

you dont want to build a new layout

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

ok I don't

RalfJ (Jun 28 2019 at 18:58, on Zulip):

maybe we can use the ptr layout

RalfJ (Jun 28 2019 at 18:58, on Zulip):

can you check what abi.is_signed() does for pointers?

RalfJ (Jun 28 2019 at 18:59, on Zulip):

you are making the &T directly from the *mut T?

RalfJ (Jun 28 2019 at 18:59, on Zulip):

argh sry wrong thread

Christian Poveda (Jun 28 2019 at 18:59, on Zulip):
    pub fn is_signed(&self) -> bool {
        match *self {
            Abi::Scalar(ref scal) => match scal.value {
                Primitive::Int(_, signed) => signed,
                _ => false,
            },
            _ => false,
        }
    }
RalfJ (Jun 28 2019 at 19:00, on Zulip):

so, it probably says unsigned

RalfJ (Jun 28 2019 at 19:01, on Zulip):

well that's great news, then I think you can just add a src_layout parameter to cast_from_ptr

RalfJ (Jun 28 2019 at 19:01, on Zulip):

and forward that

RalfJ (Jun 28 2019 at 19:01, on Zulip):

also rename ty to dst_ty

Christian Poveda (Jun 28 2019 at 19:03, on Zulip):

shouldn't be better to use the whole dest_layout instead of dest_ty for the sake of symmetry?

RalfJ (Jun 28 2019 at 19:07, on Zulip):

sure why not

Christian Poveda (Jun 28 2019 at 19:14, on Zulip):

It's compiling (https://xkcd.com/303/) but it seems it worked

Christian Poveda (Jun 28 2019 at 19:25, on Zulip):

Argh... my build failed and I have to recompile it from scratch

RalfJ (Jun 28 2019 at 20:04, on Zulip):

ouch

RalfJ (Jun 29 2019 at 09:18, on Zulip):

@Christian Poveda are you ready for some git foo? :D
https://github.com/rust-lang/rust/pull/62158#issuecomment-506942578

Christian Poveda (Jun 29 2019 at 11:58, on Zulip):

The horror

RalfJ (Jun 29 2019 at 11:58, on Zulip):

(please wait a bit before doing that, I am doing stuff TM)

RalfJ (Jun 29 2019 at 11:58, on Zulip):

also isn't it like the middle of the night in your timezone right now? I thought I had some time for the stuff TM before you came back^^

RalfJ (Jun 29 2019 at 12:20, on Zulip):

@Christian Poveda okay done with my stuff

RalfJ (Jun 29 2019 at 12:21, on Zulip):

now the instructions I posted there should work :)

Christian Poveda (Jun 29 2019 at 12:45, on Zulip):

It's 7 24

Christian Poveda (Jun 29 2019 at 12:45, on Zulip):

It's 7 24

Christian Poveda (Jun 29 2019 at 12:45, on Zulip):

Ill get out of bed at 7 30

RalfJ (Jun 29 2019 at 12:47, on Zulip):

that is the middle of the night if you ask me, at least for a weekend :P

RalfJ (Jun 29 2019 at 12:48, on Zulip):

also does that mean you are 7:30h behind me? interesting 0.5h offset

Christian Poveda (Jun 29 2019 at 12:49, on Zulip):

Hahaha

Christian Poveda (Jun 29 2019 at 12:49, on Zulip):

Hallo

Christian Poveda (Jun 29 2019 at 12:49, on Zulip):

No, the thing is that zulip broke on my phone and didn't send the message, its 7 50 now

Christian Poveda (Jun 29 2019 at 12:50, on Zulip):

that is the middle of the night if you ask me, at least for a weekend :P

I used to think like that but I try to keep my schedule consistent the whole week

RalfJ (Jun 29 2019 at 12:51, on Zulip):

lol^^ 7h then, okay. not 9h as I thought it would be. ;)

Christian Poveda (Jun 29 2019 at 12:51, on Zulip):

hehehe

Christian Poveda (Jun 29 2019 at 12:53, on Zulip):

In fact it is pretty handy. My work is done with some people at CERN in Switzerland so we have a clock with the central european time in the office, so I use it secretly to also check what hour is over there to work on Rusty stuff

RalfJ (Jun 29 2019 at 12:53, on Zulip):

hehe

RalfJ (Jun 29 2019 at 12:53, on Zulip):

and you have to tell me at some point what you are working on, working with CERN sounds awesome :D

RalfJ (Jun 29 2019 at 12:53, on Zulip):

I've visited there once. I like physics, so it was a blast :)

Christian Poveda (Jun 29 2019 at 12:53, on Zulip):

It sounds awesome, that's it :P

Christian Poveda (Jun 29 2019 at 12:56, on Zulip):

Well, i work at CMS (one of the LHC experiments). My job is to check all the nodes in the grid and report any problems that might come up

Christian Poveda (Jun 29 2019 at 12:58, on Zulip):

So if for example if the cluster that KIT has connected to the grid stops accepting job submissions I should check why, try to solve it from here and if it is a local problem, write to their sysadmin so they can fix it

RalfJ (Jun 29 2019 at 13:00, on Zulip):

I've seen the Atlas detector in Real Life but not CMS unfortunately^^

RalfJ (Jun 29 2019 at 13:01, on Zulip):

cool stuff :)

Christian Poveda (Jun 29 2019 at 13:01, on Zulip):

Oh the competition >:P

Christian Poveda (Jun 29 2019 at 13:02, on Zulip):

I have a gift for you: https://github.com/rust-lang/rust/pull/62229

RalfJ (Jun 29 2019 at 13:05, on Zulip):

just saw it. ;)

Christian Poveda (Jun 29 2019 at 13:12, on Zulip):

I'll wait for your comments and then rebase it

RalfJ (Jun 29 2019 at 13:17, on Zulip):

No need to rebase if it doesnt conflict

RalfJ (Jun 29 2019 at 13:18, on Zulip):

in the mean time, I gave you something to do at https://github.com/rust-lang/rust/pull/62158#issuecomment-506942578 :)

Christian Poveda (Jun 29 2019 at 13:21, on Zulip):

No need to rebase if it doesnt conflict

But I need to hide my embarassing intermediate commits

Christian Poveda (Jun 29 2019 at 13:21, on Zulip):

in the mean time, I gave you something to do at https://github.com/rust-lang/rust/pull/62158#issuecomment-506942578 :)

Ok done it, should I force push this?

RalfJ (Jun 29 2019 at 14:21, on Zulip):

@Christian Poveda please do :)

Christian Poveda (Jun 29 2019 at 14:22, on Zulip):

Es ist fertig :P

RalfJ (Jun 29 2019 at 14:23, on Zulip):

where's the miri submodule update?

RalfJ (Jun 29 2019 at 14:23, on Zulip):

seems you skipped that part? :D

Christian Poveda (Jun 29 2019 at 14:23, on Zulip):

no way Jose

Christian Poveda (Jun 29 2019 at 14:23, on Zulip):

I did it, but the Cargo.lock did not change

RalfJ (Jun 29 2019 at 14:23, on Zulip):

thats cool

RalfJ (Jun 29 2019 at 14:23, on Zulip):

but then you didnt commit

RalfJ (Jun 29 2019 at 14:23, on Zulip):

or didnt push

Christian Poveda (Jun 29 2019 at 14:23, on Zulip):

i did

Christian Poveda (Jun 29 2019 at 14:23, on Zulip):

/me searches his shell history

RalfJ (Jun 29 2019 at 14:24, on Zulip):

look at https://github.com/rust-lang/rust/pull/62158/commits. you didnt. ;)

Christian Poveda (Jun 29 2019 at 14:24, on Zulip):

Well IDK but nothing to commit, working tree clean

RalfJ (Jun 29 2019 at 14:25, on Zulip):

oh dang sorry that's my fault... stupid x.py automatism that I turned off long ago and forgot about

Christian Poveda (Jun 29 2019 at 14:25, on Zulip):

wait

Christian Poveda (Jun 29 2019 at 14:25, on Zulip):

second part

RalfJ (Jun 29 2019 at 14:25, on Zulip):

try again but without x.py

cd src/tools/miri
git fetch origin
git reset --hard origin/rustup
cd ../../..
git commit -am "update miri"
Christian Poveda (Jun 29 2019 at 14:25, on Zulip):

I'm working from my fork

Christian Poveda (Jun 29 2019 at 14:26, on Zulip):

so origin is pointing to christianpoveda/rust

RalfJ (Jun 29 2019 at 14:26, on Zulip):

and then do ./x.py and see if it changes anythng (git status) and if it did add that to the commit (git commit -a --amend)

Christian Poveda (Jun 29 2019 at 14:26, on Zulip):

should i use the remote pointing to rust-lang/rust?

RalfJ (Jun 29 2019 at 14:26, on Zulip):

wait what? origin in src/tools/miri?!?

Christian Poveda (Jun 29 2019 at 14:26, on Zulip):

wait what? origin in src/tools/miri?!?

Loading

RalfJ (Jun 29 2019 at 14:26, on Zulip):

but why?^^

Christian Poveda (Jun 29 2019 at 14:26, on Zulip):

Ohhh

Christian Poveda (Jun 29 2019 at 14:27, on Zulip):

nevermind I forgot that's another repo

RalfJ (Jun 29 2019 at 14:27, on Zulip):

good :)

Christian Poveda (Jun 29 2019 at 14:28, on Zulip):

tada

Christian Poveda (Jun 29 2019 at 14:28, on Zulip):

it worked

RalfJ (Jun 29 2019 at 14:28, on Zulip):

there are conflicts in the PR though

RalfJ (Jun 29 2019 at 14:28, on Zulip):

which means your rebase didnt do what you should

RalfJ (Jun 29 2019 at 14:28, on Zulip):

did you make you you rebased onto the lastest upstream master?

Christian Poveda (Jun 29 2019 at 14:28, on Zulip):

let me redo it

RalfJ (Jun 29 2019 at 14:29, on Zulip):

your git setup is weird^^ for me origin is always upstream

RalfJ (Jun 29 2019 at 14:29, on Zulip):

if thats not the case for you, I am afraid you'll have to figure things out on your own...

Christian Poveda (Jun 29 2019 at 14:29, on Zulip):

yeah I know how to fix it

Christian Poveda (Jun 29 2019 at 14:29, on Zulip):

gimme a second

RalfJ (Jun 29 2019 at 14:30, on Zulip):

when I have a fork I usually do

git clone https://github.com/rust-lang/rust.git
cd rust
git remote add ralf git@github.com:RalfJung/rust.git

and that way origin is upstream and ralf is my fork

Christian Poveda (Jun 29 2019 at 14:30, on Zulip):

I do origin -> me, upstream -> rust-lang

Christian Poveda (Jun 29 2019 at 14:30, on Zulip):

ok now it should work

RalfJ (Jun 29 2019 at 14:33, on Zulip):

I do origin -> me, upstream -> rust-lang

fair

RalfJ (Jun 29 2019 at 14:33, on Zulip):

I'llt ry to remember that for my future instructions :D

RalfJ (Jun 29 2019 at 14:33, on Zulip):

hm now there are two "update miri" commits?^^

RalfJ (Jun 29 2019 at 14:34, on Zulip):

ah no taht was GH being silly

Christian Poveda (Jun 29 2019 at 14:34, on Zulip):

because the night is dark and full of terrors

RalfJ (Jun 29 2019 at 14:34, on Zulip):

oh no

Christian Poveda (Jun 29 2019 at 14:34, on Zulip):

they were, i just squashed :P

RalfJ (Jun 29 2019 at 14:34, on Zulip):

something went very wrong

Christian Poveda (Jun 29 2019 at 14:34, on Zulip):

refresh

RalfJ (Jun 29 2019 at 14:34, on Zulip):

look at this: https://github.com/rust-lang/rust/pull/62158/commits/47f80aed9d7854699a1bd1151f0cdad4667533a9

RalfJ (Jun 29 2019 at 14:34, on Zulip):

you updated 7 submodules or so

RalfJ (Jun 29 2019 at 14:34, on Zulip):

submodules are awful :/

Christian Poveda (Jun 29 2019 at 14:34, on Zulip):

/me cries in spanish

RalfJ (Jun 29 2019 at 14:34, on Zulip):

so let's try that again

Christian Poveda (Jun 29 2019 at 14:34, on Zulip):

let me delete the commit and do it again

RalfJ (Jun 29 2019 at 14:35, on Zulip):

step 1: cleanup. git reset --hard HEAD^

RalfJ (Jun 29 2019 at 14:35, on Zulip):

yes

RalfJ (Jun 29 2019 at 14:35, on Zulip):

now, next thing, get submodules clean

RalfJ (Jun 29 2019 at 14:35, on Zulip):

I have a git subfix command for that

RalfJ (Jun 29 2019 at 14:35, on Zulip):

that's git submodule update --init --recursive

RalfJ (Jun 29 2019 at 14:35, on Zulip):

you can also do ./x.py, it does that per default (which is why it killed your miri update the last time)

RalfJ (Jun 29 2019 at 14:36, on Zulip):

after that, git diff should be clean

Christian Poveda (Jun 29 2019 at 14:36, on Zulip):

it is

RalfJ (Jun 29 2019 at 14:36, on Zulip):

always do git diff right before git commit to know what you are committing! that would have prevented this mistake

RalfJ (Jun 29 2019 at 14:36, on Zulip):

now do the cd src/tools/miri etc part

Christian Poveda (Jun 29 2019 at 14:37, on Zulip):

It worked

RalfJ (Jun 29 2019 at 14:39, on Zulip):

yay, it did :D

RalfJ (Jun 29 2019 at 14:39, on Zulip):

congrats on your first miri submodule update :D

Christian Poveda (Jun 29 2019 at 14:41, on Zulip):

/me smiles

Christian Poveda (Jun 29 2019 at 14:56, on Zulip):

It's 7 24

Christian Poveda (Jun 29 2019 at 14:56, on Zulip):

Ill get out of bed at 7 30

RalfJ (Jun 29 2019 at 14:56, on Zulip):

lol

Christian Poveda (Jun 29 2019 at 14:57, on Zulip):

Ohh nice the zulip app finally worked

Christian Poveda (Jun 29 2019 at 15:00, on Zulip):

twice

Christian Poveda (Jun 29 2019 at 15:06, on Zulip):

Do you think that using intptrcast during casting needs some tests?

RalfJ (Jun 29 2019 at 15:11, on Zulip):

on the Miri side? yes!

RalfJ (Jun 29 2019 at 15:11, on Zulip):

&42 as usize as u8

Christian Poveda (Jun 29 2019 at 15:12, on Zulip):

Oh the famous one

RalfJ (Jun 29 2019 at 15:12, on Zulip):

and then something that transmutes...

fn transmute_ptr_to_int<T>(x: *const T) -> usize { mem::transmute(x) }

transmute_ptr_to_int(&42) as u8
RalfJ (Jun 29 2019 at 15:12, on Zulip):

that's a slightly different code path

RalfJ (Jun 29 2019 at 15:13, on Zulip):

you can then even compare you get the same result both ways :D

Christian Poveda (Jun 29 2019 at 15:13, on Zulip):

Should I reuse the same intptrcast.rs test file?

RalfJ (Jun 29 2019 at 15:13, on Zulip):

yes please

Christian Poveda (Jun 29 2019 at 16:25, on Zulip):
error[E0606]: casting `&i32` as `usize` is invalid
  --> $DIR/intptrcast.rs:14:13
   |
14 |     let b = &42 as usize as u8;
   |             ---^^^^^^^^^
   |             |
   |             cannot cast `&i32` as `usize`
   |             help: dereference the expression: `*&42`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0606`.
RalfJ (Jun 29 2019 at 16:47, on Zulip):

yeah I forgot the intermediate raw pointer

RalfJ (Jun 29 2019 at 16:47, on Zulip):

just adjust it to make it work ;)

Christian Poveda (Jun 29 2019 at 16:48, on Zulip):

it does not work either

RalfJ (Jun 29 2019 at 16:48, on Zulip):

which error?

Christian Poveda (Jun 29 2019 at 16:49, on Zulip):
error[E0080]: Miri evaluation error: a raw memory access tried to access part of a pointer value as raw bytes
  --> $DIR/intptrcast.rs:14:13
   |
14 |     let b = &42 as *const i32 as usize as u8;
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Miri evaluation error: a raw memory access tried to access part of a pointer value as raw bytes
   |
   = note: inside call to `main` at /home/christian/Workspace/contrib/rust/src/libstd/rt.rs:64:34
   = note: inside call to closure at /home/christian/Workspace/contrib/rust/src/libstd/rt.rs:49:73
   = note: inside call to closure at /home/christian/Workspace/contrib/rust/src/libstd/sys_common/backtrace.rs:77:5
   = note: inside call to `std::sys_common::backtrace::__rust_begin_short_backtrace::<[closure@DefId(1:5944 ~ std[9246]::rt[0]::lang_start_internal[0]::{{closure}}[0]::{{closure}}[0]) 0:&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /home/christian/Workspace/contrib/rust/src/libstd/rt.rs:49:13
   = note: inside call to closure at /home/christian/Workspace/contrib/rust/src/libstd/panicking.rs:294:40
   = note: inside call to `std::panicking::try::do_call::<[closure@DefId(1:5943 ~ std[9246]::rt[0]::lang_start_internal[0]::{{closure}}[0]) 0:&&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /home/christian/Workspace/contrib/rust/src/libstd/panicking.rs:290:5
   = note: inside call to `std::panicking::try::<i32, [closure@DefId(1:5943 ~ std[9246]::rt[0]::lang_start_internal[0]::{{closure}}[0]) 0:&&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe]>` at /home/christian/Workspace/contrib/rust/src/libstd/panic.rs:388:9
   = note: inside call to `std::panic::catch_unwind::<[closure@DefId(1:5943 ~ std[9246]::rt[0]::lang_start_internal[0]::{{closure}}[0]) 0:&&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /home/christian/Workspace/contrib/rust/src/libstd/rt.rs:48:25
   = 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::<()>`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0080`.
RalfJ (Jun 29 2019 at 16:50, on Zulip):

does adding -Zmiri-disable-validation to the flags help?

Christian Poveda (Jun 29 2019 at 16:50, on Zulip):

Im a complete idiot

Christian Poveda (Jun 29 2019 at 16:50, on Zulip):

Oh wait, having the flag in the file enables it?

Christian Poveda (Jun 29 2019 at 16:50, on Zulip):

of course it does nvm

Christian Poveda (Jun 29 2019 at 16:50, on Zulip):

let me disable validation

RalfJ (Jun 29 2019 at 16:50, on Zulip):

only when you run it in the test suite

RalfJ (Jun 29 2019 at 16:51, on Zulip):

or just do ./miri run tests/run-pass/intptrcast -Zmiri-seed=42 -Zmiri-disable-validation

RalfJ (Jun 29 2019 at 16:51, on Zulip):

that's probably easier anyway while you are still debugging, to run the test directly

Christian Poveda (Jun 29 2019 at 16:51, on Zulip):

same error

RalfJ (Jun 29 2019 at 16:52, on Zulip):

then it looks like your patch doesn't do what it should

Christian Poveda (Jun 29 2019 at 16:52, on Zulip):

oh well...

RalfJ (Jun 29 2019 at 16:52, on Zulip):

(I didnt look at it yet.)

Christian Poveda (Jun 29 2019 at 16:55, on Zulip):

probably force_bits always fails and it's just executing the old behaviour

RalfJ (Jun 29 2019 at 17:00, on Zulip):

well you need to set -Zmiri-seed to make that not fail

RalfJ (Jun 29 2019 at 17:00, on Zulip):

but you can check that easily by making this not the first thing that intrptrcast tests

RalfJ (Jun 29 2019 at 17:00, on Zulip):

so you can tell if your prior tests still pass

Christian Poveda (Jun 29 2019 at 17:00, on Zulip):

well thats the thing

Christian Poveda (Jun 29 2019 at 17:01, on Zulip):

all the intptrcasts tests pass, including the intptrcast.rs original test

Christian Poveda (Jun 29 2019 at 17:01, on Zulip):

so, it is enabled

RalfJ (Jun 29 2019 at 17:01, on Zulip):

how exactly are you running the test?

Christian Poveda (Jun 29 2019 at 17:02, on Zulip):

./miri test intptrcast

RalfJ (Jun 29 2019 at 17:04, on Zulip):

that will take the flag into account, yeah

RalfJ (Jun 29 2019 at 17:04, on Zulip):

I usually prefer to test via ./miri run when working on a particular test case

RalfJ (Jun 29 2019 at 17:04, on Zulip):

avoids all the compiletest junk around it

RalfJ (Jun 29 2019 at 17:04, on Zulip):

but then you need to remember to pass the flags

RalfJ (Jun 29 2019 at 17:05, on Zulip):

either way, let's see where the error comes from: do MIRI_BACKTRACE=1 ./miri ...

Christian Poveda (Jun 29 2019 at 17:07, on Zulip):
stack backtrace:
   0: <rustc::mir::interpret::error::InterpErrorInfo as core::convert::From<rustc::mir::interpret::error::InterpError<u64>>>::from::h82824632c52c5588 (0x7f7418faf77d)
   1: <T as core::convert::Into<U>>::into::h20a9a7d5cf115f97 (0x55a20c2a306c)
             at /home/christian/Workspace/contrib/rust/src/libcore/convert.rs:540
      rustc_mir::interpret::cast::<impl rustc_mir::interpret::eval_context::InterpretCx<M>>::cast_from_ptr::hfcd977be000db5ce
             at /home/christian/Workspace/contrib/miri/<::rustc::mir::interpret::err macros>:2
      rustc_mir::interpret::cast::<impl rustc_mir::interpret::eval_context::InterpretCx<M>>::cast_scalar::h670bd5c154169f8c
             at /home/christian/Workspace/contrib/rust/src/librustc_mir/interpret/cast.rs:154
      rustc_mir::interpret::cast::<impl rustc_mir::interpret::eval_context::InterpretCx<M>>::cast::h557da123a3f5f2b8
             at /home/christian/Workspace/contrib/rust/src/librustc_mir/interpret/cast.rs:71
      rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpretCx<M>>::eval_rvalue_into_place::h9a9fd2a982f648ae
             at /home/christian/Workspace/contrib/rust/src/librustc_mir/interpret/step.rs:264
   2: rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpretCx<M>>::statement::h850ab37b10a6d2a9 (0x55a20c2a7b1f)
             at /home/christian/Workspace/contrib/rust/src/librustc_mir/interpret/step.rs:85
      rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpretCx<M>>::step::ha5c6b83338e2c707
             at /home/christian/Workspace/contrib/rust/src/librustc_mir/interpret/step.rs:61
      rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpretCx<M>>::run::h57378d0f609d97a3
             at /home/christian/Workspace/contrib/rust/src/librustc_mir/interpret/step.rs:40
   3: miri::eval_main::{{closure}}::h89905980517900df (0x55a20c1f0c10)
             at src/lib.rs:233
      miri::eval_main::h552c5f486ccc6954
             at src/lib.rs:232
   4: <miri::MiriCompilerCalls as rustc_driver::Callbacks>::after_analysis::{{closure}}::h3d330d8e2971815c (0x55a20c1dc85d)
             at src/bin/miri.rs:50
      rustc_interface::passes::BoxedGlobalCtxt::enter::{{closure}}::{{closure}}::h6f9191972b894d3d
             at /home/christian/Workspace/contrib/rust/src/librustc_interface/passes.rs:803
      rustc::ty::context::tls::enter_global::{{closure}}::hfd01637ff81ead61
             at /home/christian/Workspace/contrib/rust/src/librustc/ty/context.rs:1963
      rustc::ty::context::tls::enter_context::{{closure}}::hd577f6dac22a3083
             at /home/christian/Workspace/contrib/rust/src/librustc/ty/context.rs:1929
      rustc::ty::context::tls::set_tlv::h5a7b061128e7af69
             at /home/christian/Workspace/contrib/rust/src/librustc/ty/context.rs:1862
      rustc::ty::context::tls::enter_context::h0b2e865728aa47f6
             at /home/christian/Workspace/contrib/rust/src/librustc/ty/context.rs:1928
      rustc::ty::context::tls::enter_global::h467844306770a71d
             at /home/christian/Workspace/contrib/rust/src/librustc/ty/context.rs:1962
      rustc_interface::passes::BoxedGlobalCtxt::enter::{{closure}}::h2960106158652d01
             at /home/christian/Workspace/contrib/rust/src/librustc_interface/passes.rs:803
      rustc_interface::passes::BoxedGlobalCtxt::access::{{closure}}::h8c4d575a5dad4633
             at /home/christian/Workspace/contrib/miri/<::rustc_data_structures::box_region::declare_box_region_type macros>:17
   5: rustc_interface::passes::create_global_ctxt::{{closure}}::h3ff26032759f5a9e (0x7f7419b6f02a)
   6: alloc::boxed::<impl core::ops::generator::Generator for core::pin::Pin<alloc::boxed::Box<G>>>::resume::hd76abb37f74b0a9c (0x55a20c1d4dd8)
             at /home/christian/Workspace/contrib/rust/src/liballoc/boxed.rs:897
      rustc_data_structures::box_region::PinnedGenerator<I,A,R>::access::hbeeff207cbbdc75b
             at /home/christian/Workspace/contrib/rust/src/librustc_data_structures/box_region.rs:52
      rustc_interface::passes::BoxedGlobalCtxt::access::h9b1575dd5e5b40d6
             at /home/christian/Workspace/contrib/miri/<::rustc_data_structures::box_region::declare_box_region_type macros>:19
      rustc_interface::passes::BoxedGlobalCtxt::enter::h7a7f03749629a8b5
             at /home/christian/Workspace/contrib/rust/src/librustc_interface/passes.rs:803
      <miri::MiriCompilerCalls as rustc_driver::Callbacks>::after_analysis::hb6707e96b5844de3
             at src/bin/miri.rs:43
   7: rustc_interface::interface::run_compiler_in_existing_thread_pool::h6e338a04d6764d6d (0x7f7419d2f7ce)
   8: std::thread::local::LocalKey<T>::with::h4a03cecefd8dd030 (0x7f7419d37222)
   9: scoped_tls::ScopedKey<T>::set::h549ab693e1140564 (0x7f7419d3cab1)
  10: std::sys_common::backtrace::__rust_begin_short_backtrace::hb167da37f98141ab (0x7f7419d66428)
  11: __rust_maybe_catch_panic (0x7f7418384dfa)
  12: core::ops::function::FnOnce::call_once{{vtable.shim}}::h4d9f25aca7afcc7b (0x7f7419d31a99)
  13: <alloc::boxed::Box<F> as core::ops::function::FnOnce<A>>::call_once::hb025baad84174e4e (0x7f741837088f)
  14: std::sys_common::thread::start_thread::h31fed19fe88dea46 (0x7f741834e9a0)
  15: std::sys::unix::thread::Thread::new::thread_start::h1d763bf49ef30be4 (0x7f741837bd76)
  16: start_thread (0x7f74182c157f)
  17: __clone (0x7f74181d70e3)
  18: <unknown> (0x0)
Christian Poveda (Jun 29 2019 at 17:09, on Zulip):

so cast_from_ptr, right?

Christian Poveda (Jun 29 2019 at 17:10, on Zulip):

How can we be sure that the error came from the actual err!(..) inside it and not from force_ptr?

Christian Poveda (Jun 29 2019 at 17:11, on Zulip):

oh

Christian Poveda (Jun 29 2019 at 17:11, on Zulip):

forget it

Christian Poveda (Jun 29 2019 at 17:12, on Zulip):

it's my fault I'm on the "initialize ecx with 'MemoryExtra'" branch

Christian Poveda (Jun 29 2019 at 17:12, on Zulip):

plop!

RalfJ (Jun 29 2019 at 17:12, on Zulip):

aha ;)

Christian Poveda (Jun 29 2019 at 17:12, on Zulip):

Hahahaha, I should have a "jackass" version for compile coding

RalfJ (Jun 29 2019 at 17:12, on Zulip):

How can we be sure that the error came from the actual err!(..) inside it and not from force_ptr?

check the line numbers

Christian Poveda (Jun 29 2019 at 17:13, on Zulip):

ok

Christian Poveda (Jun 29 2019 at 17:13, on Zulip):

will recompile and see whats up

Christian Poveda (Jun 29 2019 at 17:14, on Zulip):

Hahahaha, I should have a "jackass" version for compile coding

Hello! I'm Christian and this is Jackass! On today's episode we'll se how to test a feature that is not implemented in the branch you're in

RalfJ (Jun 29 2019 at 17:18, on Zulip):

as someone not watching much TV while growing up (is that a TV thing?)... this went over my head^^

RalfJ (Jun 29 2019 at 17:18, on Zulip):

you can apply my review feedback while you are at it ;)

RalfJ (Jun 29 2019 at 17:18, on Zulip):

but I can guess some of it from context :D

Christian Poveda (Jun 29 2019 at 17:21, on Zulip):

I was not a TV kid either

Christian Poveda (Jun 29 2019 at 17:21, on Zulip):

but all my school buddies did watch it so I watch an episode or two. The show is a bunch of stupid guys doing crazy and dangerous stunts

Christian Poveda (Jun 29 2019 at 17:21, on Zulip):

you can apply my review feedback while you are at it ;)

I already did! In fact, I realized because I was going to do the change and then didn't found my code in the file...

RalfJ (Jun 29 2019 at 17:22, on Zulip):

^^

Christian Poveda (Jun 29 2019 at 18:12, on Zulip):

It worked

Christian Poveda (Jun 29 2019 at 18:12, on Zulip):

/me dances

Christian Poveda (Jun 29 2019 at 18:16, on Zulip):

I'm going to do a PR in miri with the new test

RalfJ (Jun 29 2019 at 18:19, on Zulip):

awesome!

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

just pushed the changes to the rustc PR branch

RalfJ (Jun 29 2019 at 18:24, on Zulip):

just r+'ed them :)

Christian Poveda (Jun 30 2019 at 09:44, on Zulip):

Hallo!

Christian Poveda (Jun 30 2019 at 09:48, on Zulip):

I removed the trailing whitespaces

Christian Poveda (Jun 30 2019 at 09:48, on Zulip):

I used to have that on my vim configuration but then I started using LSP for everything and I ended up removing the plugin I used to fix that kind of stuff

Christian Poveda (Jun 30 2019 at 10:10, on Zulip):

https://github.com/christianpoveda/dotfiles/commit/643eb93f372d7d941d51f73f308a70aa7cd6e49e :P

Christian Poveda (Jun 30 2019 at 14:48, on Zulip):

@RalfJ should I continue with the next item in the intptrcast issue?

RalfJ (Jun 30 2019 at 14:59, on Zulip):

sure :D

RalfJ (Jun 30 2019 at 14:59, on Zulip):

"partial reads from pointers" is an interesting one, you'll see some new parts of rustc

RalfJ (Jun 30 2019 at 15:00, on Zulip):

however I dont have that much time for mentoring until July 11th... :/

RalfJ (Jun 30 2019 at 15:00, on Zulip):

also have you seen https://github.com/rust-lang/miri/pull/809 ? :D

RalfJ (Jun 30 2019 at 15:00, on Zulip):

let's see if that works...

RalfJ (Jun 30 2019 at 15:00, on Zulip):

it actually might not because of my strict(er) alignment checks. well we'll see.

Christian Poveda (Jun 30 2019 at 15:36, on Zulip):

also have you seen https://github.com/rust-lang/miri/pull/809 ? :D

Ohh fancy

Christian Poveda (Jun 30 2019 at 15:37, on Zulip):

however I dont have that much time for mentoring until July 11th... :/

That would be a nice exercise for my independence :P

Christian Poveda (Jun 30 2019 at 15:39, on Zulip):

"partial reads from pointers" is an interesting one, you'll see some new parts of rustc

What does "partial" means in this context?

RalfJ (Jun 30 2019 at 15:39, on Zulip):

here's a piece of code for you, a test case for "partially loading a pointer":

let some_ptr = &42;
let ptr_to_ptr = &some_ptr as *const _ as *const u8;
let _val = *ptr_to_ptr;

this will currently fail to execute I think, even with all your stuff landed. it should work though.
this is "easy mode", once that works you can come back and ask for "hard mode". ;)

the key method that needs changing is read_scalar at https://github.com/rust-lang/rust/blob/c06f80a3c6988d28db43f0cff3f35d9a005c3ad5/src/librustc/mir/interpret/allocation.rs#L313.

RalfJ (Jun 30 2019 at 15:40, on Zulip):

what makes that test case nasty is that we are reading just one byte from a pointer... combined with how we store pointers in memory, that's a nasty special case.

RalfJ (Jun 30 2019 at 15:40, on Zulip):

in particular you can see the special case for size == cx.data_layout().pointer_size in the code I linked

Christian Poveda (Jun 30 2019 at 15:41, on Zulip):

Ohh so you have a pointer to N bytes and you cast to a pointer to M bytes with M < N and then dereference it

RalfJ (Jun 30 2019 at 15:53, on Zulip):

the key part is that it's a pointer to a pointer

RalfJ (Jun 30 2019 at 15:53, on Zulip):

that you cast to a pointer to u8

RalfJ (Jun 30 2019 at 15:53, on Zulip):

and then dereferencing it reads only the first byte of the pointer that the pointer points to

Christian Poveda (Jun 30 2019 at 15:53, on Zulip):

so that should be force_bits and then force_ptr I suppose?

RalfJ (Jun 30 2019 at 15:54, on Zulip):

well the first thing is, how are pointers even stored in memory in Miri

RalfJ (Jun 30 2019 at 15:54, on Zulip):

take a look at https://github.com/rust-lang/rust/blob/c06f80a3c6988d28db43f0cff3f35d9a005c3ad5/src/librustc/mir/interpret/allocation.rs#L17

RalfJ (Jun 30 2019 at 15:54, on Zulip):

you can see that we store a bunch of u8 in bytes

Christian Poveda (Jun 30 2019 at 15:55, on Zulip):

yes

RalfJ (Jun 30 2019 at 15:55, on Zulip):

but a Pointer consists of an offset and an alloc_id and more, that's at least 16 bytes of data and yet it only takes up 8 bytes of space (assuming 64bit)

Christian Poveda (Jun 30 2019 at 15:56, on Zulip):

so, where's the rest of the information of the Pointer?

Christian Poveda (Jun 30 2019 at 15:58, on Zulip):

or what is your point?

RalfJ (Jun 30 2019 at 15:58, on Zulip):

the rest is in the relocations

RalfJ (Jun 30 2019 at 15:59, on Zulip):

so say we have an allocation (of size 8) that stores a pointer

RalfJ (Jun 30 2019 at 16:01, on Zulip):

that allocation is going to look as follows:
bytes, has size 8, stores the offset part of the pointer (using the target's endianess)
relocations (the type is defined here) is basically a hash map, and it stores 0: (alloc_id, tag)

RalfJ (Jun 30 2019 at 16:01, on Zulip):

this thing in relocations means "at offset 0, we are storing a pointer; the next N bytes in bytes are the offset and here is the missing information"

Christian Poveda (Jun 30 2019 at 16:06, on Zulip):

the next N bytes in bytes are the offset and here is the missing information"

that offset is the offset of the allocated pointer, right?

RalfJ (Jun 30 2019 at 16:17, on Zulip):

they are the offset field of the Pointer

RalfJ (Jun 30 2019 at 16:17, on Zulip):

see https://doc.rust-lang.org/nightly/nightly-rustc/rustc/mir/interpret/struct.Pointer.html

Christian Poveda (Jun 30 2019 at 16:17, on Zulip):

yep, that's what I meant :P

Christian Poveda (Jun 30 2019 at 16:18, on Zulip):

And the AllocId and the Tag are the missing information that relocations stores in this particular case

RalfJ (Jun 30 2019 at 16:36, on Zulip):

yes

RalfJ (Jun 30 2019 at 16:36, on Zulip):

(there is no other case^^)

Christian Poveda (Jun 30 2019 at 16:37, on Zulip):

Ok

RalfJ (Jun 30 2019 at 16:37, on Zulip):

in general of course an allocation can store pointers and "raw" data, so an allocation of size 40 might have an entry like 8: (alloc_id, tag) in the relocations, indicating that the bytes 8..16 are actually a pointer (and the remaining bytes are "normal" bytes)

Christian Poveda (Jun 30 2019 at 16:38, on Zulip):

in general of course an allocation can store pointers and "raw" data, so an allocation of size 40 might have an entry like 8: (alloc_id, tag) in the relocations, indicating that the bytes 8..16 are actually a pointer (and the remaining bytes are "normal" bytes)

yeah that's what I thought when I said "in this particular case"

RalfJ (Jun 30 2019 at 16:39, on Zulip):

okay :)

Christian Poveda (Jun 30 2019 at 16:44, on Zulip):

So, when we do the ptr to ptr cast, what happens with the pointer's allocation?

RalfJ (Jun 30 2019 at 16:53, on Zulip):

nothing

RalfJ (Jun 30 2019 at 16:53, on Zulip):

a ptr to ptr cast just changes the type, and leaves the value and everything else unchanged

RalfJ (Jun 30 2019 at 16:53, on Zulip):

it's a NOP, really

Christian Poveda (Jun 30 2019 at 16:54, on Zulip):

So when you try to derefernce you're still getting the same address at the end but you end up reading less bytes

RalfJ (Jun 30 2019 at 16:54, on Zulip):

yes

Christian Poveda (Jun 30 2019 at 16:57, on Zulip):

and then, what is the real problem?

RalfJ (Jun 30 2019 at 17:06, on Zulip):

it means that when reading memory you cant just look at the bytes, you have to consider the relocations as well

RalfJ (Jun 30 2019 at 17:06, on Zulip):

and currently the only way we support to do that is when the read starts exactly where the relocation starts and has size 8

RalfJ (Jun 30 2019 at 17:06, on Zulip):

because then we can just make it a Pointer

RalfJ (Jun 30 2019 at 17:07, on Zulip):

in any other case, you are reading one byte of the offset of the pointer, and that's the wrong thing to return, so we error instead

Christian Poveda (Jun 30 2019 at 17:17, on Zulip):

and currently the only way we support to do that is when the read starts exactly where the relocation starts and has size 8

this is what read_scalar does, right?

Christian Poveda (Jun 30 2019 at 17:18, on Zulip):

I assume that's what check_relocations used for

RalfJ (Jun 30 2019 at 17:20, on Zulip):

exactly

Christian Poveda (Jun 30 2019 at 17:25, on Zulip):

So let me see If i understand

Christian Poveda (Jun 30 2019 at 17:25, on Zulip):

if we try to do a partial read

Christian Poveda (Jun 30 2019 at 17:26, on Zulip):

The size of what we are reading will be different from the original size of the pointer, so when we check_relocationsis going to fail because the pointer in fact has rellocations with its tag and allocation id

Christian Poveda (Jun 30 2019 at 17:30, on Zulip):

I'm still confused with the sizes between the size of the pointers and the size of the pointees

Christian Poveda (Jul 01 2019 at 18:15, on Zulip):

@RalfJ , hey hi! I saw the perf regression when I changed mem_extra to mem in the tag methods, should I do the inlining change?

RalfJ (Jul 01 2019 at 18:16, on Zulip):

you can but it doesnt help

RalfJ (Jul 01 2019 at 18:16, on Zulip):

I already tried that in https://github.com/rust-lang/rust/pull/62264

Christian Poveda (Jul 01 2019 at 18:16, on Zulip):

Oh damn

Christian Poveda (Jul 01 2019 at 18:16, on Zulip):

What is the course of action there? revert the change?

RalfJ (Jul 01 2019 at 18:19, on Zulip):

I'm currently benchmarking that

RalfJ (Jul 01 2019 at 18:19, on Zulip):

because I cant really believe this caused a regression^^

Christian Poveda (Jul 01 2019 at 18:20, on Zulip):

hehehe

RalfJ (Jul 01 2019 at 18:20, on Zulip):

but if the revert confirms this (by being an improvement), I'll try only reverting the parts of the change that we did "for consistency"

Christian Poveda (Jul 01 2019 at 18:20, on Zulip):

yeah that would be the tag methods only

RalfJ (Jul 01 2019 at 18:20, on Zulip):

if that fixes the problem, good for us. though I'd also really like to understand why.

RalfJ (Jul 01 2019 at 18:20, on Zulip):

these perf things are the worst :/

RalfJ (Jul 01 2019 at 18:21, on Zulip):

I spent so much time debugging perf issues in PRs that really shouldnt have any, its no fun. and takes a lot of time. and I dont really know what I am doing.

Christian Poveda (Jul 01 2019 at 18:21, on Zulip):

I'm going to take some notes about the ptr to ptr cast to understand it better and I'll be back with questions

RalfJ (Jul 01 2019 at 18:23, on Zulip):

kk

RalfJ (Jul 01 2019 at 18:24, on Zulip):

you can also first do the "force_ptr when creating a non-ZST reference." thing if you want, that's easier. ;)

Christian Poveda (Jul 01 2019 at 18:25, on Zulip):

Hehehe, no I want to tackle this. Thinking in a selfish way, my only benefit of this is learning how it works.

RalfJ (Jul 01 2019 at 18:26, on Zulip):

fair :D

RalfJ (Jul 01 2019 at 18:27, on Zulip):

it's also not on the critical path to anything so you can have as much time as you want :)

RalfJ (Jul 01 2019 at 18:27, on Zulip):

some of the other things I might do at some point to push ahead https://github.com/rust-lang/miri/issues/785

RalfJ (Jul 02 2019 at 07:26, on Zulip):

this started when intptrcast landed

RalfJ (Jul 02 2019 at 07:26, on Zulip):

@Christian Poveda hm... https://travis-ci.org/RalfJung/miri-test-libstd/builds/553141007?utm_medium=notification&utm_source=email

RalfJ (Jul 02 2019 at 07:27, on Zulip):

I'll investigate tonight ;)

Christian Poveda (Jul 02 2019 at 19:25, on Zulip):

I started looking at this and here is the trace

thread 'rustc' panicked at 'Box<Any>', src/librustc_errors/lib.rs:650:9
stack backtrace:
   0: <unknown>
   1: <unknown>
   2: <unknown>
   3: <unknown>
   4: <unknown>
   5: <unknown>
   6: <unknown>
   7: <unknown>
   8: <unknown>
   9: <unknown>
  10: <unknown>
  11: <unknown>
  12: <unknown>
  13: <rustc_mir::interpret::eval_context::InterpretCx<miri::machine::Evaluator> as miri::operator::EvalContextExt>::ptr_op
             at src/operator.rs:0
  14: <miri::machine::Evaluator as rustc_mir::interpret::machine::Machine>::ptr_op
             at src/machine.rs:170
  15: rustc_mir::interpret::operator::<impl rustc_mir::interpret::eval_context::InterpretCx<M>>::binary_op
             at /Users/christian/Workspace/contrib/rust/src/librustc_mir/interpret/operator.rs:309
  16: rustc_mir::interpret::operator::<impl rustc_mir::interpret::eval_context::InterpretCx<M>>::binop_ignore_overflow
             at /Users/christian/Workspace/contrib/rust/src/librustc_mir/interpret/operator.rs:34
  17: rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpretCx<M>>::eval_rvalue_into_place
             at /Users/christian/Workspace/contrib/rust/src/librustc_mir/interpret/step.rs:155
  18: rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpretCx<M>>::statement
             at /Users/christian/Workspace/contrib/rust/src/librustc_mir/interpret/step.rs:85
  19: rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpretCx<M>>::step
             at /Users/christian/Workspace/contrib/rust/src/librustc_mir/interpret/step.rs:61
  20: rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpretCx<M>>::run
             at /Users/christian/Workspace/contrib/rust/src/librustc_mir/interpret/step.rs:40
  21: miri::eval::eval_main::{{closure}}
             at src/eval.rs:185
  22: miri::eval::eval_main
             at src/eval.rs:184
  23: <miri::MiriCompilerCalls as rustc_driver::Callbacks>::after_analysis::{{closure}}
             at src/bin/miri.rs:50
  24: rustc_interface::passes::BoxedGlobalCtxt::enter::{{closure}}::{{closure}}
             at /Users/christian/Workspace/contrib/rust/src/librustc_interface/passes.rs:803
  25: rustc::ty::context::tls::enter_global::{{closure}}
             at /Users/christian/Workspace/contrib/rust/src/librustc/ty/context.rs:1963
  26: rustc::ty::context::tls::enter_context::{{closure}}
             at /Users/christian/Workspace/contrib/rust/src/librustc/ty/context.rs:1929
  27: rustc::ty::context::tls::set_tlv
             at /Users/christian/Workspace/contrib/rust/src/librustc/ty/context.rs:1862
  28: rustc::ty::context::tls::enter_context
             at /Users/christian/Workspace/contrib/rust/src/librustc/ty/context.rs:1928
  29: rustc::ty::context::tls::enter_global
             at /Users/christian/Workspace/contrib/rust/src/librustc/ty/context.rs:1962
  30: rustc_interface::passes::BoxedGlobalCtxt::enter::{{closure}}
             at /Users/christian/Workspace/contrib/rust/src/librustc_interface/passes.rs:803
  31: rustc_interface::passes::BoxedGlobalCtxt::access::{{closure}}
             at ./<::rustc_data_structures::box_region::declare_box_region_type macros>:17
  32: <unknown>
  33: alloc::boxed::<impl core::ops::generator::Generator for core::pin::Pin<alloc::boxed::Box<G>>>::resume
             at /Users/christian/Workspace/contrib/rust/src/liballoc/boxed.rs:976
  34: rustc_data_structures::box_region::PinnedGenerator<I,A,R>::access
             at /Users/christian/Workspace/contrib/rust/src/librustc_data_structures/box_region.rs:52
  35: rustc_interface::passes::BoxedGlobalCtxt::access
             at ./<::rustc_data_structures::box_region::declare_box_region_type macros>:19
  36: rustc_interface::passes::BoxedGlobalCtxt::enter
             at /Users/christian/Workspace/contrib/rust/src/librustc_interface/passes.rs:803
  37: <miri::MiriCompilerCalls as rustc_driver::Callbacks>::after_analysis
             at src/bin/miri.rs:43
  38: <unknown>
  39: <unknown>
  40: <unknown>
  41: <unknown>
  42: <unknown>
  43: <unknown>
  44: <unknown>
  45: <unknown>
  46: <unknown>
  47: <unknown>
  48: <unknown>
  49: <unknown>
query stack during panic:
end of query stack
error: aborting due to previous error
Christian Poveda (Jul 02 2019 at 19:26, on Zulip):

So, it seems this is happening because https://github.com/rust-lang/rust/blob/master/src/librustc_mir/interpret/operator.rs#L309

Christian Poveda (Jul 02 2019 at 19:26, on Zulip):

Was not that the change that we decided to not make at the end?

RalfJ (Jul 02 2019 at 19:37, on Zulip):

theres some crucial debug info missing from that trace

RalfJ (Jul 02 2019 at 19:37, on Zulip):

like where in Miri this goes on^^

Christian Poveda (Jul 02 2019 at 19:37, on Zulip):

I can get you the miri log

RalfJ (Jul 02 2019 at 19:37, on Zulip):

src/operator.rs:0 :/

RalfJ (Jul 02 2019 at 21:39, on Zulip):

oh I know whats going on

RalfJ (Jul 02 2019 at 21:40, on Zulip):

its fat ptr comparison

RalfJ (Jul 02 2019 at 21:40, on Zulip):

again

RalfJ (Jul 02 2019 at 21:40, on Zulip):

we cant do to_scalar for Eq and friends

RalfJ (Jul 02 2019 at 21:41, on Zulip):

I'll think about a fix... tomorrow^^

Christian Poveda (Jul 03 2019 at 18:27, on Zulip):

The version is just the commit hash, right?

RalfJ (Jul 03 2019 at 18:32, on Zulip):

yes

Christian Poveda (Jul 03 2019 at 18:35, on Zulip):

How come std::mem::transmute is being affected by the changes on explicit casts?

Christian Poveda (Jul 03 2019 at 18:48, on Zulip):

I thought transmute was an intrinsic

RalfJ (Jul 03 2019 at 20:25, on Zulip):

I have no idea

RalfJ (Jul 03 2019 at 20:25, on Zulip):

its likely a change in program state earlier that only surfaces there

RalfJ (Jul 03 2019 at 20:58, on Zulip):

@Christian Poveda found one more nit^^ can you accept the suggestion at https://github.com/rust-lang/miri/pull/803#discussion_r300141552 ? Then I'll r+ for real.

Christian Poveda (Jul 03 2019 at 21:00, on Zulip):

it is done

RalfJ (Jul 03 2019 at 21:00, on Zulip):

thanks!

Christian Poveda (Jul 03 2019 at 21:05, on Zulip):

It is good to see all those PRs advancing :P However I'm really curious about the transmute issue

RalfJ (Jul 03 2019 at 21:13, on Zulip):

uh... looks like the issue also occurs in Miri itself: https://travis-ci.com/rust-lang/miri/jobs/213103563

Christian Poveda (Jul 03 2019 at 21:14, on Zulip):

Yep, I wrote that in the PR comment

RalfJ (Jul 03 2019 at 21:14, on Zulip):

so either something went wrong when you tested it, or something else changed, or you get caught up in the fact that we now are testing way more code with intrptrcast enabled

RalfJ (Jul 03 2019 at 21:15, on Zulip):

(we are now running basically the entire miri test suite with and without intptrcast, to get better coverage. that uncovered some issues, most of which I fixed with https://github.com/rust-lang/miri/pull/820)

Christian Poveda (Jul 03 2019 at 21:15, on Zulip):

I remember that the miri tests passed in local with a custom rustc build with intptrcast for casts enabled

Christian Poveda (Jul 03 2019 at 21:15, on Zulip):

Including the ones that are failing now

RalfJ (Jul 03 2019 at 21:16, on Zulip):

those probably only ran with intrptrcast disabled back then

RalfJ (Jul 03 2019 at 21:16, on Zulip):

i.e., our test suite was not good for finding intptrcast issues

RalfJ (Jul 03 2019 at 21:16, on Zulip):

now it is better at that :D

Christian Poveda (Jul 03 2019 at 21:16, on Zulip):

:P

Christian Poveda (Jul 03 2019 at 21:16, on Zulip):

Well, how do we start looking into this one?

RalfJ (Jul 03 2019 at 21:17, on Zulip):

it's a validation failure, for function types

RalfJ (Jul 03 2019 at 21:17, on Zulip):

the code for that is in rustc, librustc_mit/interpret/validation.rs or so

RalfJ (Jul 03 2019 at 21:18, on Zulip):

oh actually this looks very familiar

RalfJ (Jul 03 2019 at 21:18, on Zulip):

I think one of my in-flight PRs fixes this

RalfJ (Jul 03 2019 at 21:18, on Zulip):

namely, https://github.com/rust-lang/rust/pull/62245

RalfJ (Jul 03 2019 at 21:19, on Zulip):

concretely, this change

Christian Poveda (Jul 03 2019 at 21:19, on Zulip):

If you want I could fetch your changes and test what happens

RalfJ (Jul 03 2019 at 21:19, on Zulip):

it's basically a missing force_ptr

RalfJ (Jul 03 2019 at 21:19, on Zulip):

sure, why not

RalfJ (Jul 03 2019 at 21:19, on Zulip):

I am going to go to sleep now ;)

Christian Poveda (Jul 03 2019 at 21:19, on Zulip):

Sure, sleep tight :P

RalfJ (Jul 03 2019 at 21:20, on Zulip):

you could submit an emergency fix that replaces the to_ptr there with a force_ptr... but probably it makes more sense to wait for my thing to land, hoping review won't take too long

Christian Poveda (Jul 04 2019 at 16:38, on Zulip):

Hi @RalfJ, what changes does your PR has that fix the bug with transmute?

RalfJ (Jul 04 2019 at 21:42, on Zulip):

quoting myself:

namely, this change

Christian Poveda (Jul 05 2019 at 14:15, on Zulip):

Should I continue with the partial reads? or should I wait for the current problems with casting to be solved?

RalfJ (Jul 05 2019 at 17:57, on Zulip):

(sorry I'll be on low-communication mode for a week. important deadline coming up.)

Christian Poveda (Jul 15 2019 at 15:03, on Zulip):

I have a couple of questions

Christian Poveda (Jul 15 2019 at 15:04, on Zulip):

(when) should we revert the changes done by @RalfJ in https://github.com/rust-lang/miri/pull/803?

Christian Poveda (Jul 15 2019 at 15:12, on Zulip):

And the second one is about:

it means that when reading memory you cant just look at the bytes, you have to consider the relocations as well

this means that https://github.com/rust-lang/rust/blob/c06f80a3c6988d28db43f0cff3f35d9a005c3ad5/src/librustc/mir/interpret/allocation.rs#L330 needs to be changed?

RalfJ (Jul 16 2019 at 12:32, on Zulip):

(when) should we revert the changes done by RalfJ in https://github.com/rust-lang/miri/pull/803?

they have all been reverted already :)

RalfJ (Jul 16 2019 at 12:34, on Zulip):

And the second one is about:

it means that when reading memory you cant just look at the bytes, you have to consider the relocations as well

this means that https://github.com/rust-lang/rust/blob/c06f80a3c6988d28db43f0cff3f35d9a005c3ad5/src/librustc/mir/interpret/allocation.rs#L330 needs to be changed?

actually get_bytes_with_undef_and_ptr will likely already need changes. with size=1, that checks that no relocation overlaps with the "edge" of that access, but of course when accessing one byte of a pointer, there will be an overlap

Christian Poveda (Jul 16 2019 at 15:27, on Zulip):

I'm not sure I'm following

RalfJ (Jul 16 2019 at 16:19, on Zulip):

I guess I am not sure what the question is^^

Christian Poveda (Jul 16 2019 at 16:19, on Zulip):

So the question is, what am I supposed to do? :P

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

I thought that read_target_uint did the reading, but the actual bytes are extracted using get_bytes_with_undef_and_ptr, right?

RalfJ (Jul 16 2019 at 16:22, on Zulip):

I'd say have a good look at alloc.rs and read the doc comments on what you find there, that should help understand how things are organized

RalfJ (Jul 16 2019 at 16:22, on Zulip):

get_bytes_with_undef_and_ptr does some "preliminary checking" and then returns a byte slice

RalfJ (Jul 16 2019 at 16:22, on Zulip):

it is an internal helper function

RalfJ (Jul 16 2019 at 16:23, on Zulip):

did we have a testcase already that this change is supposed to make work? IMO that's always a good way to start for a new feature :)

Christian Poveda (Jul 16 2019 at 16:23, on Zulip):

get_bytes_with_undef_and_ptr does some "preliminary checking" and then returns a byte slice

but that byte slice contains the actual bytes from the allocation?

Christian Poveda (Jul 16 2019 at 16:23, on Zulip):

did we have a testcase already that this change is supposed to make work? IMO that's always a good way to start for a new feature :)

yes, your ptr to ptr cast example

RalfJ (Jul 16 2019 at 16:24, on Zulip):

but that byte slice contains the actual bytes from the allocation?

right. it does not contain the relocations.

RalfJ (Jul 16 2019 at 16:24, on Zulip):

there is also get_bytes, try understanding the difference between that and get_bytes_with_undef_and_ptr (the latter does fewer checks, expects the caller to make them)

Christian Poveda (Jul 16 2019 at 16:24, on Zulip):

but that byte slice contains the actual bytes from the allocation?

right. it does not contain the relocations.

So in the case of a pointer allocation it just has the offset

RalfJ (Jul 16 2019 at 16:24, on Zulip):

yes

RalfJ (Jul 16 2019 at 16:25, on Zulip):

yes, your ptr to ptr cast example

great! try to understand where exactly in alloc.rs the error is raised

RalfJ (Jul 16 2019 at 16:25, on Zulip):

clearly that's code that has to change :D

Christian Poveda (Jul 16 2019 at 16:25, on Zulip):

hahahahaha

Christian Poveda (Jul 16 2019 at 16:27, on Zulip):

Ok I'm going to sync my fork with master and compile rustc to start messing with it

RalfJ (Jul 16 2019 at 16:28, on Zulip):

ah that reminds me I should check why miri does not build on master

RalfJ (Jul 16 2019 at 16:29, on Zulip):

maybe better sync with the version in https://github.com/rust-lang/miri/blob/master/rust-version :D

RalfJ (Jul 16 2019 at 16:39, on Zulip):

ah, it just affects 2 compile-fail tests. doesn't matter much.

Christian Poveda (Jul 16 2019 at 16:40, on Zulip):

oh

Christian Poveda (Jul 16 2019 at 16:40, on Zulip):

well anyway I had to do the changes over master eventually, so I believe i can live with those 2 tests failing

RalfJ (Jul 16 2019 at 16:53, on Zulip):

sure

Christian Poveda (Jul 17 2019 at 14:21, on Zulip):

I started looking how Allocations work. I believe the docs for https://doc.rust-lang.org/nightly/nightly-rustc/rustc/mir/interpret/struct.UndefMask.html could use some love (I could do that as a side task)

oli (Jul 17 2019 at 14:59, on Zulip):

:embarrassed:

Christian Poveda (Jul 17 2019 at 14:59, on Zulip):

Or maybe it's not so useful IDK

oli (Jul 17 2019 at 14:59, on Zulip):

UndefMask is magic

oli (Jul 17 2019 at 15:00, on Zulip):

It def needs some docs

Christian Poveda (Jul 17 2019 at 15:01, on Zulip):

I've been studying how to handle bitvectors to learn about compression so I'm in the mood right now hahaha

Andreas Molzer (Jul 17 2019 at 15:11, on Zulip):

Is there a reason to not use the BitSet from data_structures::bit_set? In any case, @RalfJ suggested changing the name to init_mask to remove unfortunate name collision with the meaning of undef (which is not uninit() at all) in llvm.

oli (Jul 17 2019 at 15:14, on Zulip):

Idk if BitSet is as optimized as UndefMask

Christian Poveda (Jul 17 2019 at 16:52, on Zulip):

I'll put in on my list anyway to do it later

RalfJ (Jul 17 2019 at 17:09, on Zulip):

@Andreas Molzer no idea. UndefMask predates me.^^

Christian Poveda (Jul 17 2019 at 17:11, on Zulip):

SoOooOOoo, I started reading why the ptr to ptr cast fails

Christian Poveda (Jul 17 2019 at 17:11, on Zulip):

well not the cast per se, the example

Christian Poveda (Jul 17 2019 at 17:12, on Zulip):

read_scalar fails because get_bytes_with_undef_and_ptr fails (just like @RalfJ said)

Christian Poveda (Jul 17 2019 at 17:14, on Zulip):

but at the end get_bytes_with_undef_and_ptr is just calling get_bytes_internal deciding to not fail when reading undefined or pointer bytes

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

except when there are relocations on the edges (And now I'm going to check that)

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

is this failing because reading the whole pointer in this case reads the relocation that its on the edge ?(Given that the vec just has one u64)

RalfJ (Jul 17 2019 at 17:18, on Zulip):

except when there are relocations on the edges (And now I'm going to check that)

yes

Christian Poveda (Jul 17 2019 at 17:21, on Zulip):

I suppose that the relocation for a ptr is indexed by 0, so that's the edge we are concerned about. It is possible that we should omit the check for the other edge?

EDIT: wait

Christian Poveda (Jul 17 2019 at 17:26, on Zulip):

what is check_relocation_edges doing here:

        self.check_relocations(cx, ptr.offset(size, cx)?, Size::ZERO)?;

why is offsetting ptr instead of changing the Size::ZERO to go to the other edge?

is the last argument the offset relative to the second argument?

Christian Poveda (Jul 18 2019 at 14:31, on Zulip):

Why is this check needed? I mean, why is it important to not have relocations on the edges of bytes?

Christian Poveda (Jul 18 2019 at 14:36, on Zulip):

I removed the check just to see what would happen and the example still fails in the same way. However, I am not sure where it is failing, here is the relevant part of the stack backtrace:

   0: backtrace::backtrace::trace::h52c09578e3caae4d (0x10a97e0b0)
             at /Users/christian/Workspace/contrib/rust/src/libcore/ptr/mod.rs:175
   1: backtrace::capture::Backtrace::new_unresolved::h60dd95aa9bbe6e2e (0x10a97bae8)
             at /Users/christian/.cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.29/src/capture.rs:176
   2: <rustc::mir::interpret::error::InterpErrorInfo as core::convert::From<rustc::mir::interpret::error::InterpError<u64>>>::from::h24fa728fd84817af (0x10a76905d)
             at /Users/christian/Workspace/contrib/rust/src/librustc/mir/interpret/error.rs:212
   3: rustc::mir::interpret::allocation::Allocation<Tag,Extra>::get_bytes_with_undef_and_ptr::h798a0727dc8638da (0x1076f1552)
             at /Users/christian/Workspace/contrib/rust/src/librustc/mir/interpret/mod.rs:5
   4: rustc::mir::interpret::allocation::Allocation<Tag,Extra>::read_scalar::ha577046c19e8cf4e (0x1076ed7b2)
             at /Users/christian/Workspace/contrib/rust/src/librustc/mir/interpret/allocation.rs:321
   5: rustc_mir::interpret::operand::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::try_read_immediate::hb3114fb259748d70 (0x1075e0694)

So, from 3 we know that get_bytes_with_undef_and_ptris still failing but then the trace goes into the error handling.

RalfJ (Jul 18 2019 at 14:40, on Zulip):

if you just remove the check, programs will just do utter garbage^^

RalfJ (Jul 18 2019 at 14:41, on Zulip):

they would read (parts of) the offset of a pointer and interpret that as raw bytes

RalfJ (Jul 18 2019 at 14:41, on Zulip):

how does that make any sense?

Christian Poveda (Jul 18 2019 at 14:41, on Zulip):

if you just remove the check, programs will just do utter garbage^^

I am just testing to see what happens

Christian Poveda (Jul 18 2019 at 14:43, on Zulip):

they would read (parts of) the offset of a pointer and interpret that as raw bytes

Wait, so the only purpose of checking for relocation in the edges is to check if the edges are just ptr offsets?

RalfJ (Jul 18 2019 at 15:33, on Zulip):

remember: the data stored in bytes is not the actual content of memory when there is a relocation!

RalfJ (Jul 18 2019 at 15:33, on Zulip):

it's just the offset part of the pointer

RalfJ (Jul 18 2019 at 15:33, on Zulip):

so the code makes sure that we never ever read stuff from bytes that has a relocation

Christian Poveda (Jul 18 2019 at 15:33, on Zulip):

but the only stuff from bytes with relocations should be pointer offsets, right?

RalfJ (Jul 18 2019 at 15:33, on Zulip):

and if there is a relocation at index 4 in the allocation, reading the byte at index 6 does overlap

Christian Poveda (Jul 18 2019 at 15:35, on Zulip):

Oh wait so when we are talking about edges, we are talking about the edges of a concrete region in bytes, not the whole vector

RalfJ (Jul 18 2019 at 15:48, on Zulip):

yes

RalfJ (Jul 18 2019 at 15:48, on Zulip):

we need pictures :/

RalfJ (Jul 18 2019 at 15:49, on Zulip):

so if an access reads from bytes 4..8, then we have to make sure that none of the bytes in there "have" a relocation

RalfJ (Jul 18 2019 at 15:49, on Zulip):

to "have" a relocation means that this byte is part of the offset of some pointer. this means that there exists a relocation at index I such that this byte is at index I+N where N in 0..8.

RalfJ (Jul 18 2019 at 15:49, on Zulip):

so e.g. if there is a relocation at index 4, the bytes 4..12 "have" a relocation

RalfJ (Jul 18 2019 at 15:50, on Zulip):

the way we check this is in two parts: think about each relocation being a "range", and the access being a "range"

RalfJ (Jul 18 2019 at 15:50, on Zulip):

we first check if the "edges" of the access are "inside" a relocation range

RalfJ (Jul 18 2019 at 15:50, on Zulip):

and then we check if there is a relocation entirely "inside" the "access range"

RalfJ (Jul 18 2019 at 15:51, on Zulip):

the reason that the latter case is different is that it actually is allowed when you read a pointer! the former case (at the edges) is never allowed currently

RalfJ (Jul 18 2019 at 15:51, on Zulip):

does this make sense?

Christian Poveda (Jul 18 2019 at 15:57, on Zulip):

we need pictures :/

Let me get my whiteboard

Christian Poveda (Jul 18 2019 at 16:07, on Zulip):

Oh ok

Christian Poveda (Jul 18 2019 at 16:10, on Zulip):

we first check if the "edges" of the access are "inside" a relocation range

This is an illegal access because this would mean you are reading some (but not all) of the bytes from the pointer offset.

and then we check if there is a relocation entirely "inside" the "access range"

This is fine because you are actually reading the whole offset and not just some part of it

RalfJ (Jul 19 2019 at 07:21, on Zulip):

we first check if the "edges" of the access are "inside" a relocation range

This is an illegal access because this would mean you are reading some (but not all) of the bytes from the pointer offset.

correct

RalfJ (Jul 19 2019 at 07:22, on Zulip):

This is fine because you are actually reading the whole offset and not just some part of it

This is fine only because read_scalar contains special code to handle relocations!

RalfJ (Jul 19 2019 at 07:22, on Zulip):

otherwise it would still return the offset as raw bytes, which is never correct

Christian Poveda (Jul 19 2019 at 07:32, on Zulip):

This is fine because you are actually reading the whole offset and not just some part of it

This is fine only because read_scalar contains special code to handle relocations!

So it creates an actual pointer from the bytes containing the offset

Christian Poveda (Jul 19 2019 at 07:32, on Zulip):

and the information in the relocations of course

Christian Poveda (Jul 19 2019 at 07:47, on Zulip):

So let me recapitulate a little

Christian Poveda (Jul 19 2019 at 07:51, on Zulip):

The ptr to ptr cast is a noop, actually the problem is the dereferencing of such pointer because that calls read_scalar.

Christian Poveda (Jul 19 2019 at 08:01, on Zulip):

read_scalar currently fails because get_bytes_internal fails and this last method checks that we access the bytes of a pointer in a proper way

RalfJ (Jul 19 2019 at 11:15, on Zulip):

not sure what you refer to with the "ptr to ptr cast" here, but otherwise -- yes

Christian Poveda (Jul 19 2019 at 14:06, on Zulip):

not sure what you refer to with the "ptr to ptr cast" here, but otherwise -- yes

Oh I mean, in your example you are casting a pointer to a pointer.

Christian Poveda (Jul 19 2019 at 14:15, on Zulip):

I believe there is something else is failing because when I tried to disable the edges (https://github.com/rust-lang/rust/blob/master/src/librustc/mir/interpret/allocation.rs#L166) check just to see what would happen when I ran your example, I got an error in get_bytes_with_undef_and_ptr anyway

Christian Poveda (Jul 19 2019 at 15:06, on Zulip):

There is just one different check done and it is the AllocationExtra check, but that's just related to stacked borrows, right?

RalfJ (Jul 20 2019 at 12:23, on Zulip):

Oh I mean, in your example you are casting a pointer to a pointer.

ah, but that's just to be able to do a 1-byte access that loads part of a pointer value

RalfJ (Jul 20 2019 at 12:24, on Zulip):

I believe there is something else is failing because when I tried to disable the edges (https://github.com/rust-lang/rust/blob/master/src/librustc/mir/interpret/allocation.rs#L166) check just to see what would happen when I ran your example, I got an error in get_bytes_with_undef_and_ptr anyway

probably the "interior relocation" check fired?

RalfJ (Jul 20 2019 at 12:24, on Zulip):

that one is at https://github.com/rust-lang/rust/blob/f69b07144a151f46aaee1b6230ba4160e9394562/src/librustc/mir/interpret/allocation.rs#L334

Christian Poveda (Jul 20 2019 at 17:36, on Zulip):

that one is at https://github.com/rust-lang/rust/blob/f69b07144a151f46aaee1b6230ba4160e9394562/src/librustc/mir/interpret/allocation.rs#L334

But the error is still inside get_bytes_with_undef_and_ptr

Christian Poveda (Jul 20 2019 at 17:42, on Zulip):

In fact I don't know, let me retry

RalfJ (Jul 20 2019 at 20:47, on Zulip):

where exactly is it?

Christian Poveda (Jul 20 2019 at 23:13, on Zulip):

exactly where you said, It took a while to recompile

Christian Poveda (Jul 22 2019 at 16:06, on Zulip):

@RalfJ, I would like to ask you something. When we do the partial read, shouldn't we read the whole pointer and then do the proper handling of the partial read?

Christian Poveda (Jul 22 2019 at 16:07, on Zulip):

If I understood correctly, right now rust is trying to access partially the pointer and that is why the edges check is failing.

RalfJ (Jul 23 2019 at 09:08, on Zulip):

yeah, you will have to read the entireoffset at least, cast it to int, and then extract the correct sub-range from that

Christian Poveda (Jul 23 2019 at 14:38, on Zulip):

so I need to change get_bytes_with_undef_and_ptr? or should I change the parameters for read_scalar

RalfJ (Jul 23 2019 at 21:31, on Zulip):

TBH for this one I dont have a complete strategy in my mind, I thought I'd let you figure something out?
my gut feeling is: make get_bytes_with_undef_and_ptr not check for edges, carefully review all existing users and add the edge check there, and then the rest can be done locally in read_scalar

Christian Poveda (Jul 24 2019 at 14:26, on Zulip):

I could modify get_bytes_with_undef_and_ptr to have a parameter to decide if edges should be checked or not

RalfJ (Jul 24 2019 at 17:10, on Zulip):

what other places call that method? maybe they should just do the edge check themselves

RalfJ (Jul 24 2019 at 17:11, on Zulip):

maybe the entire edge-vs-interior distinction just does not make sense any more

Christian Poveda (Jul 24 2019 at 18:47, on Zulip):

what other places call that method? maybe they should just do the edge check themselves

I'm going to check this in a moment

Christian Poveda (Jul 24 2019 at 19:28, on Zulip):

We have two other places calling that method, so probably doing the check inside each caller is a better idea:
- https://github.com/rust-lang/rust/blob/27a6a304e2baaabca88059753f020377f2476978/src/librustc_mir/interpret/memory.rs#L811
- https://github.com/rust-lang/rust/blob/27a6a304e2baaabca88059753f020377f2476978/src/librustc/mir/interpret/allocation.rs#L263

RalfJ (Jul 24 2019 at 19:30, on Zulip):

hm... in principle at least copy should probably also support copying parts of a pointer. but the challenge will be to find a nice interface that makes all of that possible and not messy.

RalfJ (Jul 24 2019 at 19:31, on Zulip):

@oli eventually we probably want a type like Byte in https://www.ralfj.de/blog/2018/07/24/pointers-and-bytes.html ^^

Christian Poveda (Jul 24 2019 at 19:32, on Zulip):

I'm going to start by migrating the check outside the method and check that nothing blews up.

Christian Poveda (Jul 24 2019 at 19:32, on Zulip):

Does that sounds good to you?

RalfJ (Jul 24 2019 at 19:34, on Zulip):

sounds good, yes!

Christian Poveda (Jul 24 2019 at 19:44, on Zulip):

Ok great! I just did the changes and I'm running the tests

oli (Jul 24 2019 at 20:44, on Zulip):

Can't we just ptr to int and grab the bytes we need?

RalfJ (Jul 24 2019 at 21:26, on Zulip):

we dont have a Memory :/

oli (Jul 24 2019 at 21:27, on Zulip):

Oh, this is in Allocation?

oli (Jul 24 2019 at 21:29, on Zulip):

Hmm, yea any API that supports it would be a mess or inefficient in the "everything is defined bytes" situation, which is the commkn situation

RalfJ (Jul 25 2019 at 07:47, on Zulip):

Oh, this is in Allocation?

yeah, it's in read_scalar

RalfJ (Jul 25 2019 at 07:49, on Zulip):

I think for a non-hacky solution we probably first need an "intermediate representation" that can actually represent any single byte of memory. That's not how we store memory, but it can be how we access it. So get_bytes would yield an iterator (or take a closure that takes an iterator) of things that are either a u8, or Undef, or "byte n of a Pointer".
or is that overkill?

Andreas Molzer (Jul 25 2019 at 08:34, on Zulip):

Need some similar thing in llvm codegen as well when bytes etc are to become private. It currently iterates over the relocations and all bytes on its own.

RalfJ (Jul 25 2019 at 08:49, on Zulip):

it can use get_bytes_with_undef_and_ptr for the bytes

RalfJ (Jul 25 2019 at 08:49, on Zulip):

and maybe we can open read-only access to the relocations

RalfJ (Jul 25 2019 at 08:49, on Zulip):

I don't think it needs the "abstract byte" stuff

Christian Poveda (Jul 25 2019 at 15:43, on Zulip):

I was able to move the edges check outside get_bytes_with_bla_bla without blowing up the compiler to smithereens:
https://github.com/rust-lang/rust/compare/master...christianpoveda:partial-reads?expand=1

Christian Poveda (Jul 25 2019 at 15:58, on Zulip):

Now I am going to start changing read_scalar

Christian Poveda (Jul 29 2019 at 14:54, on Zulip):

@RalfJ, the size received in read_scalar is the number of bytes we want to read from the pointer? or the number of bytes we want to read from the pointee?

Christian Poveda (Jul 29 2019 at 15:11, on Zulip):

also I just saw https://github.com/rust-lang/miri/issues/841, should I freeze partial reads then?

RalfJ (Jul 29 2019 at 15:40, on Zulip):

@Christian Poveda I am confused by that question. it's the number of bytes to be read from memory.

RalfJ (Jul 29 2019 at 15:41, on Zulip):

also I just saw https://github.com/rust-lang/miri/issues/841, should I freeze partial reads then?

not sure, do you have a concrete plan to proceed? I am not sure yet how feasible my plan in that issue is

Christian Poveda (Jul 29 2019 at 15:42, on Zulip):

also I just saw https://github.com/rust-lang/miri/issues/841, should I freeze partial reads then?

not sure, do you have a concrete plan to proceed? I am not sure yet how feasible my plan in that issue is

Not yet, I am trying to see what happens if I disable the checks inside read_scalar

Christian Poveda (Jul 30 2019 at 15:23, on Zulip):

I think partial reads might be more than I am able to understand and fix right now.

RalfJ (Jul 30 2019 at 18:23, on Zulip):

I see. That's okay. You already did a lot for intptrcast :)

Christian Poveda (Jul 30 2019 at 20:38, on Zulip):

Is it there anything else that needs to be done?

RalfJ (Jul 30 2019 at 21:09, on Zulip):

Rust has almost 5k open issues, there's lots to be done. ;) There are "E-mentor" and "E-easy"/"E-medium" tags to find issues where people can guide you.

RalfJ (Jul 30 2019 at 21:11, on Zulip):

On the Miri side you could scroll through https://github.com/rust-lang/miri/issues and see if anything peeks your interest.
Good issues can probably be found in the A-shims category (https://github.com/rust-lang/miri/issues?q=is%3Aissue+is%3Aopen+label%3AA-shims); those tend to be fairly local and not interact with tons of stuff. Depend on what kind of experience you have elsehwere in the Rust ecosystem, you might like https://github.com/rust-lang/miri/issues/546. If you want something more open-ended where you will have to do your own research and there's nobody to tell you all the answers, maybe you'll like https://github.com/rust-lang/miri/issues/739.

Christian Poveda (Jul 31 2019 at 14:40, on Zulip):

I meant, is there anything else to do in intptrcast? What about those bullet points: https://github.com/rust-lang/miri/issues/224#issue-239164007 ?

Christian Poveda (Jul 31 2019 at 14:42, on Zulip):

I suppose " Exploit "real" pointer alignment for alignment checks, but warn when we do.
" is checked by your last comment. What about checking where else to use force_ptr/force_bits?

RalfJ (Jul 31 2019 at 14:59, on Zulip):

I suppose " Exploit real pointer alignment for alignment checks, but warn when we do. " is checked by your last comment.

no it's not. but this first requires coming up with a plan for https://github.com/rust-lang/miri/issues/797. (or just implementing some hacky first version that can later be rewritten when we have a plan.^^)

Christian Poveda (Jul 31 2019 at 14:59, on Zulip):

Ha, ok I would wait

RalfJ (Jul 31 2019 at 14:59, on Zulip):

What about checking where else to use force_ptr/force_bits?

That still needs to be done, but I think it needs someone who has enough experience to look at each and every use site of these methods and figure out which is the right method.

Christian Poveda (Jul 31 2019 at 15:00, on Zulip):

ok

RalfJ (Jul 31 2019 at 15:00, on Zulip):

if I end up having to tell you the answer for every call site, it's not really saving any time compared to me doing it myself, I am afraid. ;)

Christian Poveda (Jul 31 2019 at 15:00, on Zulip):

sure

RalfJ (Jul 31 2019 at 15:00, on Zulip):

also the way this looks will change with https://github.com/rust-lang/miri/issues/841

Christian Poveda (Jul 31 2019 at 15:03, on Zulip):

Well it seems it is time for me to move on :P

Christian Poveda (Jul 31 2019 at 15:04, on Zulip):

On the Miri side you could scroll through https://github.com/rust-lang/miri/issues and see if anything peeks your interest.
Good issues can probably be found in the A-shims category (https://github.com/rust-lang/miri/issues?q=is%3Aissue+is%3Aopen+label%3AA-shims); those tend to be fairly local and not interact with tons of stuff. Depend on what kind of experience you have elsehwere in the Rust ecosystem, you might like https://github.com/rust-lang/miri/issues/546. If you want something more open-ended where you will have to do your own research and there's nobody to tell you all the answers, maybe you'll like https://github.com/rust-lang/miri/issues/739.

I'll check something here later

RalfJ (Jul 31 2019 at 17:12, on Zulip):

Well it seems it is time for me to move on :P

thanks a lot for your help with intptrcast, this helped move Miri to a whole new level :)

Christian Poveda (Jul 31 2019 at 19:53, on Zulip):

Keep me informed if some new developments happen for intptrcast, I'll be more than happy to help :)

RalfJ (Jul 31 2019 at 21:12, on Zulip):

will do :)

Last update: Nov 15 2019 at 20:05UTC