Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core::intrinsics::volatile_store ignores platform calling conventions. #29663

Closed
DiamondLovesYou opened this issue Nov 6, 2015 · 2 comments
Closed
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️

Comments

@DiamondLovesYou
Copy link
Contributor

#![feature(core_intrinsics)]

struct A<T> {
    x: T,
    y: T,
    z: T,
    w: T,
}

fn f<T>(dummy: T) -> T {
    unsafe {
        let mut dest: T = std::mem::uninitialized();
        std::intrinsics::volatile_store(&mut dest as *mut T, dummy);
        std::intrinsics::volatile_load(&dest as *const T)
    }
}

pub fn main() {
    // Either
    let (_, _, _, _) = f((1i32, 2i32, 3i32, 4i32));
    // or
    let _ = f(A {
        x: 1i32,
        y: 2,
        z: 3,
        w: 4,
    });
}

Compiling with rustc (rustc 1.6.0-dev (b14dc5bc1 2015-11-06)) gets:

Stored value type does not match pointer operand type!
  store volatile { i32, i32, i32, i32 }* %arg, { i32, i32, i32, i32 }* %dest, align 4
 { i32, i32, i32, i32 }LLVM ERROR: Broken function found, compilation aborted!

Or:

Stored value type does not match pointer operand type!
  store volatile %"A<i32>"* %arg, %"A<i32>"* %dest, align 4
 %"A<i32>" = type { i32, i32, i32, i32 }LLVM ERROR: Broken function found, compilation aborted!

It looks to me that volatile_store's second argument's value is used without looking at platform ABI requirements (ie using the load).

volatile_load might also be affected.

Also, LLVM error formatting weirdness.

@huonw huonw added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ labels Nov 6, 2015
@oli-obk
Copy link
Contributor

oli-obk commented Mar 8, 2016

minimal example:

#![feature(volatile)]

#[repr(C)]
struct X(u64, u64);

fn main() {
    let mut x = X(0, 0);
    unsafe { std::ptr::write_volatile(&mut x, X(5, 6)) };
}

on 32-bit systems using a 64-bit type also works. Anything smaller than the word size does not give an error.

@Amanieu
Copy link
Member

Amanieu commented Mar 8, 2016

A quick fix would be to make volatile_read and volatile_write use std::intrinsics::volatile_copy_nonoverlapping_memory. However the downside is that this intrinsic will force both a volatile load and store to perform the copy.

It seems that Clang will use a simple volatile load/store for primitive types but will fall back to a volatile memcpy for struct types.

bors added a commit that referenced this issue Mar 14, 2016
Fix LLVM assert with write_volatile

Fixes #29663
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️
Projects
None yet
Development

No branches or pull requests

4 participants