Skip to content

Commit

Permalink
validate dangling pointers
Browse files Browse the repository at this point in the history
  • Loading branch information
@mhasel
mhasel committed Oct 23, 2024
1 parent 49bbb60 commit 0c1227d
Show file tree
Hide file tree
Showing 5 changed files with 210 additions and 78 deletions.
40 changes: 11 additions & 29 deletions src/codegen/tests/initialization_test/complex_initializers.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1590,63 +1590,45 @@ fn temporary_variable_ref_to_local_member() {
fn temporary_variable_ref_to_temporary_variable() {
let res = codegen(
r"
FUNCTION_BLOCK foo
FUNCTION foo
VAR
ptr : REF_TO STRING := REF(s);
alias AT s : STRING;
END_VAR
VAR_TEMP
s : STRING;
ptr : REF_TO STRING := REF(s);
alias AT s : STRING;
reference_to : REFERENCE TO STRING REF= s;
reference_to : REFERENCE TO STRING REF= alias;
END_VAR
END_FUNCTION_BLOCK
FUNCTION
",
);

insta::assert_snapshot!(res, @r##"
; ModuleID = '<internal>'
source_filename = "<internal>"
%foo = type {}
@__foo__init = unnamed_addr constant %foo zeroinitializer
define void @foo(%foo* %0) {
define void @foo() {
entry:
%s = alloca [81 x i8], align 1
%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
%1 = bitcast [81 x i8]* %s to i8*
call void @llvm.memset.p0i8.i64(i8* align 1 %1, 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]* %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
store [81 x i8]* %s, [81 x i8]** %reference_to, 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 = '__initializers'
source_filename = "__initializers"
%foo = type {}
@__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"
Expand Down
92 changes: 50 additions & 42 deletions src/lowering.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,67 +85,75 @@ impl AstLowerer {
// 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_local_member(scope, flat_ref);
let lhs = self.index.find_local_member(scope, variable.get_name());
if lhs.is_some_and(|it| !it.is_temp()) && rhs.is_some_and(|it| it.is_temp()) {
// TODO: diagnostic
eprintln!("Assigning an address of a temporary variable to a stateful member variable can result in use-after-free");
return false;
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(),
));
}
self.index
.find_pou(scope)
.map(|it| match it {
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,
})
.unwrap_or_default()
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 hint = self.annotations.get_type_hint(initializer, &self.index);
if hint
if self
.annotations
.get_type_hint(initializer, &self.index)
.map(|it| it.get_type_information())
.is_some_and(|it| !(it.is_pointer() || it.is_reference_to() || it.is_alias()))
.filter(|dti| dti.is_pointer() || dti.is_reference_to() || dti.is_alias())
.is_none()
{
return;
};
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)
.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))
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 { parameters, .. }) => parameters
AstStatement::CallStatement(CallStatement { operator, parameters }) => parameters
.as_ref()
.and_then(|it| it.as_ref().get_flat_reference_name())
.and_then(|flat_ref| {
needs_qualifier(flat_ref).then(|| {
create_call_statement(
"REF",
flat_ref,
Some("self"),
self.ctxt.id_provider.clone(),
&initializer.location,
)
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,
Expand Down
63 changes: 63 additions & 0 deletions src/validation/tests/variable_validation_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1246,3 +1246,66 @@ 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[E001]: Cannot assign address of temporary variable to a member-variable
┌─ <internal>:4:40
│
4 │ s1: REF_TO DINT := REF(t1); // error
│ ^^ Cannot assign address of temporary variable to a member-variable
error[E001]: Cannot assign address of temporary variable to a member-variable
┌─ <internal>:5:23
│
5 │ s2 AT t1 : DINT; // error
│ ^^ Cannot assign address of temporary variable to a member-variable
error[E001]: Cannot assign address of temporary variable to a member-variable
┌─ <internal>:6:45
│
6 │ s3 : REFERENCE TO DINT REF= t1; // error
│ ^^ Cannot assign address of temporary variable to a member-variable
"#)
}
66 changes: 59 additions & 7 deletions src/validation/variable.rs
View file
Original file line number Diff line number Diff line change
@@ -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<T: AnnotationMap>(
Expand Down Expand Up @@ -301,6 +301,8 @@ fn validate_variable<T: AnnotationMap>(
return;
};

report_temporary_address_in_pointer_initializer(validator, context, v_entry, node);

validate_pointer_assignment(
context,
validator,
Expand Down Expand Up @@ -353,6 +355,53 @@ fn validate_variable<T: AnnotationMap>(
}
}

fn report_temporary_address_in_pointer_initializer<T: AnnotationMap>(
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),
);
}

/// Returns a diagnostic if a `REFERENCE TO` variable is incorrectly declared.
fn validate_reference_to_declaration<T: AnnotationMap>(
validator: &mut Validator,
Expand All @@ -364,7 +413,8 @@ fn validate_reference_to_declaration<T: AnnotationMap>(
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;
}
Expand All @@ -387,8 +437,10 @@ fn validate_reference_to_declaration<T: AnnotationMap>(
}

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);
}
Expand Down
Loading

0 comments on commit 0c1227d

Please sign in to comment.