c2rust icon indicating copy to clipboard operation
c2rust copied to clipboard

Missing casts in const macro translation

Open DCNick3 opened this issue 3 years ago • 1 comments

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

DCNick3 avatar Feb 24 '23 21:02 DCNick3

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))

GPHemsley avatar Sep 22 '24 02:09 GPHemsley

It may be easier to improve this when #1266 lands, as it adds information on which type the surrounding code is expecting to see. We should look into this while addressing #1295; it may be fixable as part of that work or as a follow-up.

fw-immunant avatar Jul 22 '25 23:07 fw-immunant