diff --git a/c2rust-ast-exporter/src/AstExporter.cpp b/c2rust-ast-exporter/src/AstExporter.cpp index 0126c0d6db..0f4357a989 100644 --- a/c2rust-ast-exporter/src/AstExporter.cpp +++ b/c2rust-ast-exporter/src/AstExporter.cpp @@ -2226,7 +2226,7 @@ class TranslateASTVisitor final encode_entry( VD, TagVarDecl, loc, childIds, T, - [VD, is_defn, def, is_externally_visible](CborEncoder *array) { + [this, VD, is_defn, def, is_externally_visible](CborEncoder *array) { auto name = VD->getNameAsString(); cbor_encode_string(array, name); @@ -2246,11 +2246,9 @@ class TranslateASTVisitor final cbor_encoder_create_array(array, &attr_info, CborIndefiniteLength); - bool has_attrs = def ? def->hasAttrs() : VD->hasAttrs(); + bool has_attrs = def->hasAttrs(); if (has_attrs) { - auto attrs = def ? def->getAttrs() : VD->getAttrs(); - for (auto attr : def->attrs()) { cbor_encode_text_stringz(&attr_info, attr->getSpelling()); @@ -2261,6 +2259,15 @@ class TranslateASTVisitor final } else if (auto *aa = dyn_cast(attr)) { cbor_encode_text_stringz( &attr_info, aa->getAliasee().str().c_str()); + } else if (auto *ca = dyn_cast(attr)) { + cbor_encode_uint( + &attr_info, + uintptr_t(ca->getFunctionDecl()->getCanonicalDecl())); + } else { + printDiag(Context, DiagnosticsEngine::Warning, + std::string("ignoring unsupported variable attribute: ") + + attr->getSpelling(), + def); } } } diff --git a/c2rust-transpile/src/c_ast/conversion.rs b/c2rust-transpile/src/c_ast/conversion.rs index aa9fe95532..4da3d7e750 100644 --- a/c2rust-transpile/src/c_ast/conversion.rs +++ b/c2rust-transpile/src/c_ast/conversion.rs @@ -168,54 +168,77 @@ fn parse_cast_kind(kind: &str) -> CastKind { } } -fn parse_attributes(attributes: Vec) -> IndexSet { - let mut attrs = IndexSet::new(); - let mut expect_section_value = false; - let mut expect_alias_value = false; - let mut expect_visibility_value = false; - - for attr in attributes.into_iter() { - let attr_str = from_value::(attr).expect("Decl attributes should be strings"); - - match attr_str.as_str() { - "alias" => expect_alias_value = true, - "always_inline" => { - attrs.insert(Attribute::AlwaysInline); - } - "cold" => { - attrs.insert(Attribute::Cold); - } - "gnu_inline" => { - attrs.insert(Attribute::GnuInline); - } - "noinline" => { - attrs.insert(Attribute::NoInline); - } - "used" => { - attrs.insert(Attribute::Used); +impl ConversionContext { + fn parse_attributes(&mut self, attributes: Vec) -> IndexSet { + let mut attrs = IndexSet::new(); + let mut expect_section_value = false; + let mut expect_alias_value = false; + let mut expect_visibility_value = false; + let mut expect_cleanup_id = false; + + for attr in attributes.into_iter() { + if expect_cleanup_id { + expect_cleanup_id = false; + match from_value::(attr) { + Ok(func_id) => { + attrs.insert(Attribute::Cleanup(self.visit_decl(func_id))); + } + Err(_) => { + diag!( + Diagnostic::ClangAst, + "cleanup attribute is missing its function decl id", + ); + self.invalid_clang_ast = true; + } + } + continue; } - "visibility" => expect_visibility_value = true, - "section" => expect_section_value = true, - s if expect_section_value => { - attrs.insert(Attribute::Section(s.into())); - expect_section_value = false; - } - s if expect_alias_value => { - attrs.insert(Attribute::Alias(s.into())); + let attr_str = from_value::(attr).expect("Decl attributes should be strings"); - expect_alias_value = false; - } - s if expect_visibility_value => { - attrs.insert(Attribute::Visibility(s.into())); + match attr_str.as_str() { + "alias" => expect_alias_value = true, + "always_inline" => { + attrs.insert(Attribute::AlwaysInline); + } + "cleanup" => expect_cleanup_id = true, + "cold" => { + attrs.insert(Attribute::Cold); + } + "gnu_inline" => { + attrs.insert(Attribute::GnuInline); + } + "noinline" => { + attrs.insert(Attribute::NoInline); + } + "used" => { + attrs.insert(Attribute::Used); + } + "visibility" => expect_visibility_value = true, + "section" => expect_section_value = true, + s if expect_section_value => { + attrs.insert(Attribute::Section(s.into())); + + expect_section_value = false; + } + s if expect_alias_value => { + attrs.insert(Attribute::Alias(s.into())); + + expect_alias_value = false; + } + s if expect_visibility_value => { + attrs.insert(Attribute::Visibility(s.into())); - expect_visibility_value = false; + expect_visibility_value = false; + } + s => { + diag!(Diagnostic::ClangAst, "unrecognized attribute: {}", s); + } } - _ => {} } - } - attrs + attrs + } } /// This stores the information needed to convert an `AstContext` into a `TypedAstContext`. @@ -2082,7 +2105,7 @@ impl ConversionContext { .expect("Expected to find inline visibliity"); let attributes = from_value::>(node.extras[7].clone()) .expect("Expected to find attributes"); - let attrs = parse_attributes(attributes); + let attrs = self.parse_attributes(attributes); // The always_inline attribute implies inline even if the // inline keyword is not present. @@ -2335,7 +2358,7 @@ impl ConversionContext { .expect("Expected to find type on variable declaration"); let typ = self.visit_qualified_type(typ_id); - let attrs = parse_attributes(attributes); + let attrs = self.parse_attributes(attributes); let variable_decl = CDeclKind::Variable { has_static_duration, diff --git a/c2rust-transpile/src/c_ast/mod.rs b/c2rust-transpile/src/c_ast/mod.rs index 4bef8937e5..61239a4160 100644 --- a/c2rust-transpile/src/c_ast/mod.rs +++ b/c2rust-transpile/src/c_ast/mod.rs @@ -1202,6 +1202,20 @@ impl TypedAstContext { to_walk.push(parent_id); } } + + // `__attribute__((cleanup(func)))` references its cleanup + // function through the attribute payload, not via a + // DeclRef the traversal would otherwise see, so mark it + // here. + if let CDeclKind::Variable { ref attrs, .. } = self.c_decls[&decl_id].kind { + for attr in attrs { + if let Attribute::Cleanup(fn_id) = attr { + if wanted.insert(*fn_id) { + to_walk.push(*fn_id); + } + } + } + } } // Stmts can include decls, but we'll see the DeclId itself in a later @@ -2859,6 +2873,8 @@ pub enum Attribute { Alias(String), /// __attribute__((always_inline, __always_inline__)) AlwaysInline, + /// __attribute__((cleanup(func), __cleanup__(func))) + Cleanup(CDeclId), /// __attribute__((cold, __cold__)) Cold, /// Clang `__counted_by` / `__sized_by` (`_or_null`) bounds attribute on a diff --git a/c2rust-transpile/src/cfg/mod.rs b/c2rust-transpile/src/cfg/mod.rs index 9e02c779ad..33c1182cc1 100644 --- a/c2rust-transpile/src/cfg/mod.rs +++ b/c2rust-transpile/src/cfg/mod.rs @@ -1152,6 +1152,42 @@ impl WipBlock { /// This impl block deals with creating control flow graphs impl CfgBuilder { + fn decl_has_cleanup_attr(translator: &Translation, decl_id: CDeclId) -> bool { + matches!( + translator.ast_context[decl_id].kind, + CDeclKind::Variable { ref attrs, .. } + if attrs.iter().any(|attr| matches!(attr, Attribute::Cleanup(_))) + ) + } + + fn stmt_declares_cleanup(translator: &Translation, stmt_id: CStmtId) -> bool { + match translator.ast_context[stmt_id].kind { + CStmtKind::Decls(ref decls) => decls + .iter() + .any(|&decl_id| Self::decl_has_cleanup_attr(translator, decl_id)), + CStmtKind::Label(sub_stmt) + | CStmtKind::Case(_, sub_stmt, _) + | CStmtKind::Default(sub_stmt) + | CStmtKind::Attributed { + substatement: sub_stmt, + .. + } => Self::stmt_declares_cleanup(translator, sub_stmt), + _ => false, + } + } + + fn compound_declares_cleanup(translator: &Translation, stmt_ids: &[CStmtId]) -> bool { + stmt_ids + .iter() + .any(|&stmt_id| Self::stmt_declares_cleanup(translator, stmt_id)) + } + + fn for_init_declares_cleanup(translator: &Translation, init: Option) -> bool { + init.map_or(false, |stmt_id| { + Self::stmt_declares_cleanup(translator, stmt_id) + }) + } + fn last_per_stmt_mut(&mut self) -> &mut PerStmt { self.per_stmt_stack .last_mut() @@ -1398,6 +1434,14 @@ impl CfgBuilder { .get_span(SomeId::Stmt(stmt_id)) .unwrap_or_else(Span::call_site); + let wrap_in_cleanup_scope = match translator.ast_context.index(stmt_id).kind { + CStmtKind::Compound(ref comp_stmts) => { + Self::compound_declares_cleanup(translator, comp_stmts) + } + CStmtKind::ForLoop { init, .. } => Self::for_init_declares_cleanup(translator, init), + _ => false, + }; + let out_wip: TranslationResult> = match translator .ast_context .index(stmt_id) @@ -2028,7 +2072,13 @@ impl CfgBuilder { .unwrap() .is_contained(&self.c_label_to_goto, self.currently_live.last().unwrap()) { - self.incrementally_reloop_subgraph(translator, in_tail, entry, out_wip) + self.incrementally_reloop_subgraph( + translator, + in_tail, + entry, + out_wip, + wrap_in_cleanup_scope, + ) } else { let last_per_stmt = self.per_stmt_stack.pop().unwrap(); self.per_stmt_stack @@ -2067,6 +2117,10 @@ impl CfgBuilder { // Exit WIP out_wip: Option, + + // Whether this C statement owns a cleanup variable whose lifetime ends + // at the statement boundary. + wrap_in_cleanup_scope: bool, ) -> TranslationResult> { // Close off the `wip` using a `break` terminator let brk_lbl: Label = self.fresh_label(); @@ -2126,6 +2180,13 @@ impl CfgBuilder { stmts = vec![mk().expr_stmt(block)] } + if wrap_in_cleanup_scope { + let block_span = inner_span.unwrap_or_else(Span::call_site); + let block = mk().span(block_span).block(stmts); + let block_expr = mk().span(block_span).block_expr(block); + stmts = vec![mk().span(block_span).expr_stmt(block_expr)]; + } + let mut flattened_wip = self.new_wip_block(entry); // Copy span from removed statement if there was only one. if stmts.is_empty() { diff --git a/c2rust-transpile/src/translator/mod.rs b/c2rust-transpile/src/translator/mod.rs index d4bfb88c37..888a4d5683 100644 --- a/c2rust-transpile/src/translator/mod.rs +++ b/c2rust-transpile/src/translator/mod.rs @@ -289,6 +289,7 @@ pub struct Translation<'c> { /// alongside its required imports. Each additional nested level of caching translation /// causes an additional set to be pushed onto the `deferred_imports` vector. deferred_imports: RefCell>>, + cleanup_guard_emitted: Cell, // Comment support pub comment_context: CommentContext, // Incoming comments @@ -1510,6 +1511,7 @@ impl<'c> Translation<'c> { potential_flexible_array_members: RefCell::new(IndexSet::new()), macro_expansions: RefCell::new(IndexMap::new()), deferred_imports: RefCell::new(Vec::new()), + cleanup_guard_emitted: Cell::new(false), comment_context, comment_store: RefCell::new(CommentStore::new()), spans: HashMap::new(), @@ -1539,6 +1541,28 @@ impl<'c> Translation<'c> { f(item_store) } + /// Emit the runtime helper used to translate `__attribute__((cleanup(func)))`. + /// Idempotent: only the first call adds the struct + Drop impl to the main file. + pub fn use_cleanup_guard(&self) { + if self.cleanup_guard_emitted.replace(true) { + return; + } + let struct_item: Item = syn::parse_quote! { + struct CleanupGuard(*mut T, unsafe extern "C" fn(*mut T)); + }; + let impl_item: Item = syn::parse_quote! { + impl Drop for CleanupGuard { + fn drop(&mut self) { + unsafe { (self.1)(self.0) } + } + } + }; + let mut items = self.items.borrow_mut(); + let store = items.entry(self.main_file).or_default(); + store.add_item(Box::new(struct_item)); + store.add_item(Box::new(impl_item)); + } + /// Called when translation makes use of a language feature that will require a feature-gate. pub fn use_feature(&self, feature: &'static str) { if matches!( @@ -2379,6 +2403,7 @@ impl<'c> Translation<'c> { ref ident, initializer, typ, + ref attrs, .. } => { assert!( @@ -2436,6 +2461,32 @@ impl<'c> Translation<'c> { zeroed.to_pure_expr() } .expect("Expected decl initializer to not have any statements"); + + let cleanup_guard_stmt: Option = attrs + .iter() + .find_map(|a| match a { + Attribute::Cleanup(fn_id) => Some(*fn_id), + _ => None, + }) + .map(|fn_id| { + self.use_cleanup_guard(); + self.use_feature("raw_ref_op"); + let cleanup_name = self + .renamer + .borrow() + .get(&fn_id) + .expect("cleanup function not registered with renamer"); + let cleanup_ident = mk().ident(&cleanup_name); + let var_ident = mk().ident(&rust_name); + let guard_ident = mk().ident(&format!("_cleanup_{}", rust_name)); + syn::parse_quote! { + let #guard_ident = CleanupGuard( + &raw mut #var_ident as *mut _, + #cleanup_ident, + ); + } + }); + let pat_mut = mk().mutbl().ident_pat(rust_name.clone()); let local_mut = mk().local(pat_mut, Some(ty.clone()), Some(zeroed)); if has_self_reference { @@ -2448,6 +2499,11 @@ impl<'c> Translation<'c> { decl_and_assign.append(&mut stmts); decl_and_assign.push(mk().expr_stmt(assign)); + if let Some(stmt) = cleanup_guard_stmt { + assign_stmts.push(stmt.clone()); + decl_and_assign.push(stmt); + } + Ok(cfg::DeclStmtInfo::new( vec![mk().local_stmt(Box::new(local_mut))], assign_stmts, @@ -2473,6 +2529,11 @@ impl<'c> Translation<'c> { let mut decl_and_assign = stmts; decl_and_assign.push(mk().local_stmt(Box::new(local))); + if let Some(stmt) = cleanup_guard_stmt { + assign_stmts.push(stmt.clone()); + decl_and_assign.push(stmt); + } + Ok(cfg::DeclStmtInfo::new( vec![mk().local_stmt(Box::new(local_mut))], assign_stmts, diff --git a/tests/unit/cleanup_attr/Cargo.toml b/tests/unit/cleanup_attr/Cargo.toml new file mode 100644 index 0000000000..7eab7de01a --- /dev/null +++ b/tests/unit/cleanup_attr/Cargo.toml @@ -0,0 +1,6 @@ +[package] +name = "cleanup-attr-tests" +version = "0.1.0" +edition = "2024" + +[dependencies] diff --git a/tests/unit/cleanup_attr/build.rs b/tests/unit/cleanup_attr/build.rs new file mode 100644 index 0000000000..330c2e50b0 --- /dev/null +++ b/tests/unit/cleanup_attr/build.rs @@ -0,0 +1,7 @@ +use std::env; + +fn main() { + let manifest_dir = env::var("CARGO_MANIFEST_DIR").unwrap(); + + println!("cargo:rustc-link-search=native={}", manifest_dir); +} diff --git a/tests/unit/cleanup_attr/src/cleanup.c b/tests/unit/cleanup_attr/src/cleanup.c new file mode 100644 index 0000000000..9e0643547c --- /dev/null +++ b/tests/unit/cleanup_attr/src/cleanup.c @@ -0,0 +1,109 @@ +static int trace_buf[16]; +static int trace_n; + +static void reset(void) { + trace_n = 0; +} + +static void copy_trace(int *out, int *n) { + int i; + for (i = 0; i < trace_n; i++) out[i] = trace_buf[i]; + *n = trace_n; +} + +static void record(int *p) { + if (trace_n < 16) trace_buf[trace_n++] = *p; +} + +/* Each `do_*` helper runs the cleanup-bearing code in its own function so + * that all function-scope cleanups complete on return; the outer `run_*` + * then copies the recorded trace. */ + +static void do_single(void) { + int x __attribute__((cleanup(record))) = 5; + (void)x; +} + +void run_single(int *out, int *n) { + reset(); + do_single(); + copy_trace(out, n); +} + +static void do_multiple(void) { + int a __attribute__((cleanup(record))) = 1; + int b __attribute__((cleanup(record))) = 2; + int c __attribute__((cleanup(record))) = 3; + (void)a; (void)b; (void)c; +} + +void run_multiple(int *out, int *n) { + reset(); + do_multiple(); + copy_trace(out, n); +} + +static void do_early_return(void) { + int x __attribute__((cleanup(record))) = 7; + (void)x; + return; +} + +void run_early_return(int *out, int *n) { + reset(); + do_early_return(); + copy_trace(out, n); +} + +static void do_nested(void) { + int outer __attribute__((cleanup(record))) = 10; + { + int inner __attribute__((cleanup(record))) = 20; + (void)inner; + } + int after = 30; + record(&after); + (void)outer; +} + +void run_nested(int *out, int *n) { + reset(); + do_nested(); + copy_trace(out, n); +} + +/* Back-edge via goto forces c2rust to hoist locals. The cleanup variable + * is naturally reached, but its declaration sits in a hoisted block, so + * this verifies the guard runs on the assignment site rather than on the + * bare zero-init at function top. */ +static void do_goto(void) { + int counter __attribute__((cleanup(record))) = 0; +again: + counter++; + if (counter < 3) goto again; +} + +void run_goto(int *out, int *n) { + reset(); + do_goto(); + copy_trace(out, n); +} + +typedef struct { + int v; +} widget_t; + +static void widget_destroy(widget_t *w) { + record(&w->v); +} + +static void do_typedef(void) { + widget_t w __attribute__((cleanup(widget_destroy))) = { 42 }; + (void)w; +} + +void run_typedef(int *out, int *n) { + reset(); + do_typedef(); + copy_trace(out, n); +} diff --git a/tests/unit/cleanup_attr/src/test_cleanup.rs b/tests/unit/cleanup_attr/src/test_cleanup.rs new file mode 100644 index 0000000000..fb4d154568 --- /dev/null +++ b/tests/unit/cleanup_attr/src/test_cleanup.rs @@ -0,0 +1,61 @@ +use crate::cleanup::{ + rust_run_early_return, rust_run_goto, rust_run_multiple, rust_run_nested, rust_run_single, + rust_run_typedef, +}; +use std::ffi::c_int; + +#[link(name = "test")] +unsafe extern "C" { + fn run_single(out: *mut c_int, n: *mut c_int); + fn run_multiple(out: *mut c_int, n: *mut c_int); + fn run_early_return(out: *mut c_int, n: *mut c_int); + fn run_nested(out: *mut c_int, n: *mut c_int); + fn run_goto(out: *mut c_int, n: *mut c_int); + fn run_typedef(out: *mut c_int, n: *mut c_int); +} + +type EntryFn = unsafe extern "C" fn(out: *mut c_int, n: *mut c_int); + +fn collect(entry: EntryFn) -> Vec { + let mut buf = [0i32; 16]; + let mut n: c_int = 0; + unsafe { entry(buf.as_mut_ptr(), &mut n) }; + buf[..n as usize].to_vec() +} + +fn check(c_entry: EntryFn, rust_entry: EntryFn, expected: &[c_int]) { + let c_trace = collect(c_entry); + let rust_trace = collect(rust_entry); + assert_eq!(c_trace, expected, "C side diverged from expected"); + assert_eq!(rust_trace, expected, "Rust side diverged from expected"); +} + +#[test] +pub fn test_single() { + check(run_single, rust_run_single, &[5]); +} + +#[test] +pub fn test_reverse_declaration_order() { + check(run_multiple, rust_run_multiple, &[3, 2, 1]); +} + +#[test] +pub fn test_fires_through_early_return() { + check(run_early_return, rust_run_early_return, &[7]); +} + +#[test] +pub fn test_nested_block() { + check(run_nested, rust_run_nested, &[20, 30, 10]); +} + +#[test] +pub fn test_goto_with_hoisted_local() { + check(run_goto, rust_run_goto, &[3]); +} + +#[test] +pub fn test_typedef_signature() { + check(run_typedef, rust_run_typedef, &[42]); +}