AtomicPtr wrapping_add using fetch_ptr_add not working

[matthri]

Hi,

I am trying to create an AtomicPtr to an array and increment the pointer to the array elements with a wrap around if the end of the array is reached.

For a non atomic pointer the following code works as expected

        let mut array = [1, 2];
        let ptr = array.as_mut_ptr();
        assert_eq!(unsafe { *ptr }, 1);
        let ptr = ptr.wrapping_add(1);
        assert_eq!(unsafe { *ptr }, 2);
        let ptr = ptr.wrapping_add(1);
        assert_eq!(unsafe { *ptr }, 1); // <- points again to the first element

But when I am using an atomic pointer like this it does not wrap around and returns arbitrary values since it reads uninitialized memory.

        let mut array = [1, 2];
        let ptr = AtomicPtr::new(array.as_mut_ptr());
        assert_eq!(unsafe { *ptr.load(Ordering::SeqCst) }, 1);
        ptr.fetch_ptr_add(1, Ordering::SeqCst);
        assert_eq!(unsafe { *ptr.load(Ordering::SeqCst) }, 2); // <- Correctly points to the second element after the add
        ptr.fetch_ptr_add(1, Ordering::SeqCst);
        assert_eq!(unsafe { *ptr.load(Ordering::SeqCst) }, 1); // <- Fails, no wrap around

I assumed this to work, since the documentation for the fetch_ptr_add says:

Offsets the pointer's address by adding val (in units of T), returning the previous pointer.

This is equivalent to using [wrapping_add] to atomically perform the equivalent of ptr = ptr.wrapping_add(val);

Maybe someone could point me in the right direction , many thanks in advance for our help

[paramagnetic]

That's not what "wrapping" means. It means wrapping around the end of the address space. Your code simply attempts to read one past the end of the array, which is UB.

Just index the array with an index modulo its length:

let array = [1, 2];
let mut i = 0;
assert_eq!(array[i], 1);
i = (i + 1) % array.len();
assert_eq!(array[i], 2);
i = (i + 1) % array.len();
assert_eq!(array[i], 1);

[matthri]

Ok that makes sense thank you.

Just for my understanding why does it seem to be working for the non atomic pointer?

[paramagnetic]

Because it's UB. It can do anything. Anything includes the expected behavior, or wiping your hard drive.

---

By the way, when using APIs and assuming their behavior, first think about whether what you want is at all possible. (Better yet, read the documentation!)

Your version of wrapping_add is impossible. The pointer simply doesn't contain length information (it's not a fat pointer to a slice). How could it know when to wrap around?

[farnz]

@matthri

For a non atomic pointer the following code works as expected

This is just by chance, since the code you've given exhibits UB. When I ask Miri to interpret it in the Rust Playground, Miri (under the Tools menu) detects that you've written bad unsafe code:

error: Undefined Behavior: memory access failed: alloc1106 has size 8, so pointer to 4 bytes starting at offset 8 is out-of-bounds
 --> src/main.rs:8:25
  |
8 |     assert_eq!(unsafe { *ptr }, 1); // <- points again to the first element
  |                         ^^^^ memory access failed: alloc1106 has size 8, so pointer to 4 bytes starting at offset 8 is out-of-bounds
  |
  = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
  = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
help: alloc1106 was allocated here:
 --> src/main.rs:2:9
  |
2 |     let mut array = [1, 2];
  |         ^^^^^^^^^
  = note: BACKTRACE (of the first span):
  = note: inside `main` at src/main.rs:8:25: 8:29

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

And, indeed, I can show that it's UB that's making this appear to "work" by changing the last assert_eq. Now that it's checking against 42, instead of against 1, the assert doesn't fire in Release builds, but in Debug builds, it sometimes does, and says that the value is not 1, but some random number.

[matthri]

Thanks again for your explanations @farnz @paramagnetic it's a lot clearer now and I understand the problem.

In fact, I read the documentation but still messed it up badly due to my wrong assumption.

[farnz]

As a useful rule of thumb, run Miri (cargo +nightly miri run or cargo +nightly miri test to run it locally) against any unsafe code you write, and before assuming that it's doing the "right" thing, ensure that Miri is completely happy with your unsafe code.

Miri is not perfect, not least because some of the rules (Stacked Borrows, Tree Borrows) that it checks against are experimental, but unless you really, really, really understand what you're doing, it's more likely that Miri's complaints are the result of you writing a buggy piece of unsafe code than a limitation of Miri.