Stream: t-lang/wg-unsafe-code-guidelines

Topic: hello


Welcome Bot (Aug 08 2018 at 18:14, on Zulip):

Welcome to #wg-unsafe-code-guidelines.

Description: Discussing about what Unsafe Code means. Or doesn't.

memoryruins (Aug 21 2018 at 19:34, on Zulip):

perhaps a question more for libs team. is there an existing guideline for defining certain types of PRs using unsafe that should require a fcp, such as public data structures in std? would it make sense to require group reviews for changes such as #52553 ? (opened issues that followed: #53529 #53564 #53566)

nikomatsakis (Aug 21 2018 at 20:46, on Zulip):

seems like a decent idea, but I'm not sure -- would definitely be something to ask the libs team

Jake Goulding (Aug 21 2018 at 21:53, on Zulip):

@memoryruins we have all the unsafe guidelines we need

Jake Goulding (Aug 21 2018 at 21:54, on Zulip):

Although technically, I reviewed that original PR somewhat, as did Simon. Simon is a smart programmer, I make no claims for myself.

Jake Goulding (Aug 21 2018 at 21:55, on Zulip):

I'd rather remove humans as much as possible. A combination of Miri + Valgrind + Proptest + Fuzzing would make me feel better.

nikomatsakis (Aug 21 2018 at 21:56, on Zulip):

@memoryruins my personal desire is to have a valgrind plugin that checks for UB; I can't say whether such a plugin would have caught those cases, as I'm not sure of the error... @RalfJ is looking into this, however (the plugin, I mean)

Jake Goulding (Aug 21 2018 at 21:56, on Zulip):

I mean, some of the UB was already caught by Miri, just no one had ever run it before

Jake Goulding (Aug 21 2018 at 21:56, on Zulip):

@nikomatsakis what would such a plugin do that Valgrind doesn't already?

nikomatsakis (Aug 21 2018 at 21:57, on Zulip):

check Rust's aliasing rules

nikomatsakis (Aug 21 2018 at 21:57, on Zulip):

once we have them :)

Jake Goulding (Aug 21 2018 at 21:57, on Zulip):

Someone should start a team to construct those rules...

nikomatsakis (Aug 21 2018 at 21:57, on Zulip):

e.g., consider the add'l invariants described in the stacked borrows blog post

Jake Goulding (Aug 21 2018 at 21:58, on Zulip):

Why part of Valgrind instead of Miri? So that it catches codegen issues as well?

Jake Goulding (Aug 21 2018 at 21:58, on Zulip):

I'm reading blog now in case it's explained there

nikomatsakis (Aug 21 2018 at 21:58, on Zulip):

yes and so it can e.g. check C code

nikomatsakis (Aug 21 2018 at 21:59, on Zulip):

i.e., you could just run all your tests

Jake Goulding (Aug 21 2018 at 22:00, on Zulip):

check C code

huh... does linked C code need to uphold Rust's rules?

nikomatsakis (Aug 21 2018 at 22:00, on Zulip):

of course

nikomatsakis (Aug 21 2018 at 22:00, on Zulip):

if e.g. it has a ptr from Rust

nikomatsakis (Aug 21 2018 at 22:01, on Zulip):

if there is some memory that C code allocates and uses but never "escapes" to Rust

nikomatsakis (Aug 21 2018 at 22:01, on Zulip):

then we don't care

nikomatsakis (Aug 21 2018 at 22:01, on Zulip):

and the rules would not add any add'l constraints on such memory beyond what valgrind already checks

Jake Goulding (Aug 21 2018 at 22:02, on Zulip):

OK, the amount of interop there makes more sense. I was thinking you were including completely distinct code.

Jake Goulding (Aug 21 2018 at 22:05, on Zulip):

An address in memory, and a tag used to store the time when the reference was created

Time to get those custom fat pointers working ;-)

memoryruins (Aug 22 2018 at 02:25, on Zulip):

@Jake Goulding lmao those photos. i agree with your sentiments for combination of checks

memoryruins (Aug 22 2018 at 02:32, on Zulip):

would be more consistent than only a group review

memoryruins (Aug 22 2018 at 02:32, on Zulip):

very keen to what ralf discovers while looking into the plugin

memoryruins (Aug 22 2018 at 02:32, on Zulip):

awesome that miri caught it too

RalfJ (Aug 22 2018 at 13:24, on Zulip):

miri caught sth else though

RalfJ (Aug 22 2018 at 13:24, on Zulip):

uninitialized data used in Debug

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

looking at https://play.rust-lang.org/?gist=7f32ef3123acb517dcfba377806f391f&version=stable&mode=debug&edition=2015, miri does not see anything... strange. or is that using a libstd without the broken VecDeque?

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

@Jake Goulding is there any way for me to see which nightly this miri is using?

RalfJ (Aug 22 2018 at 13:31, on Zulip):

ah yes, locally it finds the problem :D

Jake Goulding (Aug 22 2018 at 13:36, on Zulip):

Not trivially https://github.com/integer32llc/rust-playground/issues/300

Jake Goulding (Aug 22 2018 at 13:37, on Zulip):

You can look at the build status

Jake Goulding (Aug 22 2018 at 13:38, on Zulip):

Add see that error: `rust-src` component not found. Run `rustup component add rust-src`.

Jake Goulding (Aug 22 2018 at 13:38, on Zulip):

which doesn't make any damn sense

Jake Goulding (Aug 22 2018 at 13:38, on Zulip):

considering it was installed a few lines earlier

RalfJ (Aug 22 2018 at 13:40, on Zulip):

