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

Topic: miri error: stopping looking for borrow


Jake Goulding (Jan 02 2019 at 01:55, on Zulip):

@RalfJ Does this Miri error mean I'm doing the bad things?

error[E0080]: constant evaluation error: Stopping looking for borrow being accessed (Shr(None)) because of barrier (2013)
Jake Goulding (Jan 02 2019 at 01:57, on Zulip):

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=42581a2568aee144aa129ec2b02ca0ee

gnzlbg (Jan 02 2019 at 10:12, on Zulip):

This doesn't have the issue: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=de8998f70e7e8392adead15af45a12b6

gnzlbg (Jan 02 2019 at 10:12, on Zulip):

Basically in fake_foo_value you are creating a *mut from a *const and reading through it.

gnzlbg (Jan 02 2019 at 10:14, on Zulip):

And that *const is created from a & in FooBorrowed::value.

RalfJ (Jan 02 2019 at 15:00, on Zulip):

@Jake Goulding

Does this Miri error mean I'm doing the bad things?

yes

RalfJ (Jan 02 2019 at 15:13, on Zulip):

Basically in fake_foo_value you are creating a *mut from a *const and reading through it.

reading through it should be fine though, only writing should be a problem

RalfJ (Jan 02 2019 at 15:15, on Zulip):

or rather, should be a problem if shared references were involved

RalfJ (Jan 02 2019 at 15:15, on Zulip):

*const and *mut dont make any difference

RalfJ (Jan 02 2019 at 15:20, on Zulip):

note that c_void has size 1, so in general this is a somewhat dangerous pattern. &FooBorrowed actually asserts that there is some dereferencable memory there and gives rustc license to insert spurious reads.

RalfJ (Jan 02 2019 at 15:24, on Zulip):

oh but you are creating a mutable reference here!

RalfJ (Jan 02 2019 at 15:24, on Zulip):

that's like writing

RalfJ (Jan 02 2019 at 15:24, on Zulip):

and you cant write to this pointer because it was obtained from a shared reference (the &self in FooBorrowed::value)

Jake Goulding (Jan 02 2019 at 18:34, on Zulip):

@RalfJ I'd love an improved error message, but I have no concrete suggestion.

Jake Goulding (Jan 02 2019 at 18:39, on Zulip):

creating a *mut from a *const and reading through it.

oh but you are creating a mutable reference here!
that's like writing

Hmm. I didn't actually want a mutable reference there; I just blindly applied the same changes from _new and _free. It's a bit unfortunate that there's no lint for that case, but I can see why it'd be hard to have one.

Jake Goulding (Jan 02 2019 at 18:41, on Zulip):

Thank you, @RalfJ and @gnzlbg !

Jake Goulding (Jan 02 2019 at 18:43, on Zulip):

note that c_void has size 1, so in general this is a somewhat dangerous pattern. &FooBorrowed actually asserts that there is some dereferencable memory there and gives rustc license to insert spurious reads.

In the broader answer, I suggest that the better pattern is to use an extern type (once available); would that solve the problem here?

RalfJ (Jan 02 2019 at 19:15, on Zulip):

@RalfJ I'd love an improved error message, but I have no concrete suggestion.

diagnosis could generally need tons of improvements

RalfJ (Jan 02 2019 at 19:15, on Zulip):

I also have a few ideas

RalfJ (Jan 02 2019 at 19:15, on Zulip):

but no time to implement them

RalfJ (Jan 02 2019 at 19:16, on Zulip):

In the broader answer, I suggest that the better pattern is to use an extern type (once available); would that solve the problem here?

yes

Jake Goulding (Jan 02 2019 at 19:46, on Zulip):

Too bad we can't actually change c_void to be an extern type when available

RalfJ (Jan 02 2019 at 20:05, on Zulip):

yeah

Jake Goulding (Jan 02 2019 at 21:04, on Zulip):

Since c_void is 1 byte for LLVM reasons, will the same thing need to happen for extern types...

RalfJ (Jan 03 2019 at 10:43, on Zulip):

extern types model struct Foo;, while c_void models void in ptr types. so I don't think the problem applies to extern types as well.

gnzlbg (Jan 03 2019 at 14:20, on Zulip):

*const and *mut dont make any difference

