Static string is not borrowed correctly when used multiple times

[LegNeato]

(note, I probably don't know c2rust enough to correctly describe the problem)

When transpiling the c library h3, there exists a static string (in a macro?):

https://github.com/uber/h3/blob/139e5bff48ec16e5af745136d514a5b104869427/src/apps/testapps/testH3CellAreaExhaustive.c#L53

Which is transpiled like so:

let mut pos: [libc::c_char; 49] = *::core::mem::transmute::<&[u8; 49], &mut [libc::c_char; 49]>(
        b"distance between cell centers should be positive\0",
    );

I don't believe it should be mutable as the string is never changed. The variable is later referenced multiple times like this:

pos.as_mut_ptr()

which throws a borrow checker error. I was able to fix the issue by just inlining the static string at the pos.as_mut_ptr() callsite.

Code in LegNeato/unsafe-h3lib@386d0f4#r89264842 (note, takes a bit to load)

[kkysen]

I think this a transpiler bug. I'm also unsure how we should be properly handling this coming from an array (e.x. [libc::c_char; 49]). Arrays (and slices, any non-primitives) cannot be as casted to a pointer. This leaves us with slice::as_ptr and slice::as_mut_ptr.

as_mut_ptr, the current behavior, is not viable as it takes &mut self, which is why multiple active calls cannot be done, or else there will be multiple &muts on the array (derefed to a slice), which is why there are borrow checker errors.

as_ptr, the other option, says in its documentation that

The caller must also ensure that the memory the pointer (non-transitively) points to is never written to (except inside an UnsafeCell) using this pointer or any pointer derived from it. If you need to mutate the contents of the slice, use as_mut_ptr.

This works when the mutable pointer is not actually written to, as I believe is the case in your use case, but it definitely could be written to, and according to my interpretation of this documentation, that would be UB, as it's not done through an UnsafeCell. If anyone knows more about unsafety and *mut pointers, please chime in, as otherwise, I'm unsure how to solve this issue, but it also seems quite basic. Perhaps the solution would be to, where .as_mut_ptr() calls share a scope and thus conflict with the borrow checker, re-use the previous .as_mut_ptr() for the next. I'm unsure how big of a change doing that would be in the transpiler, but this seems like a significant transpiler bug.

Also, I'm unsure why we're using transmute here to just convert from u8 to libc::c_char, but that's another issue.

[LegNeato]

It sounds like mut_ptr is needed to uphold potential invariants and the reason why this code errors is the way the mut_ptr is used. Perhaps improving the logic at the usage location is the way to go? Can we detect when you need to clone or inline at the usage location? Is it even safe to clone?

[kkysen]

Inlining and cloning shouldn't be necessary. I'm thinking what should be done is to create the mutable array as the backing store, and then immediately shadow it as a mutable pointer, which is then used for all use-sites. So something like this:

let mut pos = pos.as_mut_ptr();

That more closely matches C's array-to-pointer decaying and gets us immediately back to pointer land in Rust, vs. reference land, which the array is in.

I also noticed that the generated code is UB:

let mut pos: [libc::c_char; 49] = *::core::mem::transmute::<&[u8; 49], &mut [libc::c_char; 49]>(
        b"distance between cell centers should be positive\0",
    );

&T to &mut T transmutes are UB. (This is part of the reason why I don't think we should use transmute where we can help it. from_raw_parts_mut is better.) If pos is written to, that writes to a string literal, which I believe is placed in read-only data, and so a write would probably segfault. This part might be worth a separate issue. I think this would fix it:

let mut pos: [std::ffi::c_char; 49] = unsafe {
        ::core::mem::transmute::<[u8; 49], [std::ffi::c_char; 49]>(
            *b"distance between cell centers should be positive\0",
        )
    };