hm I see. well I noticed that for nightlies it shows the commit, but doesn't show the toolchain name either

RalfJ (Aug 22 2018 at 13:40, on Zulip):

of course, with toolchain name != version shown by rustc --version, this is also not easy

RalfJ (Aug 22 2018 at 13:41, on Zulip):

@Jake Goulding that's a strange error indeed... I'd expect a build failure, but it doesn't even try to build miri?

RalfJ (Aug 22 2018 at 13:42, on Zulip):

oh I think I know what is happening

RalfJ (Aug 22 2018 at 13:42, on Zulip):

this is probably related to https://github.com/solson/miri/blob/master/rust-toolchain

Jake Goulding (Aug 22 2018 at 13:42, on Zulip):

I've disabled build failures (there's a || true) because I don't want a failure in a tool to prevent deploying the UI. I should change it so it shows up as red in travis though

RalfJ (Aug 22 2018 at 13:43, on Zulip):

I am still not sure what the best solution is there

RalfJ (Aug 22 2018 at 13:43, on Zulip):

I'd like someone downloading miri to know a nightly version they can use

RalfJ (Aug 22 2018 at 13:43, on Zulip):

so currently we put that into rust-toolchain, which means that gets used automatically by rustup

RalfJ (Aug 22 2018 at 13:44, on Zulip):

people that absolutely want to use the latest nightly are supposed to do rustup override set nightly

Jake Goulding (Aug 22 2018 at 13:44, on Zulip):

mmm. My build should /probably/ ignore that

RalfJ (Aug 22 2018 at 13:44, on Zulip):

yeah I think it should do that override

RalfJ (Aug 22 2018 at 13:44, on Zulip):

currently it doesnt find rust-src for the nightly-XXXX toolchain, hence the error

RalfJ (Aug 22 2018 at 13:45, on Zulip):

that error would remain even when we get miri green again

RalfJ (Aug 22 2018 at 13:45, on Zulip):

should change it so it shows up as red in travis though

I think travis has an option to ignore failing some tasks

RalfJ (Aug 22 2018 at 13:46, on Zulip):

so then you could remove the || true such that travis knows the actual state, but still keep deploying the rest

Jake Goulding (Aug 22 2018 at 13:46, on Zulip):

it does; i just never set it up (or it wasn't present when I set it up x years ago)

Jake Goulding (Aug 22 2018 at 13:46, on Zulip):

https://github.com/integer32llc/rust-playground/issues/387

Jake Goulding (Aug 22 2018 at 13:52, on Zulip):

The trick is that the playground doesn't use a build matrix directly. It uses build stages, which doesn't obviously have a fail is ok config

RalfJ (Aug 22 2018 at 13:53, on Zulip):

hm yeah

RalfJ (Aug 22 2018 at 13:54, on Zulip):

I tried to match the syntax but that doesn't seem right^^

RalfJ (Aug 22 2018 at 13:54, on Zulip):

of course they dont have a working syntax checker :/

Jake Goulding (Aug 22 2018 at 13:54, on Zulip):

https://github.com/travis-ci/travis-ci/issues/7789

RalfJ (Aug 22 2018 at 13:54, on Zulip):

I'll just remove the 2nd commit

Jake Goulding (Aug 22 2018 at 13:55, on Zulip):

oh, you cant fix that anyway. PRs don't build containers ;-)

RalfJ (Aug 22 2018 at 13:55, on Zulip):

dang^^

RalfJ (Aug 22 2018 at 13:55, on Zulip):

wow so I have to repeat the allowed-to-fail config?

RalfJ (Aug 22 2018 at 13:55, on Zulip):

wtf

RalfJ (Aug 22 2018 at 13:58, on Zulip):

So I guess that would be the right syntax?

Jake Goulding (Aug 22 2018 at 13:58, on Zulip):

When I originally set it up, there definitely wasn't a way to skip build stages, which there is now

Jake Goulding (Aug 22 2018 at 13:58, on Zulip):

so that's another thing I wanted to revisit

RalfJ (Aug 22 2018 at 13:58, on Zulip):

https://github.com/integer32llc/rust-playground/pull/388/commits/69347ef8aa022ae377beefe1dd3a79feff512ac3

RalfJ (Aug 22 2018 at 14:00, on Zulip):

hm not sure if I have to repeat the script part

RalfJ (Aug 22 2018 at 14:01, on Zulip):

but how else is it going to match?^^

RalfJ (Aug 22 2018 at 14:01, on Zulip):

why couldnt they just add a "allow_fail: true" to the individual job yml?!?

Jake Goulding (Aug 22 2018 at 14:05, on Zulip):

The travis YML is a prime example of organically grown software with many opposed requirements

RalfJ (Aug 22 2018 at 14:06, on Zulip):

and no clear design

RalfJ (Aug 22 2018 at 14:07, on Zulip):

OTOH, at least they have matrices, which I wish gitlab CI had as well...

RalfJ (Aug 22 2018 at 14:09, on Zulip):

oh dang... yaml why are you so annoying

RalfJ (Aug 22 2018 at 14:09, on Zulip):

turns out false is not the program false, but parsed as boolean

alercah (Aug 23 2018 at 16:36, on Zulip):

Why Valgrind and not a sanitizer?

nikomatsakis (Aug 23 2018 at 17:30, on Zulip):

I don't really draw a strong line between those two, actually. :P But I do want something that can enforce the stacked borrows rules, which requires some "shadow memory" and so forth.

Last update: Nov 19 2019 at 18:00UTC