<re_irc>
<@adamgreig:matrix.org> > This does not mean that all APIs that might allow /proc/self/mem to be opened and read from or written must be unsafe. Rust’s safety guarantees only cover what the program itself can do, and not what entities outside the program can do to it. /proc/self/mem is considered to be such an external entity, along with debugging interfaces, and people with physical access to the hardware. This is true even in cases where the...
<re_irc>
... program is controlling the external entity.
<re_irc>
<@adamgreig:matrix.org> nothing new really
<re_irc>
<@jamesmunns:beeper.com> I dunno, my $0.02 is that it makes sense for open() to be safe, because you have to use it in an unusual way to get dma-ish behavior, whereas with dma, that's the ONLY thing you can do with that interface
<re_irc>
<@jamesmunns:beeper.com> that being said that being said, I also totally see "pacs/rals/pals" should always be unsafe (like libc) and it's your job as a hal (or std) to make it safe
<re_irc>
<@adamgreig:matrix.org> no one's seriously saying we should just make dma safe, the opinion is more make everything unsafe, which is a bit funny given the rest of the discussion but still
<re_irc>
<@adamgreig:matrix.org> but so much energy has been spent on working out how to make a safe dma abstraction, so it's funny to think we could just... not
<re_irc>
<@adamgreig:matrix.org> put a little note in the docstring "btw don't use this to change memory in a way that's ub, otherwise you'll get ub, ok?"
<re_irc>
<@jamesmunns:beeper.com> I mean it's at different levels I suppose?
<re_irc>
<@jamesmunns:beeper.com> if the pac exposed dma and it was unsafe and it just said "use it right plz", that's cool with me
<re_irc>
<@jamesmunns:beeper.com> but I think that's a pretty lame guarantee for a hal. even if std does it sorta in some cases.
<re_irc>
<@adamgreig:matrix.org> I'm saying it was _safe_ and said that, obviously if it was unsafe it's fine
<re_irc>
<@adamgreig:matrix.org> I think one compelling reason that pacs need to be unsafe (or use owned singletons) is the race condition business to what's basically shared memory
<re_irc>
<@adamgreig:matrix.org> even if you discount all the "writing particular values might get you ub due to external things modifying memory" argument s (dma, basically), you still have read-modify-write races on shared memory, so
<re_irc>
<@jamesmunns:beeper.com> like, IF there was some I2C bus that you COULD address an internal flash (or let's say your main XIP flash!) and cause it to do UB things, like erase the whole flash you are XIPing out of, I wouldn't say "make embedded-hal::i2c unsafe"
<re_irc>
<@jamesmunns:beeper.com> same as why open(/proc/mem) isn't unsafe
<re_irc>
<@adamgreig:matrix.org> sure, same way we're not making gpio unsafe even though it COULD short a power rail or turn off the chip or trigger an external debugger to change memory or something
<re_irc>
<@adamgreig:matrix.org> I take your argument that "dma is different because it's specifically for modifying memory", but nevertheless I think the reasoning in the rust docs for proc/self/mem still applies
<re_irc>
<@adamgreig:matrix.org> just... maybe the reasoning in those docs isn't perfect
<re_irc>
<@adamgreig:matrix.org> (I don't think we should make all dma access just totally safe, to be clear)
<re_irc>
<@jamesmunns:beeper.com> docs are non-normative :D
<re_irc>
<@adamgreig:matrix.org> (just amused at the concept, given how much energy has been expended on this problem)
<re_irc>
<@adamgreig:matrix.org> is anything normative except what rustc actually does, which is allow it?
<re_irc>
<@jamesmunns:beeper.com> ehhhhhhhh
<re_irc>
<@adamgreig:matrix.org> call me when ferrocene is done :P
<re_irc>
<@adamgreig:matrix.org> "io::open() is unsafe now"
<re_irc>
<@jamesmunns:beeper.com> NOTHING is normative except rustc, tho that may change for the specific "distros" like ferrocene, which may make their docs normative (that doesn't transfer "upstream" tho)
<re_irc>
<@adamgreig:matrix.org> that is also why we don't really know what's UB and what's not
<re_irc>
<@jamesmunns:beeper.com> : could be, or you have an integration requirement that says "don't use it with /proc/mem", which is way more reasonable in safety critical than general usage
<re_irc>
<@jamesmunns:beeper.com> : agreed. it's up to hal writers, like std writers, to make safe abstractions for their environments.
<re_irc>
<@jamesmunns:beeper.com> in the same way "what might be safe on linux is racy on windows"
<re_irc>
<@jamesmunns:beeper.com> (or the other way around)
<re_irc>
<@jamesmunns:beeper.com> I'd _probably_ say you're better off forking postcard and using that version for msp430 targets, especially as this is an ephemeral bug that might be fixed in the future
<cr1901>
I somehow doubt they're gonna break the entirity of compiler-builtins for me
<cr1901>
no matter what, fixing this is a breaking change (except for possibly forcing linking against libgcc and hoping for the best)
<cr1901>
Anyways, I'll keep that in mind. God, I am pissed
<re_irc>
<@jamesmunns:beeper.com> if it does need to be a forever fix, I'd probably be somewhat willing to take an "experimental-msp430-errata" or similar feature flag with no stability guarantees
<re_irc>
<@jamesmunns:beeper.com> but honestly, you'd wanna get it working in a fork anyway first
<re_irc>
<@jamesmunns:beeper.com> I don't totally grok how many places that would need to be fixed in
<cr1901>
well, my proposed patch would work for all targets, it just would be ugly lmfao (call the compiler_builtins function directly just for the shift)
<cr1901>
I'll wait and see what happens
<re_irc>
<@jamesmunns:beeper.com> cr1901: _that_ I would probably not accept.
<re_irc>
<@jamesmunns:beeper.com> for context: shifts in varint encoding/decoding are very much hotloops in a lot of postcard
<re_irc>
<@jamesmunns:beeper.com> I really would not accept anything for all targets that could potentially make that wonky. If it needs to be all over the crate, and not like in one or two functions, then yeah, I'd probably say fork it.
<re_irc>
<@jamesmunns:beeper.com> (the postcard wire format is stable, so that should be fine if you are using postcard-msp430 on one target and normal postcard on another)
<cr1901>
Then I could use #[cfg(target_arch = "msp430")] blocks. But I'm not doing anything tonight
<cr1901>
I'm all msp430'd and Rust'd out
<re_irc>
<@jamesmunns:beeper.com> <3
<cr1901>
This shouldn't have taken a month+ to write.
<re_irc>
<@jamesmunns:beeper.com> the siren song of the yaks calls to all of us
<re_irc>
<@jannic:matrix.org> : Just because it would be in line with the /proc/self/mem handling, it doesn't become a good idea.
<re_irc>
What I take from this whole discussion is that there is no definite set of rules to follow, but we need to apply judgement and take details of the individual API and its use cases into account.
<re_irc>
Which on the one hand is bad because it's much more work than just following some rules, but on the other hand it's good because it opens the opportunity to design a more ergonomic API.
rardiol has joined #rust-embedded
IlPalazzo-ojiisa has joined #rust-embedded
<re_irc>
<@thejpster:matrix.org> Yes, but I hear chip vendors tell me they need consistency and would greatly appreciate a service of rules for unsafe on Embedded.
<re_irc>
<@thejpster:matrix.org> (Which implies that, chip vendors, plural, are talking to me about supporting Rust)
<re_irc>
<@mendelt:matrix.org> Hi, one of my coworkers ran into an issue in the heapless crate that could use some eyeballs https://github.com/japaric/heapless/issues/360 . Please point me to a better place to ping people about that if this is not the right spot
IlPalazzo-ojiisa has quit [Quit: Leaving.]
<re_irc>
<@jannic:matrix.org> I only had a quick look and am not familiar with the code, but as far as I understand it, this really is a bug and the fix looks correct.
<re_irc>
<@dirbaio:matrix.org> : no! will read now 👀
<re_irc>
the "?Leak" idea is not mine btw, it has been floating around since the original leakpocalypse I think
<re_irc>
<@dirbaio:matrix.org> this is a bit different though, it's "cannot drop", while for DMA we need just "can drop, cannot leak"
<re_irc>
<@adamgreig:matrix.org> yea
<re_irc>
<@adamgreig:matrix.org> in principle I think they'd suggest that core forget() wouldn't take a !Drop T, but I don't know if you'd somehow prohibit leaking via an Rc cycle or something like that
<re_irc>
<@dirbaio:matrix.org> yeah.. :S
<re_irc>
<@dirbaio:matrix.org> I think the only fix is to completely forbid putting "!Leak" types into Rc/Arc
<re_irc>
<@jamesmunns:beeper.com> I think "refcycle leaks" are actually okay, the failure mode is deadlock. mem::forget has a different failure mode.
<re_irc>
<@jamesmunns:beeper.com> the dma token _existing_ forever is fine. it _disappearing_ is not.
<re_irc>
<@dirbaio:matrix.org> no
<re_irc>
<@dirbaio:matrix.org> the issue is, if the thing driving DMA (like the future) borrows the buffers, and stops DMA on drop
<re_irc>
<@dirbaio:matrix.org> you can put the thing on a Rc cycle, so it never gets dropped
<re_irc>
<@dirbaio:matrix.org> but drop all the handles to the Rc, so that from the borrow checker's point of view, the thing no longer exists
<re_irc>
<@dirbaio:matrix.org> which clears the borrow on the buffer
<re_irc>
<@jamesmunns:beeper.com> wouldn't the buffer have to be "'static" to allow that?
<re_irc>
<@jamesmunns:beeper.com> totally possible I could be misunderstanding tho. most of my memory of the discussion is coming from "embedded-dma", not the more modern stuff you're doing with embassy.
<re_irc>
<@dirbaio:matrix.org> : yeah that's what "embedded-dma" does
<re_irc>
<@jamesmunns:beeper.com> hm yeah, my memory is faulty here :D
<re_irc>
<@dirbaio:matrix.org> something like "?Leak" or "?Drop" would allow to do it with non-static buffers soundly
<re_irc>
<@dirbaio:matrix.org> requiring static buffers is too unergonomic
<re_irc>
<@jamesmunns:beeper.com> Yeah, I feel like having a lifetime parameter on the "dma token" should be enough, and would prevent Rc loops as well? But I feel like that was something that was discussed as well. It might have been too unergonomic in non-async code
<re_irc>
<@jamesmunns:beeper.com> (like, the "token" should have the same lifetime of the buffer itself). I'm probably rehashing already-made arguments from years ago :p
<re_irc>
<@dngrs:matrix.org> ow, that bri gs back memories
dc740 has joined #rust-embedded
<re_irc>
<@dngrs:matrix.org> * brings
<re_irc>
<@dirbaio:matrix.org> you can't do that with lifetimes
<re_irc>
<@dirbaio:matrix.org> with Rc cycles you can delay drop forever
<re_irc>
<@jamesmunns:beeper.com> I know there's an issue somewhere that has all of this discussion. Don't need to go find that tho.
<re_irc>
<@dirbaio:matrix.org> even for "after" the lifetime on the thing expires
<re_irc>
<@jamesmunns:beeper.com> Yeah but if it's in an Rc (not a refcell) it's on the heap, yeah? so living forever is fine, other than the leaked memory
<re_irc>
<@jamesmunns:beeper.com> I was thinking more for stackful dma buffers where forgetting the token means dma never gets stopped, but the stack frame expires
<re_irc>
<@jamesmunns:beeper.com> again, I really don't mean to rehash old arguments, I can drop it here and admit you probably know/are more recently familiar way the hell more than me :D
<re_irc>
<@dngrs:matrix.org> 3 hard problems in computer science… caching things, DMA, and elegant state machines
<re_irc>
<@jamesmunns:beeper.com> at least off by one errors are solved :D
<re_irc>
<@dngrs:matrix.org> * ~3~4
<re_irc>
<@dngrs:matrix.org> updated my comment
<re_irc>
<@dirbaio:matrix.org> : the future gets to live forever in the Rc, but the future borrows the buffer, it doesn't contain it
<re_irc>
<@dirbaio:matrix.org> so the buffer could be on stack
emerent has quit [Ping timeout: 246 seconds]
emerent has joined #rust-embedded
<re_irc>
<@firefrommoonlight:matrix.org> Truly, we haf discovered the Filisofic Mercury
dc740 has quit [Remote host closed the connection]
bjc has quit [Ping timeout: 276 seconds]
IlPalazzo-ojiisa has joined #rust-embedded
bjc has joined #rust-embedded
<re_irc>
<@grantm11235:matrix.org> : That sort of race condition isn't UB (as long as you assume that volatile accesses are atomic (which everyone does))
<re_irc>
<@jamesmunns:beeper.com> like, one read is atomic, and one write is atomic, but if you get interrupted after the read, you could clobber whatever the interrupt touched
<re_irc>
<@grantm11235:matrix.org> You can definitely get bugs with concurrent RMW stuff, but it's not UB
<re_irc>
<@jamesmunns:beeper.com> sorry yeah, that'd be a data race, not UB.
<re_irc>
<@jamesmunns:beeper.com> (which could LEAD to UB, but yeah, being pedantic)
<re_irc>
<@thejpster:matrix.org> We need our own Leads to Lava guide
<re_irc>
<@grantm11235:matrix.org> (if you _really_ wanted to be pedantic, you would bring up the fact that volatile reads/writes aren't actually guaranteed to be atomic)
<re_irc>
<@jamesmunns:beeper.com> I think that might be more contentious to claim?
<re_irc>
<@jamesmunns:beeper.com> I thiiiiiiink volatile writes must be done in a non-torn fashion? but I can't quote a spec back to you
<re_irc>
<@grantm11235:matrix.org> > Just like in C, whether an operation is volatile has no bearing whatsoever on questions involving concurrent access from multiple threads. Volatile accesses behave exactly like non-atomic accesses in that regard. In particular, a race between a read_volatile and any write operation to the same location is undefined behavior.
<re_irc>
<@diondokter:matrix.org> I only heard that about cortex-m 32-bit words. I think a 64-bit read could be torn
<re_irc>
<@jamesmunns:beeper.com> e.g. a 32-bit volatile write must not be torn into 2x16-bit reads, even if that is possible, and I don't think it's allowed to use "write/read_volatle") to unaligned addresses
<re_irc>
<@jamesmunns:beeper.com> : oh, I think this is the difference between atomic and Atomic. Like: it may not have coherency with core::sync::atomic usage, but it is "atomic", in that it may not be broken into multiple read/writes.
<re_irc>
<@jamesmunns:beeper.com> I was referring to the latter, but you're totally right, volatile access is not guaranteed to be coherent with atomic orderings.
<re_irc>
<@jamesmunns:beeper.com> words are hard lmao
<re_irc>
<@grantm11235:matrix.org> It's definitely allowed to break up reads/writes in some cases, since you can use it with arbitrarily large types
<re_irc>
<@jamesmunns:beeper.com> lmao really?
bjc has quit [Ping timeout: 246 seconds]
<re_irc>
<@jamesmunns:beeper.com> I guess I've never tried, but that makes sense. I guess limit my claims to "of a size suitable for the architecture", so <= 32bit on embedded
<re_irc>
<@jamesmunns:beeper.com> you're right, but that's so wrong lol
<re_irc>
<@jamesmunns:beeper.com> "thanks, I hate it."
<re_irc>
<@diondokter:matrix.org> I thought volatile just meant something like: 'Even if the compiler thinks a read or write isn
<re_irc>
<@diondokter:matrix.org> * isn't required, with a volatile operation the compiler is forced to do it anyways
<re_irc>
<@diondokter:matrix.org> * anyways'
<re_irc>
<@diondokter:matrix.org> Afaik, it has nothing to do with atomicity
<re_irc>
<@jamesmunns:beeper.com> I dunno, I'm not gunna look it up. even the definition is cursed and has a lot of shady ambiguity from what I remember
<re_irc>
<@jamesmunns:beeper.com> and then what compilers actually do vs the spec is even shadier
<re_irc>
<@diondokter:matrix.org> Haha, I do remember not understanding in when I was using C
<re_irc>
<@jamesmunns:beeper.com> then how that fits into rust's memory model is even shadier
<re_irc>
<@grantm11235:matrix.org> If rustc/llvm started replacing our 32bit writes with 4x 8bit writes, we would all get very mad
<re_irc>
<@diondokter:matrix.org> * it
<re_irc>
<@AsumFace:matrix.org> In LLVM a "volatile" access can be relied on to not get split up _if it's legal on the target_
<re_irc>
<@diondokter:matrix.org> Hahahaha, it says this in the docs:
<re_irc>
> Rust does not currently have a rigorously and formally defined memory model, so the precise semantics of what “volatile” means here is subject to change over time.
<re_irc>
<@jamesmunns:beeper.com> (and in rust it does say src/dst must be properly aligned)
<re_irc>
<@jamesmunns:beeper.com> so writing a u32 to something 16-bit aligned is ub
<re_irc>
<@diondokter:matrix.org> Oh hey, that's actually pretty weird. You can't do a volatile unaligned read/write in Rust
<re_irc>
<@jamesmunns:beeper.com> yeah, it's kinda a silly request, because read/write tearing
<re_irc>
<@jamesmunns:beeper.com> that being said, we apparently can use it with larger-than-usize values, so who knows
<re_irc>
<@jamesmunns:beeper.com> ask silly questions, get silly answers I suppose.
<re_irc>
<@diondokter:matrix.org> Yeah, according to the docs the only two things that are guaranteed is that a volatile read is not elided or reordered with respect to other volatile operations.
<re_irc>
<@jamesmunns:beeper.com> I'm more and more of the opinion that if you want to do tricky/clever things, you should just write asm to do them. At least then the sematics and behavior will be clear.
<re_irc>
<@jamesmunns:beeper.com> its not portable, but neither is living in the grey area of the rust/c specs/memory models
<re_irc>
<@thejpster:matrix.org> : I have a horrible feeling I’m going to have to convince people to fix that
<re_irc>
<@thejpster:matrix.org> Or write down what my compiler does and tell people to buy that
bjc has joined #rust-embedded
<re_irc>
<@diondokter:matrix.org> : Seems to work nicely :P
<re_irc>
<@grantm11235:matrix.org> One promising proposal is for the compiler to view volatile reads/writes as blackbox functions with arbitrary side effects
<re_irc>
<@thejpster:matrix.org> I mean honestly you could argue it’s in my interest for upstream to remain fuzzy and ambiguous. But only for a narrow definition of interest.
<re_irc>
<@grantm11235:matrix.org> > = note: the `thumbv7em-none-eabihf` target may not be installed
<re_irc>
= help: consider downloading the target with `rustup target add thumbv7em-none-eabihf`
<NishanthMenon>
re_irc: gaah.... i am blind...
<re_irc>
<@jamesmunns:beeper.com> There's a lot of duplicates in that output there, but usually you see that error if you don't have the target like Grant said, or if you have a crate (or the lib dot rs in your project) that is missing "#![no_std]"
<re_irc>
<@jamesmunns:beeper.com> "rustup show" will tell you if you have those targets installed.
<re_irc>
<@jamesmunns:beeper.com> (the "installed targets" bit)
<re_irc>
<@thejpster:matrix.org> TIL about show
lehmrob has joined #rust-embedded
tetramelly has joined #rust-embedded
<cr1901>
>(as long as you assume that volatile accesses are atomic (which everyone does)) <-- I thought volatile accesses weren't atomic, but as long as the access is wrapped in a Cell, data races are not UB (just undesirable)
<cr1901>
oh wait, read fail
<cr1901>
yea, as long as the volatile access (single read or write) is atomic (less or equal to bus width), then torn reads/writes can't happen.
tetramelly has quit [Ping timeout: 260 seconds]
dc740 has joined #rust-embedded
tetramelly has joined #rust-embedded
bjc has quit [Remote host closed the connection]
IlPalazzo-ojiisa has quit [Remote host closed the connection]