immunant / c2rust

Migrate C code to Rust
https://c2rust.com/
Other
3.99k stars 238 forks source link

Missing casts in const macro translation #853

Open DCNick3 opened 1 year ago

DCNick3 commented 1 year ago

A bug I found when working on unsafe-libopus

Apparently, when const macro translation is enabled, some casts are dropped, which leads to a change in semantics.

Reproducible case

#include <stdio.h>

#define silk_int16_MIN ((short)0x8000)

int test() { return silk_int16_MIN; }

int main() {
    printf("%d\n", test());
}

Without --translate-const-macros:

#![allow(dead_code, mutable_transmutes, non_camel_case_types, non_snake_case, non_upper_case_globals, unused_assignments, unused_mut)]
extern "C" {
    fn printf(_: *const libc::c_char, _: ...) -> libc::c_int;
}
#[no_mangle]
pub unsafe extern "C" fn test() -> libc::c_int {
    return 0x8000 as libc::c_int as libc::c_short as libc::c_int;
}
unsafe fn main_0() -> libc::c_int {
    printf(b"%d\n\0" as *const u8 as *const libc::c_char, test());
    return 0;
}
pub fn main() {
    unsafe { ::std::process::exit(main_0() as i32) }
}

Outputs -32768

With --translate-const-macros:

#![allow(dead_code, mutable_transmutes, non_camel_case_types, non_snake_case, non_upper_case_globals, unused_assignments, unused_mut)]
extern "C" {
    fn printf(_: *const libc::c_char, _: ...) -> libc::c_int;
}
pub const silk_int16_MIN: libc::c_int = 0x8000 as libc::c_int;
#[no_mangle]
pub unsafe extern "C" fn test() -> libc::c_int {
    return silk_int16_MIN;
}
unsafe fn main_0() -> libc::c_int {
    printf(b"%d\n\0" as *const u8 as *const libc::c_char, test());
    return 0;
}
pub fn main() {
    unsafe { ::std::process::exit(main_0() as i32) }
}

Outputs 32768

The bug

When no constants are generated the value is correctly cast to c_short before being widened back: 0x8000 as libc::c_int as libc::c_short as libc::c_int

When constant translation is enabled, however, the 0x8000 constant is not cast to the short type and stored directly as an int, which is incorrect

GPHemsley commented 1 month ago

This appears to be because canonical_macro_replacement() is calling resolve_expr_type_id(), which then discards the intermediate types:

https://github.com/immunant/c2rust/blob/03b949c1865a9f6946afc4ff1783866197adc7ea/c2rust-transpile/src/translator/mod.rs#L2184-L2230

https://github.com/immunant/c2rust/blob/03b949c1865a9f6946afc4ff1783866197adc7ea/c2rust-transpile/src/c_ast/mod.rs#L426-L454

#define TEST_CAST ((short)0x8000)

int test_cast() { return TEST_CAST; }
trace: Expanding macro CDeclId(337): Located { loc: Some(SrcSpan { fileid: 1, begin_line: 84, begin_column: 9, end_line: 84, end_column: 33 }), kind: MacroObject { name: "TEST_CAST" } }
trace: resolve_expr_type: Some(CTypeId(15)) // ImplicitCast(CQualTypeId { qualifiers: Qualifiers { is_const: false, is_restrict: false, is_volatile: false }, ctype: CTypeId(15) }, CExprId(338), IntegralCast, None, RValue)
trace: resolve_expr_type: Some(CTypeId(66)) // ExplicitCast(CQualTypeId { qualifiers: Qualifiers { is_const: false, is_restrict: false, is_volatile: false }, ctype: CTypeId(66) }, CExprId(340), IntegralCast, None, RValue)
trace: resolve_expr_type: Some(CTypeId(15)) // Literal(CQualTypeId { qualifiers: Qualifiers { is_const: false, is_restrict: false, is_volatile: false }, ctype: CTypeId(15) }, Integer(32768, Hex))