Welcome to #wg-unsafe-code-guidelines.
Description: Discussing about what Unsafe Code means. Or doesn't.
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)
seems like a decent idea, but I'm not sure -- would definitely be something to ask the libs team
@memoryruins we have all the unsafe guidelines we need
Although technically, I reviewed that original PR somewhat, as did Simon. Simon is a smart programmer, I make no claims for myself.
I'd rather remove humans as much as possible. A combination of Miri + Valgrind + Proptest + Fuzzing would make me feel better.
@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)
I mean, some of the UB was already caught by Miri, just no one had ever run it before
@nikomatsakis what would such a plugin do that Valgrind doesn't already?
check Rust's aliasing rules
once we have them :)
Someone should start a team to construct those rules...
e.g., consider the add'l invariants described in the stacked borrows blog post
Why part of Valgrind instead of Miri? So that it catches codegen issues as well?
I'm reading blog now in case it's explained there
yes and so it can e.g. check C code
i.e., you could just run all your tests
check C code
huh... does linked C code need to uphold Rust's rules?
if e.g. it has a ptr from Rust
if there is some memory that C code allocates and uses but never "escapes" to Rust
then we don't care
and the rules would not add any add'l constraints on such memory beyond what valgrind already checks
OK, the amount of interop there makes more sense. I was thinking you were including completely distinct code.
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 ;-)
@Jake Goulding lmao those photos. i agree with your sentiments for combination of checks
would be more consistent than only a group review
very keen to what ralf discovers while looking into the plugin
awesome that miri caught it too
miri caught sth else though
uninitialized data used in
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?
@Jake Goulding is there any way for me to see which nightly this miri is using?
ah yes, locally it finds the problem :D
You can look at the build status
Add see that
error: `rust-src` component not found. Run `rustup component add rust-src`.
which doesn't make any damn sense
considering it was installed a few lines earlier
hm I see. well I noticed that for nightlies it shows the commit, but doesn't show the toolchain name either
of course, with toolchain name != version shown by
rustc --version, this is also not easy
@Jake Goulding that's a strange error indeed... I'd expect a build failure, but it doesn't even try to build miri?
oh I think I know what is happening
this is probably related to https://github.com/solson/miri/blob/master/rust-toolchain
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
I am still not sure what the best solution is there
I'd like someone downloading miri to know a nightly version they can use
so currently we put that into rust-toolchain, which means that gets used automatically by rustup
people that absolutely want to use the latest nightly are supposed to do
rustup override set nightly
mmm. My build should /probably/ ignore that
yeah I think it should do that override
currently it doesnt find rust-src for the nightly-XXXX toolchain, hence the error
that error would remain even when we get miri green again
should change it so it shows up as red in travis though
I think travis has an option to ignore failing some tasks
so then you could remove the
|| true such that travis knows the actual state, but still keep deploying the rest
it does; i just never set it up (or it wasn't present when I set it up x years ago)
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
I tried to match the syntax but that doesn't seem right^^
of course they dont have a working syntax checker :/
I'll just remove the 2nd commit
oh, you cant fix that anyway. PRs don't build containers ;-)
wow so I have to repeat the allowed-to-fail config?
So I guess that would be the right syntax?
When I originally set it up, there definitely wasn't a way to skip build stages, which there is now
so that's another thing I wanted to revisit
hm not sure if I have to repeat the script part
but how else is it going to match?^^
why couldnt they just add a "allow_fail: true" to the individual job yml?!?
The travis YML is a prime example of organically grown software with many opposed requirements
and no clear design
OTOH, at least they have matrices, which I wish gitlab CI had as well...
oh dang... yaml why are you so annoying
false is not the program false, but parsed as boolean
Why Valgrind and not a sanitizer?
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.