ah yes, the problem is only that the code is calling as_mut instead of as_ref, and that creates a &mut T from a pointer derived from a &

Jake Goulding (Jan 04 2019 at 00:55, on Zulip):

@RalfJ I think I'm misunderstanding or missing your point. IIRC, c_void is a one-byte type to enable optimizations in LLVM. Presumably, the same thing will need to be done for extern types. However, I could see if that transformation happens at a lower level (codegen?) and user-written code could not take advantage of it.

RalfJ (Jan 04 2019 at 10:46, on Zulip):

@Jake Goulding AFAIK c_void is 1-byte for the sole reason that *mut c_void should be the same as void*

Jake Goulding (Jan 05 2019 at 03:06, on Zulip):

@RalfJ I'm not sure if we are agreeing or disagreeing or other. For context, I'm referring to this comment

RalfJ (Jan 05 2019 at 10:42, on Zulip):

I disagree with extern type having any of the same issues as c_void

RalfJ (Jan 05 2019 at 10:42, on Zulip):

c_void is defined such that *mut c_void can be used in Rust where void* is used in C

RalfJ (Jan 05 2019 at 10:43, on Zulip):

that's not at all what extern type is for -- so why would any part of that comment be relevant? I am confused.^^ in particular, extern type is going to be unsized in Rust, not have size 1.

Jake Goulding (Jan 05 2019 at 14:51, on Zulip):

that's not at all what extern type is for

Perhaps this is where my knowledge gap is. I expect to use void * when I want to have an opaque type that I don't know the exact type of. I expect to use an extern type when I have an opaque type with a more concrete/specific type.

I expect to use a pointer to an extern type in many of the cases where I currently would use *mut c_void.

RalfJ (Jan 05 2019 at 14:52, on Zulip):

the usual way (AFAIK) to handle opaque types in C is to use struct Foo; (note the missing field list), and then Foo*

RalfJ (Jan 05 2019 at 14:53, on Zulip):

void* isn't for opaque types, it's for unkown types -- basically, for generic code

RalfJ (Jan 05 2019 at 14:54, on Zulip):

both of these concepts (will) have their counterpart in Rust for FFI, these counterparts being extern { type Foo; } for struct Foo; with *mut Foo for Foo*; and *mut c_void for void*. The Rust side of FFI should then match what the C side of it does (in terms of picking opaque vs unknown types). But my FFI knowledge is very limited so maybe there are reasons to divert here?

RalfJ (Jan 05 2019 at 14:54, on Zulip):

neither of these (c_void and extern type) have any use outside FFI, AFAIK

Jake Goulding (Jan 05 2019 at 14:55, on Zulip):

The usual way (yeah, to model opaque types) now is an empty struct, but that's still a constructable type. An extern type is not. An empty struct is AFAIK not part of the C standard (but it is allowed by both gcc and clang)

RalfJ (Jan 05 2019 at 14:56, on Zulip):

the usual way to model opaque types, right? not to model void*?

RalfJ (Jan 05 2019 at 14:56, on Zulip):

struct Foo; is not an empty struct

RalfJ (Jan 05 2019 at 14:56, on Zulip):

its a forward declaration

RalfJ (Jan 05 2019 at 14:56, on Zulip):

and AFAIK that's completely standards compliant

RalfJ (Jan 05 2019 at 14:56, on Zulip):

so none of what I said above has anything to do with empty structs

Jake Goulding (Jan 05 2019 at 14:56, on Zulip):

struct Foo; is not an empty struct

Point.

Jake Goulding (Jan 05 2019 at 14:57, on Zulip):

it is in Rust, so I wasn't sure which you meant

RalfJ (Jan 05 2019 at 14:57, on Zulip):

fair. I was talking about the C side.

RalfJ (Jan 05 2019 at 14:58, on Zulip):

so the current situation is

C: void* <-> Rust: *mut c_void
C: sruct Foo; <-> Rust: https://doc.rust-lang.org/nightly/nomicon/ffi.html#representing-opaque-structs

RalfJ (Jan 05 2019 at 14:58, on Zulip):

and the intent is, on the Rust side, for extern type to replace https://doc.rust-lang.org/nightly/nomicon/ffi.html#representing-opaque-structs

