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

Incorrect translation of alloca in block scope #759

Open
hvdijk opened this issue Dec 10, 2022 · 4 comments
Open

Incorrect translation of alloca in block scope #759

hvdijk opened this issue Dec 10, 2022 · 4 comments
Labels
bug Something isn't working

Comments

@hvdijk
Copy link

hvdijk commented Dec 10, 2022

Tested with c2rust 0.16.0

#include <stdio.h>
#include <string.h>

int one = 1;

int main(void) {
  char *p;
  if (one) {
    p = __builtin_alloca(100);
  }
  strcpy(p, "Hello, world!");
  puts(p);
}

This is a program that has no memory errors. The if (one) is guaranteed to be entered, though the compiler cannot see that, and the result of __builtin_alloca persists until the function ends, even after the block has ended.

It is translated to the following Rust code:

#![allow(dead_code, mutable_transmutes, non_camel_case_types, non_snake_case, non_upper_case_globals, unused_assignments, unused_mut)]
#![register_tool(c2rust)]
#![feature(register_tool)]
use ::c2rust_out::*;
extern "C" {
    fn puts(__s: *const libc::c_char) -> libc::c_int;
    fn strcpy(_: *mut libc::c_char, _: *const libc::c_char) -> *mut libc::c_char;
}
#[no_mangle]
pub static mut one: libc::c_int = 1 as libc::c_int;
unsafe fn main_0() -> libc::c_int {
    let mut p: *mut libc::c_char = 0 as *mut libc::c_char;
    if one != 0 {
        let mut fresh0 = ::std::vec::from_elem(
            0,
            100 as libc::c_int as libc::c_uint as usize,
        );
        p = fresh0.as_mut_ptr() as *mut libc::c_char;
    }
    strcpy(p, b"Hello, world!\0" as *const u8 as *const libc::c_char);
    puts(p);
    return 0;
}
pub fn main() {
    unsafe { ::std::process::exit(main_0() as i32) }
}

Note here the let mut fresh0 = ::std::vec::from_elem(0, 100 as libc::c_int as libc::c_uint as usize); in block scope. Here, unlike in the C program, the vec will be dropped when the block is exited, and p is left a dangling pointer.

A possible corrected translation would be to make fresh0 a function-scope variable.

@kkysen kkysen added the bug Something isn't working label Dec 10, 2022
@kkysen
Copy link
Contributor

kkysen commented Dec 11, 2022

Also, Vec is heap-allocated and alloca is stack-allocated. Shouldn't we try to preserve this?

@hvdijk
Copy link
Author

hvdijk commented Dec 11, 2022

Also, Vec is heap-allocated and alloca is stack-allocated. Shouldn't we try to preserve this?

Ideally, yes, but that may not be possible as Rust does not have alloca. There was an RFC to add it, rust-lang/rfcs#618, but it was closed in favour of https://doc.rust-lang.org/beta/unstable-book/language-features/unsized-locals.html. The unsized-locals feature, I believe, will be a suitable replacement for many uses of alloca, but not all of them, and not the one in the example I gave here. However, as the unsized-locals feature is not yet finalised, it is possible that it will be extended and become a full replacement.

@kkysen kkysen changed the title Incorrect translation of alloca in block scope Incorrect translation of alloca in block scope Jan 4, 2023
@burdges
Copy link

burdges commented Feb 13, 2023

@hvdijk
Copy link
Author

hvdijk commented Feb 13, 2023

https://github.com/playXE/alloca-rs

That only handles a subset of the possible uses of alloca as well, and I suspect it may be a smaller subset than what will be handled by that unsized-locals feature once that includes VLAs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants