Skip to content

transpile: Consider volatile reads a side effect #1978

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.

Sign up for GitHub

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

Merged
ahomescu merged 4 commits into from
Aug 27, 2026
Merged

transpile: Consider volatile reads a side effect #1978

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ed620d0
transpile: Add tests for unused volatile reads
Rua Aug 20, 2026
d913b16
transpile: Move `LValueToRValue` handling to `convert_cast`
Rua Aug 20, 2026
c175e45
transpile: Use `make_cast` in `LValueToRValue` cast handling
Rua Aug 20, 2026
fe9ed9f
transpile: Consider volatile reads a side effect
Rua Aug 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions c2rust-transpile/src/c_ast/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2565,6 +2565,13 @@ impl Qualifiers {
false => Mutability::Mutable,
}
}

pub(crate) fn not_volatile(self) -> Self {
Self {
is_volatile: false,
..self
}
}
}

/// Qualified type
Expand All @@ -2589,6 +2596,13 @@ impl CQualTypeId {
pub fn mutability(self) -> Mutability {
self.qualifiers.mutability()
}

pub(crate) fn not_volatile(self) -> Self {
Self {
qualifiers: self.qualifiers.not_volatile(),
..self
}
}
}

// TODO: these may be interesting, but I'm not sure if they fit here:
Expand Down
97 changes: 53 additions & 44 deletions c2rust-transpile/src/translator/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3923,15 +3923,61 @@ impl<'c> Translation<'c> {
opt_field_id: Option<CDeclId>,
is_explicit: bool,
) -> TranslationResult<WithStmts<Box<Expr>>> {
if matches!(
kind,
CastKind::IntegralToBoolean | CastKind::FloatingToBoolean | CastKind::PointerToBoolean
) {
return self.convert_condition(ctx, true, expr);
}
let source_ty = if let Some(func_decl) = self
.ast_context
.fn_declref_decl(expr)
.filter(|_| is_explicit)
{
// If we're casting a function, look for its declared ty to use as a more
// precise source type. The AST node's type will not preserve typedef arg types
// but the function's declaration will.
let kind_with_declared_args = self.ast_context.fn_decl_ty_with_declared_args(func_decl);
let func_ty = self.ast_context.type_for_kind(&kind_with_declared_args);
let func_ptr_ty = self
.ast_context
.type_for_kind(&CTypeKind::Pointer(CQualTypeId::new(func_ty)));

CQualTypeId::new(func_ptr_ty)
} else {
self.ast_context
.index_unwrap_parens(expr)
.kind
.get_qual_type()
.ok_or_else(|| format_err!("bad source type"))?
};
let target_ty = override_ty.unwrap_or(ty);

match kind {
CastKind::LValueToRValue => {
let val = if source_ty.qualifiers.is_volatile {
// If the expression is volatile and used as something that isn't an LValue,
// this constitutes a volatile read. A volatile read is a side effect, so it
// needs to be included even if the expression is unused.
let val = self
.convert_expr(ctx.used(), expr, None)?
.try_map(|val| self.volatile_read(val, source_ty))?;
self.convert_side_effects_expr(
ctx,
val,
"LValueToRValue value is not supposed to be used",
)
} else {
self.convert_expr(ctx, expr, None)?
};

// if the context wants a different type, add a cast
return self.make_cast(ctx, source_ty.not_volatile(), target_ty, val);
}

CastKind::IntegralToBoolean
| CastKind::FloatingToBoolean
| CastKind::PointerToBoolean => {
return self.convert_condition(ctx, true, expr);
}

_ => {}
}

// In general, if we are casting the result of an expression, then the inner
// expression should be translated to whatever type it normally would.
// But for some expression types, if we don't absolutely have to cast,
Expand Down Expand Up @@ -3968,29 +4014,6 @@ impl<'c> Translation<'c> {
return Ok(val);
}

let source_ty = if let Some(func_decl) = self
.ast_context
.fn_declref_decl(expr)
.filter(|_| is_explicit)
{
// If we're casting a function, look for its declared ty to use as a more
// precise source type. The AST node's type will not preserve typedef arg types
// but the function's declaration will.
let kind_with_declared_args = self.ast_context.fn_decl_ty_with_declared_args(func_decl);
let func_ty = self.ast_context.type_for_kind(&kind_with_declared_args);
let func_ptr_ty = self
.ast_context
.type_for_kind(&CTypeKind::Pointer(CQualTypeId::new(func_ty)));

CQualTypeId::new(func_ptr_ty)
} else {
self.ast_context
.index_unwrap_parens(expr)
.kind
.get_qual_type()
.ok_or_else(|| format_err!("bad source type"))?
};

self.make_cast_full(
ctx,
source_ty,
Expand Down Expand Up @@ -4206,21 +4229,7 @@ impl<'c> Translation<'c> {
}

CastKind::LValueToRValue => {
let mut val = if source_cty.qualifiers.is_volatile {
// If the expression is volatile and used as something that isn't an LValue,
// this constitutes a volatile read.
val.try_map(|val| self.volatile_read(val, target_cty))?
} else {
val
};

// if the context wants a different type, add a cast
if target_cty.ctype != source_cty.ctype {
let ty = self.convert_type(target_cty.ctype)?;
val = val.map(|val| mk().cast_expr(val, ty));
}

Ok(val)
panic!("LValueToRValue casts must be handled in convert_cast")
}

CastKind::ToVoid | CastKind::ConstCast => Ok(val),
Expand Down
4 changes: 1 addition & 3 deletions c2rust-transpile/tests/snapshots/snapshots__transpile@auto_type.c.2021.clang15.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ pub unsafe extern "C" fn foo() -> ::core::ffi::c_int {
let mut px: *mut ::core::ffi::c_int = &raw mut x;
let mut sx: ::core::ffi::c_ulong =
::core::mem::size_of::<::core::ffi::c_int>() as ::core::ffi::c_ulong;
let mut y: bar = bar {
x: x as ::core::ffi::c_int,
} as bar;
let mut y: bar = bar { x: x };
Comment thread
ahomescu marked this conversation as resolved.
return y.x;
}
4 changes: 1 addition & 3 deletions c2rust-transpile/tests/snapshots/snapshots__transpile@auto_type.c.2021.clang22.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ pub unsafe extern "C" fn foo() -> ::core::ffi::c_int {
let mut x: ::core::ffi::c_int = 42 as ::core::ffi::c_int;
let mut px: *mut ::core::ffi::c_int = &raw mut x;
let mut sx: usize = ::core::mem::size_of::<::core::ffi::c_int>();
let mut y: bar = bar {
x: x as ::core::ffi::c_int,
} as bar;
let mut y: bar = bar { x: x };
return y.x;
}
4 changes: 1 addition & 3 deletions c2rust-transpile/tests/snapshots/snapshots__transpile@auto_type.c.2024.clang15.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ pub unsafe extern "C" fn foo() -> ::core::ffi::c_int {
let mut px: *mut ::core::ffi::c_int = &raw mut x;
let mut sx: ::core::ffi::c_ulong =
::core::mem::size_of::<::core::ffi::c_int>() as ::core::ffi::c_ulong;
let mut y: bar = bar {
x: x as ::core::ffi::c_int,
} as bar;
let mut y: bar = bar { x: x };
return y.x;
}
4 changes: 1 addition & 3 deletions c2rust-transpile/tests/snapshots/snapshots__transpile@auto_type.c.2024.clang22.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ pub unsafe extern "C" fn foo() -> ::core::ffi::c_int {
let mut x: ::core::ffi::c_int = 42 as ::core::ffi::c_int;
let mut px: *mut ::core::ffi::c_int = &raw mut x;
let mut sx: usize = ::core::mem::size_of::<::core::ffi::c_int>();
let mut y: bar = bar {
x: x as ::core::ffi::c_int,
} as bar;
let mut y: bar = bar { x: x };
return y.x;
}
16 changes: 8 additions & 8 deletions c2rust-transpile/tests/snapshots/snapshots__transpile@conditions.c.2021.clang15.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,12 @@ pub unsafe extern "C" fn conditional_operator(
let mut x: ::core::ffi::c_int = 0 as ::core::ffi::c_int;
let mut y: ::core::ffi::c_int = 1 as ::core::ffi::c_int;
*if false { &raw mut y } else { &raw mut x } = 10 as ::core::ffi::c_int;
*(buf as *mut ::core::ffi::c_int).offset(2isize) = if true {
*buf.offset(2isize) = if true {
2 as ::core::ffi::c_int
} else {
3 as ::core::ffi::c_int
};
*(buf as *mut ::core::ffi::c_int).offset(3isize) = if false {
*buf.offset(3isize) = if false {
2 as ::core::ffi::c_int
} else {
3 as ::core::ffi::c_int
Expand All @@ -129,37 +129,37 @@ pub unsafe extern "C" fn binary_conditional_operator(
buf: *mut ::core::ffi::c_int,
) {
let c2rust_fresh0 = id(0 as ::core::ffi::c_int);
*(buf as *mut ::core::ffi::c_int).offset(0isize) = if c2rust_fresh0 != 0 {
*buf.offset(0isize) = if c2rust_fresh0 != 0 {
c2rust_fresh0
} else {
id(1 as ::core::ffi::c_int)
};
let c2rust_fresh1 = id(2 as ::core::ffi::c_int);
*(buf as *mut ::core::ffi::c_int).offset(1isize) = if c2rust_fresh1 != 0 {
*buf.offset(1isize) = if c2rust_fresh1 != 0 {
c2rust_fresh1
} else {
id(3 as ::core::ffi::c_int)
};
if add(
(buf as *mut ::core::ffi::c_int).offset(2 as ::core::ffi::c_int as isize),
buf.offset(2 as ::core::ffi::c_int as isize),
2 as ::core::ffi::c_int,
0 as ::core::ffi::c_int,
) == 0
{
add(
(buf as *mut ::core::ffi::c_int).offset(3 as ::core::ffi::c_int as isize),
buf.offset(3 as ::core::ffi::c_int as isize),
3 as ::core::ffi::c_int,
0 as ::core::ffi::c_int,
);
}
if add(
(buf as *mut ::core::ffi::c_int).offset(4 as ::core::ffi::c_int as isize),
buf.offset(4 as ::core::ffi::c_int as isize),
4 as ::core::ffi::c_int,
1 as ::core::ffi::c_int,
) == 0
{
add(
(buf as *mut ::core::ffi::c_int).offset(5 as ::core::ffi::c_int as isize),
buf.offset(5 as ::core::ffi::c_int as isize),
5 as ::core::ffi::c_int,
0 as ::core::ffi::c_int,
);
Expand Down
16 changes: 8 additions & 8 deletions c2rust-transpile/tests/snapshots/snapshots__transpile@conditions.c.2024.clang15.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,12 @@ pub unsafe extern "C" fn conditional_operator(
let mut x: ::core::ffi::c_int = 0 as ::core::ffi::c_int;
let mut y: ::core::ffi::c_int = 1 as ::core::ffi::c_int;
*if false { &raw mut y } else { &raw mut x } = 10 as ::core::ffi::c_int;
*(buf as *mut ::core::ffi::c_int).offset(2isize) = if true {
*buf.offset(2isize) = if true {
2 as ::core::ffi::c_int
} else {
3 as ::core::ffi::c_int
};
*(buf as *mut ::core::ffi::c_int).offset(3isize) = if false {
*buf.offset(3isize) = if false {
2 as ::core::ffi::c_int
} else {
3 as ::core::ffi::c_int
Expand All @@ -129,37 +129,37 @@ pub unsafe extern "C" fn binary_conditional_operator(
buf: *mut ::core::ffi::c_int,
) {
let c2rust_fresh0 = id(0 as ::core::ffi::c_int);
*(buf as *mut ::core::ffi::c_int).offset(0isize) = if c2rust_fresh0 != 0 {
*buf.offset(0isize) = if c2rust_fresh0 != 0 {
c2rust_fresh0
} else {
id(1 as ::core::ffi::c_int)
};
let c2rust_fresh1 = id(2 as ::core::ffi::c_int);
*(buf as *mut ::core::ffi::c_int).offset(1isize) = if c2rust_fresh1 != 0 {
*buf.offset(1isize) = if c2rust_fresh1 != 0 {
c2rust_fresh1
} else {
id(3 as ::core::ffi::c_int)
};
if add(
(buf as *mut ::core::ffi::c_int).offset(2 as ::core::ffi::c_int as isize),
buf.offset(2 as ::core::ffi::c_int as isize),
2 as ::core::ffi::c_int,
0 as ::core::ffi::c_int,
) == 0
{
add(
(buf as *mut ::core::ffi::c_int).offset(3 as ::core::ffi::c_int as isize),
buf.offset(3 as ::core::ffi::c_int as isize),
3 as ::core::ffi::c_int,
0 as ::core::ffi::c_int,
);
}
if add(
(buf as *mut ::core::ffi::c_int).offset(4 as ::core::ffi::c_int as isize),
buf.offset(4 as ::core::ffi::c_int as isize),
4 as ::core::ffi::c_int,
1 as ::core::ffi::c_int,
) == 0
{
add(
(buf as *mut ::core::ffi::c_int).offset(5 as ::core::ffi::c_int as isize),
buf.offset(5 as ::core::ffi::c_int as isize),
5 as ::core::ffi::c_int,
0 as ::core::ffi::c_int,
);
Expand Down
2 changes: 1 addition & 1 deletion c2rust-transpile/tests/snapshots/snapshots__transpile@generics.c.2021.clang15.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,5 @@ pub unsafe extern "C" fn foo() {
let mut t1: *mut t = p1 as *mut t;
let mut t2: *const t = p2 as *const t;
let cx: ::core::ffi::c_int = n;
let mut ut_y: ::core::ffi::c_int = cx as ::core::ffi::c_int;
let mut ut_y: ::core::ffi::c_int = cx;
}
2 changes: 1 addition & 1 deletion c2rust-transpile/tests/snapshots/snapshots__transpile@generics.c.2024.clang15.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,5 @@ pub unsafe extern "C" fn foo() {
let mut t1: *mut t = p1 as *mut t;
let mut t2: *const t = p2 as *const t;
let cx: ::core::ffi::c_int = n;
let mut ut_y: ::core::ffi::c_int = cx as ::core::ffi::c_int;
let mut ut_y: ::core::ffi::c_int = cx;
}
7 changes: 7 additions & 0 deletions c2rust-transpile/tests/snapshots/snapshots__transpile@volatile.c.2021.clang15.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,11 @@ pub unsafe extern "C" fn test_volatile() {
::core::ptr::read_volatile::<*mut ::core::ffi::c_int>(&raw const volatile_global_struct.p)
.offset(-1),
);
::core::ptr::read_volatile::<::core::ffi::c_int>(&raw const vi);
-::core::ptr::read_volatile::<::core::ffi::c_int>(&raw const vi);
::core::ptr::read_volatile::<::core::ffi::c_int>(&raw const vi);
::core::ptr::read_volatile::<::core::ffi::c_int>(pvi);
::core::ptr::read_volatile::<*mut ::core::ffi::c_int>(&raw const volatile_global_struct.p);
::core::ptr::read_volatile::<*mut ::core::ffi::c_int>(&raw const volatile_global_struct.p);
*::core::ptr::read_volatile::<*mut ::core::ffi::c_int>(&raw const volatile_global_struct.p);
}
7 changes: 7 additions & 0 deletions c2rust-transpile/tests/snapshots/snapshots__transpile@volatile.c.2024.clang15.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,11 @@ pub unsafe extern "C" fn test_volatile() {
::core::ptr::read_volatile::<*mut ::core::ffi::c_int>(&raw const volatile_global_struct.p)
.offset(-1),
);
::core::ptr::read_volatile::<::core::ffi::c_int>(&raw const vi);
-::core::ptr::read_volatile::<::core::ffi::c_int>(&raw const vi);
::core::ptr::read_volatile::<::core::ffi::c_int>(&raw const vi);
::core::ptr::read_volatile::<::core::ffi::c_int>(pvi);
::core::ptr::read_volatile::<*mut ::core::ffi::c_int>(&raw const volatile_global_struct.p);
::core::ptr::read_volatile::<*mut ::core::ffi::c_int>(&raw const volatile_global_struct.p);
*::core::ptr::read_volatile::<*mut ::core::ffi::c_int>(&raw const volatile_global_struct.p);
}
9 changes: 9 additions & 0 deletions c2rust-transpile/tests/snapshots/volatile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,13 @@ void test_volatile(void) {

// https://github.com/immunant/c2rust/issues/1237
--(volatile_global_struct.p);

// Unused reads, should be included as side effects.
Comment thread
ahomescu marked this conversation as resolved.
vi;
-vi;
vi + 1;
*pvi;
volatile_global_struct.p;
volatile_global_struct.p + 1;
*volatile_global_struct.p;
}
Loading