Jake Goulding (Jan 05 2019 at 14:59, on Zulip):

I still think that people will use an extern type as a reification of a void * when they are "instantiating a generic C function"

Jake Goulding (Jan 05 2019 at 14:59, on Zulip):

although I guess in that case they would want to be able to poke into the struct, so opaque is useless to them

RalfJ (Jan 05 2019 at 15:00, on Zulip):

that's a mismatch between the C and Rust side of FFI. it might be valid in some cases (talk to an FFI expert, I am not one^^), and there might be reasons to do that (dito), but the direct translation of the C interface is as I described above

Jake Goulding (Jan 05 2019 at 15:01, on Zulip):

However, my original point was about lower down details, but I still haven't quite picked up what you mean on that.

RalfJ (Jan 05 2019 at 15:01, on Zulip):

what's the question that is still open?

Jake Goulding (Jan 05 2019 at 15:02, on Zulip):

Based on the comment I linked, c_void needs to have a size in order to enable optimizations inside of LLVM, but you've said that an extern type is unsized. Does that mean that such optimizations will be precluded from extern types?

Jake Goulding (Jan 05 2019 at 15:03, on Zulip):

Or, will an extern type just be "effectively unsized" in the Rust frontend, but always have a "size" of 1 when generating code so that LLVM can optimize

RalfJ (Jan 05 2019 at 15:05, on Zulip):

my reading of that comment (mentioning malloc/free) is that we need this to give our malloc/free the right types on the LLVM side

RalfJ (Jan 05 2019 at 15:05, on Zulip):

this is C FFI with functions that have void* in their type, so we must use the proper thing on the Rust side

RalfJ (Jan 05 2019 at 15:06, on Zulip):

so I dont think this is a concern forextern type. But I am guessing here, maybe bring this up in the tracking issue?

Jake Goulding (Jan 05 2019 at 15:07, on Zulip):

So you think the optimization is limited to those specific functions? That would be believable. That is, LLVM looks for *i8 malloc(void) and treats it specially, and at one point we did *ZeroSizedType malloc(void) and blew away the optimization

RalfJ (Jan 05 2019 at 15:07, on Zulip):

AFAIK our extern type will generate the exact same LLVM types as C's forward declarations, and hence should be optimized the same way

RalfJ (Jan 05 2019 at 15:07, on Zulip):

however I have no idea what these types are^^

Jake Goulding (Jan 05 2019 at 15:13, on Zulip):
extern {
    type nuffin_t;

    fn myalloc() -> *mut nuffin_t;
    fn myfree(_: *mut nuffin_t);
}
%"::nuffin_t" = type {}

declare %"::nuffin_t"* @myalloc() unnamed_addr #0
declare void @myfree(%"::nuffin_t"*) unnamed_addr #0
Jake Goulding (Jan 05 2019 at 15:13, on Zulip):

Now to figure out the C equivalent

Jake Goulding (Jan 05 2019 at 15:18, on Zulip):
struct nuffin_t;

struct nuffin_t *myalloc();
void myfree(struct nuffin_t*);
%struct.nuffin_t = type opaque

declare dso_local void @myfree(%struct.nuffin_t*) #1
declare dso_local %struct.nuffin_t* @myalloc(...) #1
Jake Goulding (Jan 05 2019 at 15:18, on Zulip):

so that seems like a bug to me.

RalfJ (Jan 05 2019 at 15:18, on Zulip):

yeah seems like we are using an empty struct instead of an opaque one, or so?

RalfJ (Jan 05 2019 at 15:18, on Zulip):

no idea^^

RalfJ (Jan 05 2019 at 15:19, on Zulip):

Cc @eddyb

Jake Goulding (Jan 05 2019 at 15:19, on Zulip):

@RalfJ haha, check it; right at the top in the tracking issue:

In std's source, it is mentioned that LLVM expects i8* for C's void*.
We'd need to continue to hack this for the two c_voids in std and libc.
But perhaps this should be done across-the-board for all extern types?
Somebody should check what Clang does.

RalfJ (Jan 05 2019 at 15:19, on Zulip):

;)

RalfJ (Jan 05 2019 at 15:20, on Zulip):

well you just checked what clang does

Last update: Nov 19 2019 at 19:10UTC