diff --git a/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs b/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs index e705809f0d..47ce291439 100644 --- a/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs +++ b/compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs @@ -210,6 +210,7 @@ lazy_static! { E106, Warning, include_str!("./error_codes/E106.md"), // VAR_EXTERNAL have no effect E107, Error, include_str!("./error_codes/E107.md"), // Missing configuration for template variable E108, Error, include_str!("./error_codes/E108.md"), // Template variable is configured multiple times + E109, Error, include_str!("./error_codes/E109.md"), // Stateful pointer variable initialized with temporary value ); } diff --git a/compiler/plc_diagnostics/src/diagnostics/error_codes/E109.md b/compiler/plc_diagnostics/src/diagnostics/error_codes/E109.md new file mode 100644 index 0000000000..e41486e245 --- /dev/null +++ b/compiler/plc_diagnostics/src/diagnostics/error_codes/E109.md @@ -0,0 +1,16 @@ +# Stateful member variable initialized with temporary reference + +Stack-local variables do not yet exist at the time of initialization. Additionally, pointing to a temporary variable will lead to a dangling pointer +as soon as it goes out of scope - potential use after free. + +Erroneous code example: +``` +FUNCTION_BLOCK foo + VAR + a : REF_TO BOOL := REF(b); + END_VAR + VAR_TEMP + b : BOOL; + END_VAR +END_FUNCTION_BLOCK +``` \ No newline at end of file diff --git a/src/codegen/generators/pou_generator.rs b/src/codegen/generators/pou_generator.rs index a57a6940f7..793643330c 100644 --- a/src/codegen/generators/pou_generator.rs +++ b/src/codegen/generators/pou_generator.rs @@ -772,6 +772,20 @@ impl<'ink, 'cg> PouGenerator<'ink, 'cg> { variable.source_location.get_column(), ); } + + if self + .index + .find_effective_type_by_name(variable.get_type_name()) + .map(|it| it.get_type_information()) + .is_some_and(|it| it.is_reference_to() || it.is_alias()) + { + // aliases and reference to variables have special handling for initialization. initialize with a nullpointer + self.llvm.builder.build_store( + left, + left.get_type().get_element_type().into_pointer_type().const_null(), + ); + continue; + }; let right_stmt = self.index.get_const_expressions().maybe_get_constant_statement(&variable.initial_value); self.generate_variable_initializer(variable, left, right_stmt, &exp_gen)?; diff --git a/src/codegen/tests/initialization_test/complex_initializers.rs b/src/codegen/tests/initialization_test/complex_initializers.rs index bbe44a304b..ccb21b866b 100644 --- a/src/codegen/tests/initialization_test/complex_initializers.rs +++ b/src/codegen/tests/initialization_test/complex_initializers.rs @@ -1368,3 +1368,275 @@ fn var_external_blocks_are_ignored_in_init_functions() { } "###) } + +#[test] +fn ref_to_local_member() { + let res = codegen( + r" + FUNCTION_BLOCK foo + VAR + s : STRING; + ptr : REF_TO STRING := REF(s); + alias AT s : STRING; + reference_to : REFERENCE TO STRING REF= s; + END_VAR + END_FUNCTION_BLOCK + ", + ); + + insta::assert_snapshot!(res, @r#" + ; ModuleID = '' + source_filename = "" + + %foo = type { [81 x i8], [81 x i8]*, [81 x i8]*, [81 x i8]* } + + @__foo__init = unnamed_addr constant %foo zeroinitializer + + define void @foo(%foo* %0) { + entry: + %s = getelementptr inbounds %foo, %foo* %0, i32 0, i32 0 + %ptr = getelementptr inbounds %foo, %foo* %0, i32 0, i32 1 + %alias = getelementptr inbounds %foo, %foo* %0, i32 0, i32 2 + %reference_to = getelementptr inbounds %foo, %foo* %0, i32 0, i32 3 + ret void + } + ; ModuleID = '__initializers' + source_filename = "__initializers" + + %foo = type { [81 x i8], [81 x i8]*, [81 x i8]*, [81 x i8]* } + + @__foo__init = external global %foo + + define void @__init_foo(%foo* %0) { + entry: + %self = alloca %foo*, align 8 + store %foo* %0, %foo** %self, align 8 + %deref = load %foo*, %foo** %self, align 8 + %ptr = getelementptr inbounds %foo, %foo* %deref, i32 0, i32 1 + %deref1 = load %foo*, %foo** %self, align 8 + %s = getelementptr inbounds %foo, %foo* %deref1, i32 0, i32 0 + store [81 x i8]* %s, [81 x i8]** %ptr, align 8 + %deref2 = load %foo*, %foo** %self, align 8 + %alias = getelementptr inbounds %foo, %foo* %deref2, i32 0, i32 2 + %deref3 = load %foo*, %foo** %self, align 8 + %s4 = getelementptr inbounds %foo, %foo* %deref3, i32 0, i32 0 + store [81 x i8]* %s4, [81 x i8]** %alias, align 8 + %deref5 = load %foo*, %foo** %self, align 8 + %reference_to = getelementptr inbounds %foo, %foo* %deref5, i32 0, i32 3 + %deref6 = load %foo*, %foo** %self, align 8 + %s7 = getelementptr inbounds %foo, %foo* %deref6, i32 0, i32 0 + store [81 x i8]* %s7, [81 x i8]** %reference_to, align 8 + ret void + } + + declare void @foo(%foo*) + ; ModuleID = '__init___testproject' + source_filename = "__init___testproject" + + @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @__init___testproject, i8* null }] + + define void @__init___testproject() { + entry: + ret void + } + "#) +} + +#[test] +fn ref_to_local_member_shadows_global() { + let res = codegen( + r" + VAR_GLOBAL + s : STRING; + END_VAR + + FUNCTION_BLOCK foo + VAR + s : STRING; + ptr : REF_TO STRING := REF(s); + alias AT s : STRING; + reference_to : REFERENCE TO STRING REF= s; + END_VAR + END_FUNCTION_BLOCK + ", + ); + + insta::assert_snapshot!(res, @r#" + ; ModuleID = '' + source_filename = "" + + %foo = type { [81 x i8], [81 x i8]*, [81 x i8]*, [81 x i8]* } + + @s = global [81 x i8] zeroinitializer + @__foo__init = unnamed_addr constant %foo zeroinitializer + + define void @foo(%foo* %0) { + entry: + %s = getelementptr inbounds %foo, %foo* %0, i32 0, i32 0 + %ptr = getelementptr inbounds %foo, %foo* %0, i32 0, i32 1 + %alias = getelementptr inbounds %foo, %foo* %0, i32 0, i32 2 + %reference_to = getelementptr inbounds %foo, %foo* %0, i32 0, i32 3 + ret void + } + ; ModuleID = '__initializers' + source_filename = "__initializers" + + %foo = type { [81 x i8], [81 x i8]*, [81 x i8]*, [81 x i8]* } + + @__foo__init = external global %foo + + define void @__init_foo(%foo* %0) { + entry: + %self = alloca %foo*, align 8 + store %foo* %0, %foo** %self, align 8 + %deref = load %foo*, %foo** %self, align 8 + %ptr = getelementptr inbounds %foo, %foo* %deref, i32 0, i32 1 + %deref1 = load %foo*, %foo** %self, align 8 + %s = getelementptr inbounds %foo, %foo* %deref1, i32 0, i32 0 + store [81 x i8]* %s, [81 x i8]** %ptr, align 8 + %deref2 = load %foo*, %foo** %self, align 8 + %alias = getelementptr inbounds %foo, %foo* %deref2, i32 0, i32 2 + %deref3 = load %foo*, %foo** %self, align 8 + %s4 = getelementptr inbounds %foo, %foo* %deref3, i32 0, i32 0 + store [81 x i8]* %s4, [81 x i8]** %alias, align 8 + %deref5 = load %foo*, %foo** %self, align 8 + %reference_to = getelementptr inbounds %foo, %foo* %deref5, i32 0, i32 3 + %deref6 = load %foo*, %foo** %self, align 8 + %s7 = getelementptr inbounds %foo, %foo* %deref6, i32 0, i32 0 + store [81 x i8]* %s7, [81 x i8]** %reference_to, align 8 + ret void + } + + declare void @foo(%foo*) + ; ModuleID = '__init___testproject' + source_filename = "__init___testproject" + + @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @__init___testproject, i8* null }] + + define void @__init___testproject() { + entry: + ret void + } + "#) +} + +#[test] +fn temporary_variable_ref_to_local_member() { + let res = codegen( + r" + FUNCTION_BLOCK foo + VAR + s : STRING; + END_VAR + VAR_TEMP + ptr : REF_TO STRING := REF(s); + alias AT s : STRING; + reference_to : REFERENCE TO STRING REF= s; + END_VAR + END_FUNCTION_BLOCK + ", + ); + + insta::assert_snapshot!(res, @r#" + ; ModuleID = '' + source_filename = "" + + %foo = type { [81 x i8] } + + @__foo__init = unnamed_addr constant %foo zeroinitializer + + define void @foo(%foo* %0) { + entry: + %s = getelementptr inbounds %foo, %foo* %0, i32 0, i32 0 + %ptr = alloca [81 x i8]*, align 8 + %alias = alloca [81 x i8]*, align 8 + %reference_to = alloca [81 x i8]*, align 8 + store [81 x i8]* %s, [81 x i8]** %ptr, align 8 + store [81 x i8]* null, [81 x i8]** %alias, align 8 + store [81 x i8]* null, [81 x i8]** %reference_to, align 8 + store [81 x i8]* %s, [81 x i8]** %ptr, align 8 + store [81 x i8]* %s, [81 x i8]** %alias, align 8 + store [81 x i8]* %s, [81 x i8]** %reference_to, align 8 + ret void + } + ; ModuleID = '__initializers' + source_filename = "__initializers" + + %foo = type { [81 x i8] } + + @__foo__init = external global %foo + + define void @__init_foo(%foo* %0) { + entry: + %self = alloca %foo*, align 8 + store %foo* %0, %foo** %self, align 8 + ret void + } + + declare void @foo(%foo*) + ; ModuleID = '__init___testproject' + source_filename = "__init___testproject" + + @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @__init___testproject, i8* null }] + + define void @__init___testproject() { + entry: + ret void + } + "#) +} + +#[test] +fn temporary_variable_ref_to_temporary_variable() { + let res = codegen( + r" + FUNCTION foo + VAR + ptr : REF_TO STRING := REF(s); + alias AT s : STRING; + END_VAR + VAR_TEMP + s : STRING; + reference_to : REFERENCE TO STRING REF= alias; + END_VAR + FUNCTION + ", + ); + + insta::assert_snapshot!(res, @r##" + ; ModuleID = '' + source_filename = "" + + define void @foo() { + entry: + %ptr = alloca [81 x i8]*, align 8 + %alias = alloca [81 x i8]*, align 8 + %s = alloca [81 x i8], align 1 + %reference_to = alloca [81 x i8]*, align 8 + store [81 x i8]* %s, [81 x i8]** %ptr, align 8 + store [81 x i8]* null, [81 x i8]** %alias, align 8 + %0 = bitcast [81 x i8]* %s to i8* + call void @llvm.memset.p0i8.i64(i8* align 1 %0, i8 0, i64 ptrtoint ([81 x i8]* getelementptr ([81 x i8], [81 x i8]* null, i32 1) to i64), i1 false) + store [81 x i8]* null, [81 x i8]** %reference_to, align 8 + store [81 x i8]* %s, [81 x i8]** %ptr, align 8 + store [81 x i8]* %s, [81 x i8]** %alias, align 8 + %deref = load [81 x i8]*, [81 x i8]** %alias, align 8 + store [81 x i8]* %deref, [81 x i8]** %reference_to, align 8 + ret void + } + + ; Function Attrs: argmemonly nofree nounwind willreturn writeonly + declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg) #0 + + attributes #0 = { argmemonly nofree nounwind willreturn writeonly } + ; ModuleID = '__init___testproject' + source_filename = "__init___testproject" + + @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @__init___testproject, i8* null }] + + define void @__init___testproject() { + entry: + ret void + } + "##) +} diff --git a/src/codegen/tests/statement_codegen_test.rs b/src/codegen/tests/statement_codegen_test.rs index e09a20b5ac..967a37e476 100644 --- a/src/codegen/tests/statement_codegen_test.rs +++ b/src/codegen/tests/statement_codegen_test.rs @@ -297,7 +297,6 @@ fn reference_to_string_assignment() { VAR a : REF_TO STRING; END_VAR - a^ := 'hello'; END_FUNCTION "#, @@ -351,7 +350,7 @@ fn local_alias() { "#, ); - assert_snapshot!(content, @r###" + assert_snapshot!(content, @r#" ; ModuleID = '' source_filename = "" @@ -359,8 +358,7 @@ fn local_alias() { entry: %foo = alloca i32*, align 8 %bar = alloca i32, align 4 - %load_bar = load i32, i32* %bar, align 4 - store i32 %load_bar, i32** %foo, align 4 + store i32* null, i32** %foo, align 8 store i32 0, i32* %bar, align 4 store i32* %bar, i32** %foo, align 8 ret void @@ -374,7 +372,7 @@ fn local_alias() { entry: ret void } - "###); + "#); } #[test] @@ -390,7 +388,7 @@ fn local_string_alias() { "#, ); - assert_snapshot!(content, @r###" + assert_snapshot!(content, @r##" ; ModuleID = '' source_filename = "" @@ -398,8 +396,7 @@ fn local_string_alias() { entry: %foo = alloca [81 x i8]*, align 8 %bar = alloca [81 x i8], align 1 - %load_bar = load [81 x i8], [81 x i8]* %bar, align 1 - store [81 x i8] %load_bar, [81 x i8]** %foo, align 1 + store [81 x i8]* null, [81 x i8]** %foo, align 8 %0 = bitcast [81 x i8]* %bar to i8* call void @llvm.memset.p0i8.i64(i8* align 1 %0, i8 0, i64 ptrtoint ([81 x i8]* getelementptr ([81 x i8], [81 x i8]* null, i32 1) to i64), i1 false) store [81 x i8]* %bar, [81 x i8]** %foo, align 8 @@ -419,7 +416,7 @@ fn local_string_alias() { entry: ret void } - "###); + "##); } #[test] diff --git a/src/lowering.rs b/src/lowering.rs index 885a629a43..b2d765cccd 100644 --- a/src/lowering.rs +++ b/src/lowering.rs @@ -1,10 +1,13 @@ use crate::{ - index::{get_init_fn_name, Index, VariableIndexEntry}, - resolver::{const_evaluator::UnresolvableConstant, AstAnnotations}, + index::{get_init_fn_name, Index, PouIndexEntry, VariableIndexEntry}, + resolver::{const_evaluator::UnresolvableConstant, AnnotationMap, AstAnnotations}, }; use initializers::{Init, InitAssignments, Initializers, GLOBAL_SCOPE}; use plc_ast::{ - ast::{AstFactory, AstNode, CompilationUnit, ConfigVariable, DataType, LinkageType, PouType}, + ast::{ + AstFactory, AstNode, AstStatement, CallStatement, CompilationUnit, ConfigVariable, DataType, + LinkageType, PouType, + }, mut_visitor::{AstVisitorMut, WalkerMut}, provider::IdProvider, visit_all_nodes_mut, @@ -78,28 +81,89 @@ impl AstLowerer { self.ctxt.scope(old); } - fn collect_alias_and_reference_to_inits(&mut self, variable: &mut plc_ast::ast::Variable) { + fn update_initializer(&mut self, variable: &mut plc_ast::ast::Variable) { + // flat references to stateful pou-local variables need to have a qualifier added, so they can be resolved in the init functions + let scope = self.ctxt.get_scope().as_ref().map(|it| it.as_str()).unwrap_or(GLOBAL_SCOPE); + let needs_qualifier = |flat_ref| { + let rhs = self.index.find_member(scope, flat_ref); + let lhs = self.index.find_member(scope, variable.get_name()); + let Some(pou) = self.index.find_pou(scope) else { return Ok(false) }; + if !pou.is_function() && lhs.is_some_and(|it| !it.is_temp()) && rhs.is_some_and(|it| it.is_temp()) + { + // Unable to initialize a stateful member variable with an address of a temporary value since it doesn't exist at the time of initialization + // On top of that, even if we were to initialize it, it would lead to a dangling pointer/potential use-after-free + return Err(AstFactory::create_empty_statement( + SourceLocation::internal(), + self.ctxt.get_id_provider().next_id(), + )); + } + Ok(match pou { + PouIndexEntry::Program { .. } + | PouIndexEntry::FunctionBlock { .. } + | PouIndexEntry::Class { .. } + // we only want to add qualifiers to local, non-temporary variables + if rhs.is_some_and(|it| !it.is_temp()) && lhs.is_some_and(|it| !it.is_temp())=> + { + true + } + PouIndexEntry::Method { .. } => { + unimplemented!("We'll worry about this once we get around to OOP") + } + _ => false, + }) + }; + if let Some(initializer) = variable.initializer.as_ref() { - let Some(variable_ty) = variable - .data_type_declaration - .get_referenced_type() - .and_then(|it| self.index.find_effective_type_by_name(&it)) - else { - return variable.walk(self); + if self + .annotations + .get_type_hint(initializer, &self.index) + .map(|it| it.get_type_information()) + .filter(|dti| dti.is_pointer() || dti.is_reference_to() || dti.is_alias()) + .is_none() + { + return; }; - - let variable_is_auto_deref_pointer = { - variable_ty.get_type_information().is_alias() - || variable_ty.get_type_information().is_reference_to() + let updated_initializer = match &initializer.get_stmt() { + // no call-statement in the initializer, so something like `a AT b` or `a : REFERENCE TO ... REF= b` + AstStatement::ReferenceExpr(_) => { + initializer.get_flat_reference_name().and_then(|flat_ref| { + needs_qualifier(flat_ref).map_or_else(Option::Some, |q| { + q.then_some("self") + .map(|it| create_member_reference(it, self.ctxt.get_id_provider(), None)) + .and_then(|base| { + initializer.get_flat_reference_name().map(|it| { + create_member_reference(it, self.ctxt.get_id_provider(), Some(base)) + }) + }) + }) + }) + } + // we found a call-statement, must be `a : REF_TO ... := REF(b) | ADR(b)` + AstStatement::CallStatement(CallStatement { operator, parameters }) => parameters + .as_ref() + .and_then(|it| it.as_ref().get_flat_reference_name()) + .and_then(|flat_ref| { + let op = operator.as_ref().get_flat_reference_name()?; + needs_qualifier(flat_ref).map_or_else(Option::Some, |q| { + q.then(|| { + create_call_statement( + op, + flat_ref, + Some("self"), + self.ctxt.id_provider.clone(), + &initializer.location, + ) + }) + }) + }), + _ => return, }; - if variable_is_auto_deref_pointer { - self.unresolved_initializers.insert_initializer( - self.ctxt.get_scope().as_ref().map(|it| it.as_str()).unwrap_or(GLOBAL_SCOPE), - Some(&variable.name), - &Some(initializer.clone()), - ); - } + self.unresolved_initializers.insert_initializer( + scope, + Some(&variable.name), + &updated_initializer.or(Some(initializer.clone())), + ); } } @@ -258,7 +322,7 @@ impl AstVisitorMut for AstLowerer { fn visit_variable(&mut self, variable: &mut plc_ast::ast::Variable) { self.maybe_add_global_instance_initializer(variable); - self.collect_alias_and_reference_to_inits(variable); + self.update_initializer(variable); variable.walk(self); } diff --git a/src/resolver/tests/resolve_and_lower_init_functions.rs b/src/resolver/tests/resolve_and_lower_init_functions.rs index aa252dd4dd..d97cd2cc48 100644 --- a/src/resolver/tests/resolve_and_lower_init_functions.rs +++ b/src/resolver/tests/resolve_and_lower_init_functions.rs @@ -43,7 +43,7 @@ fn function_block_init_fn_created() { // this init-function is expected to have a single assignment statement in its function body let statements = &implementation.statements; assert_eq!(statements.len(), 1); - assert_debug_snapshot!(statements[0], @r###" + assert_debug_snapshot!(statements[0], @r#" Assignment { left: ReferenceExpr { kind: Member( @@ -78,12 +78,21 @@ fn function_block_init_fn_created() { name: "s", }, ), - base: None, + base: Some( + ReferenceExpr { + kind: Member( + Identifier { + name: "self", + }, + ), + base: None, + }, + ), }, ), }, } - "###); + "#); } #[test] @@ -125,7 +134,7 @@ fn program_init_fn_created() { // this init-function is expected to have a single assignment statement in its function body let statements = &implementation.statements; assert_eq!(statements.len(), 1); - assert_debug_snapshot!(statements[0], @r###" + assert_debug_snapshot!(statements[0], @r#" Assignment { left: ReferenceExpr { kind: Member( @@ -160,12 +169,21 @@ fn program_init_fn_created() { name: "s", }, ), - base: None, + base: Some( + ReferenceExpr { + kind: Member( + Identifier { + name: "self", + }, + ), + base: None, + }, + ), }, ), }, } - "###); + "#); } #[test] diff --git a/src/tests/adr/initializer_functions_adr.rs b/src/tests/adr/initializer_functions_adr.rs index 1a0fcd2bb7..ce1321ee0d 100644 --- a/src/tests/adr/initializer_functions_adr.rs +++ b/src/tests/adr/initializer_functions_adr.rs @@ -116,7 +116,7 @@ fn initializers_are_assigned_or_delegated_to_respective_init_functions() { assert_eq!(&init_foo_impl.name, "__init_foo"); let statements = &init_foo_impl.statements; assert_eq!(statements.len(), 1); - assert_debug_snapshot!(statements[0], @r###" + assert_debug_snapshot!(statements[0], @r#" Assignment { left: ReferenceExpr { kind: Member( @@ -151,12 +151,21 @@ fn initializers_are_assigned_or_delegated_to_respective_init_functions() { name: "s", }, ), - base: None, + base: Some( + ReferenceExpr { + kind: Member( + Identifier { + name: "self", + }, + ), + base: None, + }, + ), }, ), }, } - "###); + "#); // the init-function for `bar` will have a `CallStatement` to `__init_foo` as its only statement, passing the member-instance `self.fb` let init_bar_impl = &units[1].implementations[1]; @@ -202,7 +211,7 @@ fn initializers_are_assigned_or_delegated_to_respective_init_functions() { assert_eq!(&init_baz_impl.name, "__init_baz"); let statements = &init_baz_impl.statements; assert_eq!(statements.len(), 2); - assert_debug_snapshot!(statements[0], @r###" + assert_debug_snapshot!(statements[0], @r#" Assignment { left: ReferenceExpr { kind: Member( @@ -227,10 +236,19 @@ fn initializers_are_assigned_or_delegated_to_respective_init_functions() { name: "d", }, ), - base: None, + base: Some( + ReferenceExpr { + kind: Member( + Identifier { + name: "self", + }, + ), + base: None, + }, + ), }, } - "###); + "#); assert_debug_snapshot!(statements[1], @r###" CallStatement { @@ -688,7 +706,7 @@ fn intializing_temporary_variables() { "; let res = codegen(src); - assert_snapshot!(res, @r###" + assert_snapshot!(res, @r##" ; ModuleID = '' source_filename = "" @@ -715,8 +733,7 @@ fn intializing_temporary_variables() { %s2 = alloca [81 x i8]*, align 8 %0 = bitcast %foo* %fb to i8* call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %0, i8* align 1 bitcast (%foo* @__foo__init to i8*), i64 ptrtoint (%foo* getelementptr (%foo, %foo* null, i32 1) to i64), i1 false) - %load_ps = load [81 x i8], [81 x i8]* @ps, align 1 - store [81 x i8] %load_ps, [81 x i8]** %s, align 1 + store [81 x i8]* null, [81 x i8]** %s, align 8 store [81 x i8]* @ps2, [81 x i8]** %s2, align 8 store i32 0, i32* %main, align 4 store [81 x i8]* @ps, [81 x i8]** %s, align 8 @@ -761,5 +778,5 @@ fn intializing_temporary_variables() { entry: ret void } - "###) + "##) } diff --git a/src/validation/tests/variable_validation_tests.rs b/src/validation/tests/variable_validation_tests.rs index aff4257196..bbdb65df80 100644 --- a/src/validation/tests/variable_validation_tests.rs +++ b/src/validation/tests/variable_validation_tests.rs @@ -1246,3 +1246,65 @@ fn using_var_external_variable_without_matching_global_will_not_resolve() { "###); } + +#[test] +fn assigning_a_temp_reference_to_stateful_var_is_error() { + let diagnostics = parse_and_validate_buffered( + r#" + FUNCTION_BLOCK foo + VAR + s1: REF_TO DINT := REF(t1); // error + s2 AT t1 : DINT; // error + s3 : REFERENCE TO DINT REF= t1; // error + s4 AT s2 : DINT; // OK + s5 : REF_TO DINT := REF(s4); // OK + s6 : REFERENCE TO DINT REF= s5; // OK + END_VAR + VAR_TEMP + t1 : DINT; + t2 : REF_TO DINT := REF(t1); // OK + t3 AT s1 : DINT; // OK + t4 : REFERENCE TO DINT REF= t3; // OK + END_VAR + END_FUNCTION_BLOCK + + // all of these assignments are okay in a function, since they are all stack-variables + FUNCTION bar + VAR + s1: REF_TO DINT := REF(t1); + s2 AT t1 : DINT; + s3 : REFERENCE TO DINT REF= t1; + s4 AT s2 : DINT; + s5 : REF_TO DINT := REF(s4); + s6 : REFERENCE TO DINT REF= s5; + END_VAR + VAR_TEMP + t1 : DINT; + t2 : REF_TO DINT := REF(t1); + t3 AT s1 : DINT; + t4 : REFERENCE TO DINT REF= t3; + END_VAR + END_FUNCTION + "#, + ); + + assert_snapshot!(diagnostics, @r###" + error[E109]: Cannot assign address of temporary variable to a member-variable + ┌─ :4:40 + │ + 4 │ s1: REF_TO DINT := REF(t1); // error + │ ^^ Cannot assign address of temporary variable to a member-variable + + error[E109]: Cannot assign address of temporary variable to a member-variable + ┌─ :5:23 + │ + 5 │ s2 AT t1 : DINT; // error + │ ^^ Cannot assign address of temporary variable to a member-variable + + error[E109]: Cannot assign address of temporary variable to a member-variable + ┌─ :6:45 + │ + 6 │ s3 : REFERENCE TO DINT REF= t1; // error + │ ^^ Cannot assign address of temporary variable to a member-variable + "###) +} diff --git a/src/validation/variable.rs b/src/validation/variable.rs index eb32c0269d..57ec35eaf4 100644 --- a/src/validation/variable.rs +++ b/src/validation/variable.rs @@ -1,18 +1,18 @@ use plc_ast::ast::{ - ArgumentProperty, ConfigVariable, Pou, PouType, Variable, VariableBlock, VariableBlockType, + ArgumentProperty, AstNode, AstStatement, CallStatement, ConfigVariable, Pou, PouType, Variable, + VariableBlock, VariableBlockType, }; use plc_diagnostics::diagnostics::Diagnostic; use super::{ array::validate_array_assignment, - statement::visit_statement, + statement::{validate_pointer_assignment, visit_statement}, types::{data_type_is_fb_or_class_instance, visit_data_type_declaration}, ValidationContext, Validator, Validators, }; -use crate::validation::statement::validate_enum_variant_assignment; -use crate::validation::statement::validate_pointer_assignment; use crate::{index::const_expressions::ConstExpression, resolver::AnnotationMap}; use crate::{index::const_expressions::UnresolvableKind, typesystem::DataTypeInformation}; +use crate::{index::PouIndexEntry, validation::statement::validate_enum_variant_assignment}; use crate::{index::VariableIndexEntry, resolver::StatementAnnotation}; pub fn visit_config_variable( @@ -301,6 +301,8 @@ fn validate_variable( return; }; + report_temporary_address_in_pointer_initializer(validator, context, v_entry, node); + validate_pointer_assignment( context, validator, @@ -353,6 +355,54 @@ fn validate_variable( } } +fn report_temporary_address_in_pointer_initializer( + validator: &mut Validator<'_>, + context: &ValidationContext<'_, T>, + v_entry: &VariableIndexEntry, + initializer: &AstNode, +) { + if v_entry.is_temp() { + return; + } + if let Some(pou) = context.qualifier.and_then(|q| context.index.find_pou(q)) { + match pou { + PouIndexEntry::Program { .. } + | PouIndexEntry::FunctionBlock { .. } + | PouIndexEntry::Class { .. } => (), + PouIndexEntry::Method { .. } => { + unimplemented!("We'll worry about this once we get around to OOP") + } + _ => return, + } + } + + let (Some(flat_ref), Some(location)) = (match &initializer.get_stmt() { + AstStatement::ReferenceExpr(_) => { + (initializer.get_flat_reference_name(), Some(initializer.get_location())) + } + AstStatement::CallStatement(CallStatement { parameters, .. }) => ( + parameters.as_ref().and_then(|it| it.as_ref().get_flat_reference_name()), + parameters.as_ref().map(|it| it.get_location()), + ), + _ => (None, None), + }) else { + return; + }; + + let Some(rhs_entry) = context.index.find_member(context.qualifier.unwrap_or_default(), flat_ref) else { + return; + }; + if !rhs_entry.is_temp() { + return; + } + + validator.diagnostics.push( + Diagnostic::new("Cannot assign address of temporary variable to a member-variable") + .with_location(location) + .with_error_code("E109"), + ); +} + /// Returns a diagnostic if a `REFERENCE TO` variable is incorrectly declared. fn validate_reference_to_declaration( validator: &mut Validator, @@ -364,7 +414,8 @@ fn validate_reference_to_declaration( return; }; - if !variable_ty.get_type_information().is_reference_to() && !variable_ty.get_type_information().is_alias() + if !(variable_ty.get_type_information().is_reference_to() + || variable_ty.get_type_information().is_alias()) { return; } @@ -387,8 +438,10 @@ fn validate_reference_to_declaration( } if let Some(ref initializer) = variable.initializer { - let type_lhs = context.index.find_type(inner_ty_name).unwrap(); - let type_rhs = context.annotations.get_type(initializer, context.index).unwrap(); + report_temporary_address_in_pointer_initializer(validator, context, variable_entry, initializer); + + let Some(type_lhs) = context.annotations.get_type_hint(initializer, context.index) else { return }; + let Some(type_rhs) = context.annotations.get_type(initializer, context.index) else { return }; validate_pointer_assignment(context, validator, type_lhs, type_rhs, &initializer.location); } diff --git a/tests/lit/single/init/function_locals.st b/tests/lit/single/init/function_locals.st index 0ec25048bc..a9022b166c 100644 --- a/tests/lit/single/init/function_locals.st +++ b/tests/lit/single/init/function_locals.st @@ -2,6 +2,7 @@ VAR_GLOBAL ps: STRING := 'Hello world!'; ps2: STRING; + x : DINT := 1337; END_VAR FUNCTION_BLOCK foo @@ -19,9 +20,68 @@ END_VAR printf('%s$N', REF(s2)); END_FUNCTION_BLOCK +FUNCTION_BLOCK bar +VAR + x : DINT := 69; + shadowed_global : REF_TO DINT := REF(x); + alias_to_local AT x : DINT; + reference_to_local : REFERENCE TO DINT REF= x; +END_VAR +VAR_TEMP + y : DINT := 420; + temp_ref_to_temp : REF_TO DINT := REF(y); + temp_alias AT y :DINT; + temp_reference_to : REFERENCE TO DINT REF= y; +END_VAR + printf('%d$N', shadowed_global^); // CHECK 69 + printf('%d$N', temp_ref_to_temp^); // CHECK 420 + printf('%d$N', alias_to_local); // CHECK 69 + printf('%d$N', reference_to_local); // CHECK 69 + printf('%d$N', temp_alias); // CHECK 420 + printf('%d$N', temp_reference_to); // CHECK 420 +END_FUNCTION_BLOCK + FUNCTION main: DINT VAR fb: foo; + bb: bar; + + z : DINT := 42; + temp_ref_to_temp : REF_TO DINT := REF(z); + temp_alias AT z :DINT; + temp_reference_to : REFERENCE TO DINT REF= z; END_VAR fb(); + bb(); + baz(); + + printf('%d$N', temp_ref_to_temp^); // CHECK 42 + printf('%d$N', temp_alias); // CHECK 42 + printf('%d$N', temp_reference_to); // CHECK 42 +END_FUNCTION + +FUNCTION baz + VAR + s1: REF_TO LINT := REF(t1); + s2 AT t1 : LINT; + s3 : REFERENCE TO LINT REF= t1; + s4 AT s2 : LINT; + s5 : REF_TO LINT := REF(s4); + s6 : REFERENCE TO LINT REF= s4; + END_VAR + VAR_TEMP + t1 : LINT := 16#DEADBEEF; + t2 : REF_TO LINT := REF(t1); + t3 AT s2 : LINT; + t4 : REFERENCE TO LINT REF= s4; + END_VAR + printf('%#010x$N', s1^); // CHECK 0xdeadbeef + printf('%#010x$N', s2); // CHECK 0xdeadbeef + printf('%#010x$N', s3); // CHECK 0xdeadbeef + printf('%#010x$N', s4); // CHECK 0xdeadbeef + printf('%#010x$N', s5^); // CHECK 0xdeadbeef + printf('%#010x$N', s6); // CHECK 0xdeadbeef + printf('%#010x$N', t2^); // CHECK 0xdeadbeef + printf('%#010x$N', t3); // CHECK 0xdeadbeef + printf('%#010x$N', t4); // CHECK 0xdeadbeef END_FUNCTION \ No newline at end of file