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

PowerPC altivec intrinsics are unsound due to subnormal flushing #129886

Open
beetrees opened this issue Sep 2, 2024 · 3 comments
Open

PowerPC altivec intrinsics are unsound due to subnormal flushing #129886

beetrees opened this issue Sep 2, 2024 · 3 comments
Labels
A-floating-point Area: Floating point numbers and arithmetic C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness O-PowerPC Target: PowerPC processors P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@beetrees
Copy link
Contributor

beetrees commented Sep 2, 2024

I tried this code, compiled on a PowerPC target with -O -Ctarget-feature=+altivec (-O -Ctarget-feature=-vsx is required instead if the chosen PowerPC target uses a default target CPU of pwr7 or later):

#![feature(stdarch_powerpc, powerpc_target_feature)]

#[cfg(target_arch = "powerpc64")]
use std::arch::powerpc64::{vector_float, vec_add};
#[cfg(target_arch = "powerpc")]
use std::arch::powerpc::{vector_float, vec_add};
use std::mem::transmute;

#[inline(never)]
fn print_vals(x: vector_float, i: usize, vals_i: u32) {
    println!("x={x:?} i={i} vals[i]={vals_i}");
}

const ZERO: vector_float = unsafe { transmute([0, 0, 0, 0]) };
const INC: vector_float = unsafe { transmute([f32::MIN_POSITIVE / 128.0, f32::MIN_POSITIVE / 128.0, f32::MIN_POSITIVE / 128.0, f32::MIN_POSITIVE / 128.0]) };
const TARGET: u128 = unsafe { transmute([f32::MIN_POSITIVE, f32::MIN_POSITIVE, f32::MIN_POSITIVE, f32::MIN_POSITIVE]) };

#[inline(never)]
#[target_feature(enable = "altivec")]
pub unsafe fn evil(vals: &[u32; 300]) {
    let mut x: vector_float = ZERO;
    let mut i: usize = 0;
    while unsafe { transmute::<vector_float, u128>(x) } != TARGET {
        print_vals(x, i, vals[i]);
        x = unsafe { vec_add(x, INC) };
        x = unsafe { vec_add(x, INC) };
        i += 2;
    }
    dbg!(unsafe { transmute::<vector_float, u128>(x) });
}

pub fn main() {
	let mut vals: [u32; 300] = [0; 300];
    for i in 0..300 { vals[i as usize] = i; }
    unsafe { evil(&vals) };
}

#[cfg(not(target_feature = "altivec"))]
compile_error!("-Ctarget-feature=+altivec required");

#[cfg(target_feature = "vsx")]
compile_error!("-Ctarget-feature=-vsx required");

I expected to see this happen: The program does not segfault.

Instead, this happened: The program segfaults as LLVM optimisations presume that subnormals are not flushed to zero, whereas the altivec instructions flush subnormals to zero. This only occurs when the vsx feature is not enabled; the vsx instructions do not flush subnormals and therefore do not have this issue.

This is the PowerPC equivalent of #129880.

Meta

rustc --version --verbose:

rustc 1.82.0-nightly (a7399ba69 2024-08-31)
binary: rustc
commit-hash: a7399ba69d37b019677a9c47fe89ceb8dd82db2d
commit-date: 2024-08-31
host: x86_64-unknown-linux-gnu
release: 1.82.0-nightly
LLVM version: 19.1.0
@beetrees beetrees added the C-bug Category: This is a bug. label Sep 2, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 2, 2024
@beetrees
Copy link
Contributor Author

beetrees commented Sep 2, 2024

@rustbot label +O-PowerPC +I-unsound +A-floating-point

cc @RalfJung

@rustbot rustbot added A-floating-point Area: Floating point numbers and arithmetic I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness O-PowerPC Target: PowerPC processors I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Sep 2, 2024
@RalfJung
Copy link
Member

RalfJung commented Sep 2, 2024

Cc @nikic

@lolbinarycat lolbinarycat added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 3, 2024
@apiraino
Copy link
Contributor

apiraino commented Sep 3, 2024

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Sep 3, 2024
@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-floating-point Area: Floating point numbers and arithmetic C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness O-PowerPC Target: PowerPC processors P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants