Skip to content

Commit ab83531

Browse files
mhaselmhasel
authored
fix: various stack overflows in recursive functions when dealing with… (#1424)
* fix: various stack overflows in recursive functions when dealing with recursive datatypes
1 parent 454e1e5 commit ab83531

File tree

10 files changed

+160
-62
lines changed

10 files changed

+160
-62
lines changed

‎src/codegen/debug.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ impl<'ink> DebugBuilder<'ink> {
294294
//Adjust the offset based on the field alignment
295295
let type_info = dt.get_type_information();
296296
let alignment = type_info.get_alignment(index);
297-
let size = type_info.get_size(index);
297+
let size = type_info.get_size(index).unwrap();
298298
running_offset = running_offset.align_to(alignment);
299299

300300
types.push(
@@ -629,7 +629,7 @@ impl<'ink> Debug<'ink> for DebugBuilder<'ink> {
629629
//check if the type is currently registered
630630
if !self.types.contains_key(&name.to_lowercase()) {
631631
let type_info = datatype.get_type_information();
632-
let size = type_info.get_size(index);
632+
let size = type_info.get_size(index).unwrap();
633633
let alignment = type_info.get_alignment(index);
634634
let location = &datatype.location;
635635
match type_info {

‎src/codegen/llvm_typesystem.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ impl<'ctx, 'cast> Castable<'ctx, 'cast> for BasicValueEnum<'ctx> {
151151

152152
impl<'ctx, 'cast> Castable<'ctx, 'cast> for IntValue<'ctx> {
153153
fn cast(self, cast_data: &CastInstructionData<'ctx, 'cast>) -> BasicValueEnum<'ctx> {
154-
let lsize = cast_data.target_type.get_size_in_bits(cast_data.index);
154+
let lsize = cast_data.target_type.get_size_in_bits(cast_data.index).unwrap();
155155
match cast_data.target_type {
156156
DataTypeInformation::Integer { .. } => {
157157
//its important to use the real type's size here, because we may have an i1 which is annotated as BOOL (8 bit)
@@ -191,7 +191,7 @@ impl<'ctx, 'cast> Castable<'ctx, 'cast> for IntValue<'ctx> {
191191

192192
impl<'ctx, 'cast> Castable<'ctx, 'cast> for FloatValue<'ctx> {
193193
fn cast(self, cast_data: &CastInstructionData<'ctx, 'cast>) -> BasicValueEnum<'ctx> {
194-
let rsize = &cast_data.value_type.get_size_in_bits(cast_data.index);
194+
let rsize = &cast_data.value_type.get_size_in_bits(cast_data.index).unwrap();
195195
match cast_data.target_type {
196196
DataTypeInformation::Float { size: lsize, .. } => {
197197
if lsize < rsize {

‎src/datalayout.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ impl Default for DataLayout {
3535
}
3636

3737
/// An representation of a Byte unit, used to represent sizes, and alignments
38-
#[derive(PartialEq, Eq, Copy, Clone, Debug, PartialOrd)]
38+
#[derive(PartialEq, Eq, Copy, Clone, Debug, PartialOrd, Default)]
3939
pub struct Bytes(u32);
4040

4141
impl Add for Bytes {
@@ -186,7 +186,7 @@ mod tests {
186186

187187
let struct_type = index.get_effective_type_by_name("MyStruct").unwrap().get_type_information();
188188
// And the struct size takes the alignment into account
189-
assert_eq!(struct_type.get_size(&index).bits(), 192);
189+
assert_eq!(struct_type.get_size(&index).unwrap().bits(), 192);
190190
assert_eq!(struct_type.get_alignment(&index), Bytes::new(8)) //Struct alignment is 64 by default
191191
}
192192
}

‎src/index.rs

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,11 +1287,23 @@ impl Index {
12871287

12881288
/// Searches for variable name in the given container, if not found, attempts to search for it in super classes
12891289
pub fn find_member(&self, container_name: &str, variable_name: &str) -> Option<&VariableIndexEntry> {
1290+
self.find_member_recursive(container_name, variable_name, &mut FxHashSet::default())
1291+
}
1292+
1293+
fn find_member_recursive<'b>(
1294+
&'b self,
1295+
container_name: &str,
1296+
variable_name: &str,
1297+
seen: &mut FxHashSet<&'b str>,
1298+
) -> Option<&'b VariableIndexEntry> {
12901299
// Find pou in index
12911300
self.find_local_member(container_name, variable_name)
12921301
.or_else(|| {
12931302
if let Some(class) = self.find_pou(container_name).and_then(|it| it.get_super_class()) {
1294-
self.find_member(class, variable_name).filter(|it| !(it.is_temp()))
1303+
if !seen.insert(class) {
1304+
return None;
1305+
}
1306+
self.find_member_recursive(class, variable_name, seen).filter(|it| !(it.is_temp()))
12951307
} else {
12961308
None
12971309
}
@@ -1906,14 +1918,15 @@ impl Index {
19061918
/// Returns all methods declared on container, or its parents.
19071919
/// If a method is declared in the container the parent method is not included
19081920
pub fn find_methods(&self, container: &str) -> Vec<&PouIndexEntry> {
1909-
self.find_method_recursive(container, vec![])
1921+
self.find_method_recursive(container, vec![], &mut FxHashSet::default())
19101922
}
19111923

1912-
fn find_method_recursive<'a>(
1913-
&'a self,
1924+
fn find_method_recursive<'b>(
1925+
&'b self,
19141926
container: &str,
1915-
current_methods: Vec<&'a PouIndexEntry>,
1916-
) -> Vec<&'a PouIndexEntry> {
1927+
current_methods: Vec<&'b PouIndexEntry>,
1928+
seen: &mut FxHashSet<&'b str>,
1929+
) -> Vec<&'b PouIndexEntry> {
19171930
if let Some(pou) = self.find_pou(container) {
19181931
let mut res = self
19191932
.get_pous()
@@ -1928,7 +1941,10 @@ impl Index {
19281941
.collect::<Vec<_>>();
19291942
res.extend(current_methods);
19301943
if let Some(super_class) = pou.get_super_class() {
1931-
self.find_method_recursive(super_class, res)
1944+
if !seen.insert(super_class) {
1945+
return res;
1946+
};
1947+
self.find_method_recursive(super_class, res, seen)
19321948
} else {
19331949
res
19341950
}

‎src/typesystem.rs

Lines changed: 37 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ use std::{
55
ops::{Range, RangeInclusive},
66
};
77

8+
use anyhow::{anyhow, Result};
89
use plc_ast::{
910
ast::{AstNode, AutoDerefType, Operator, PouType, TypeNature},
1011
literals::{AstLiteral, StringValue},
1112
};
1213
use plc_source::source_location::SourceLocation;
14+
use rustc_hash::FxHashSet;
1315

1416
use crate::{
1517
datalayout::{Bytes, MemoryLocation},
@@ -613,52 +615,59 @@ impl DataTypeInformation {
613615
if let DataTypeInformation::Integer { semantic_size: Some(s), .. } = self {
614616
return *s;
615617
}
616-
self.get_size_in_bits(index)
618+
self.get_size_in_bits(index).unwrap_or_default()
617619
}
618620

619621
/// returns the number of bits used to store this type
620-
pub fn get_size_in_bits(&self, index: &Index) -> u32 {
621-
self.get_size(index).bits()
622+
pub fn get_size_in_bits(&self, index: &Index) -> Result<u32> {
623+
self.get_size(index).map(|it| it.bits())
622624
}
623625

624-
pub fn get_size(&self, index: &Index) -> Bytes {
626+
pub fn get_size(&self, index: &Index) -> Result<Bytes> {
627+
self.get_size_recursive(index, &mut FxHashSet::default())
628+
}
629+
630+
fn get_size_recursive<'b>(&'b self, index: &'b Index, seen: &mut FxHashSet<&'b str>) -> Result<Bytes> {
631+
if self.is_struct() && !seen.insert(self.get_name()) {
632+
return Err(anyhow!("Recursive type detected: {}", self.get_name()));
633+
}
625634
match self {
626-
DataTypeInformation::Integer { size, .. } => Bytes::from_bits(*size),
627-
DataTypeInformation::Float { size, .. } => Bytes::from_bits(*size),
628-
DataTypeInformation::String { size, encoding } => size
635+
DataTypeInformation::Integer { size, .. } => Ok(Bytes::from_bits(*size)),
636+
DataTypeInformation::Float { size, .. } => Ok(Bytes::from_bits(*size)),
637+
DataTypeInformation::String { size, encoding } => Ok(size
629638
.as_int_value(index)
630639
.map(|size| encoding.get_bytes_per_char() * size as u32)
631640
.map(Bytes::new)
632-
.unwrap(),
641+
.unwrap()),
633642
DataTypeInformation::Struct { members, .. } => members
634643
.iter()
635644
.map(|it| it.get_type_name())
636-
.fold(MemoryLocation::new(0), |prev, it| {
637-
let type_info = index.get_type_information_or_void(it);
638-
let size = type_info.get_size(index).value();
645+
.try_fold(MemoryLocation::new(0), |prev, it| {
646+
let type_info: &DataTypeInformation = index.get_type_information_or_void(it);
647+
let size = type_info.get_size_recursive(index, seen)?.value();
639648
let after_align = prev.align_to(type_info.get_alignment(index)).value();
640649
let res = after_align + size;
641-
MemoryLocation::new(res)
650+
Ok(MemoryLocation::new(res))
642651
})
643-
.into(),
652+
.map(Into::into),
644653
DataTypeInformation::Array { inner_type_name, dimensions, .. } => {
645654
let inner_type = index.get_type_information_or_void(inner_type_name);
646-
let inner_size = inner_type.get_size_in_bits(index);
655+
let inner_size = inner_type.get_size_in_bits(index)?;
647656
let element_count: u32 =
648657
dimensions.iter().map(|dim| dim.get_length(index).unwrap()).product();
649-
Bytes::from_bits(inner_size * element_count)
658+
Ok(Bytes::from_bits(inner_size * element_count))
650659
}
651-
DataTypeInformation::Pointer { .. } => Bytes::from_bits(POINTER_SIZE),
660+
DataTypeInformation::Pointer { .. } => Ok(Bytes::from_bits(POINTER_SIZE)),
652661
DataTypeInformation::Alias { referenced_type, .. }
653662
| DataTypeInformation::SubRange { referenced_type, .. } => {
654663
let inner_type = index.get_type_information_or_void(referenced_type);
655-
inner_type.get_size(index)
664+
inner_type.get_size_recursive(index, seen)
656665
}
657666
DataTypeInformation::Enum { referenced_type, .. } => index
658667
.find_effective_type_info(referenced_type)
659668
.map(|it| it.get_size(index))
660-
.unwrap_or_else(|| Bytes::from_bits(DINT_SIZE)),
661-
DataTypeInformation::Generic { .. } | DataTypeInformation::Void => Bytes::from_bits(0),
669+
.unwrap_or_else(|| Ok(Bytes::from_bits(DINT_SIZE))),
670+
DataTypeInformation::Generic { .. } | DataTypeInformation::Void => Ok(Bytes::from_bits(0)),
662671
}
663672
}
664673

@@ -673,7 +682,7 @@ impl DataTypeInformation {
673682
}
674683

675684
pub fn get_alignment(&self, index: &Index) -> Bytes {
676-
if self.get_size(index).value() == 0 {
685+
if self.get_size(index).unwrap_or_default().value() == 0 {
677686
return Bytes::new(0);
678687
}
679688

@@ -783,8 +792,8 @@ impl DataTypeInformation {
783792

784793
let DataTypeInformation::Array { inner_type_name, .. } = self else { return None };
785794
let inner_type_info = intrinsic_array_type(index, inner_type_name).get_type_information();
786-
let inner_type_size = inner_type_info.get_size_in_bits(index);
787-
let arr_size = self.get_size_in_bits(index);
795+
let inner_type_size = inner_type_info.get_size_in_bits(index).ok()?;
796+
let arr_size = self.get_size_in_bits(index).ok()?;
788797

789798
if inner_type_size == 0 {
790799
return None;
@@ -1303,7 +1312,7 @@ fn get_rank(type_information: &DataTypeInformation, index: &Index) -> u32 {
13031312
DataTypeInformation::SubRange { name, .. } | DataTypeInformation::Alias { name, .. } => {
13041313
get_rank(index.get_intrinsic_type_by_name(name).get_type_information(), index)
13051314
}
1306-
_ => type_information.get_size_in_bits(index),
1315+
_ => type_information.get_size_in_bits(index).unwrap_or_default(),
13071316
}
13081317
}
13091318

@@ -1344,7 +1353,7 @@ pub fn is_same_type_class(ltype: &DataTypeInformation, rtype: &DataTypeInformati
13441353
let l_inner_type = index.get_type_information_or_void(l_inner_type_name);
13451354
let r_inner_type = index.get_type_information_or_void(r_inner_type_name);
13461355
is_same_type_class(l_inner_type, r_inner_type, index)
1347-
&& ltype.get_size(index) == rtype.get_size(index)
1356+
&& ltype.get_size(index).unwrap_or_default() == rtype.get_size(index).unwrap_or_default()
13481357
}
13491358
_ => false,
13501359
},
@@ -1373,8 +1382,10 @@ pub fn get_bigger_type<'t, T: DataTypeInformationProvider<'t> + std::convert::Fr
13731382
// check is_numerical() on TypeNature e.g. DataTypeInformation::Integer is numerical but also used for CHARS which are not considered as numerical
13741383
if (ldt.is_numerical() && rdt.is_numerical()) && (ldt.is_real() || rdt.is_real()) {
13751384
let real_type = index.get_type_or_panic(REAL_TYPE);
1376-
let real_size = real_type.get_type_information().get_size_in_bits(index);
1377-
if lt.get_size_in_bits(index) > real_size || rt.get_size_in_bits(index) > real_size {
1385+
let real_size = real_type.get_type_information().get_size_in_bits(index).unwrap();
1386+
if lt.get_size_in_bits(index).unwrap_or_default() > real_size
1387+
|| rt.get_size_in_bits(index).unwrap_or_default() > real_size
1388+
{
13781389
return index.get_type_or_panic(LREAL_TYPE).into();
13791390
} else {
13801391
return real_type.into();

‎src/typesystem/tests.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ fn array_size_single_dim_tests() {
537537
location: SourceLocation::internal(),
538538
};
539539
//the size of the array is 20*size(int)
540-
assert_eq!(320, array_20.get_type_information().get_size_in_bits(&index));
540+
assert_eq!(320, array_20.get_type_information().get_size_in_bits(&index).unwrap());
541541
}
542542

543543
#[test]
@@ -565,7 +565,7 @@ fn array_size_multi_dim_tests() {
565565
location: SourceLocation::internal(),
566566
};
567567
//the size of the array is 20*size(int)
568-
assert_eq!(6400, array_20_20.get_type_information().get_size_in_bits(&index));
568+
assert_eq!(6400, array_20_20.get_type_information().get_size_in_bits(&index).unwrap());
569569
}
570570

571571
#[test]
@@ -603,5 +603,5 @@ fn array_size_nested_tests() {
603603
};
604604

605605
//the size of the array is 20*size(int)
606-
assert_eq!(6400, nested_array.get_type_information().get_size_in_bits(&index));
606+
assert_eq!(6400, nested_array.get_type_information().get_size_in_bits(&index).unwrap());
607607
}

‎src/validation/pou.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -636,8 +636,8 @@ pub(super) mod signature_validation {
636636
let method_ref = ctxt.method_ref;
637637
let l_encoding = left.get_string_character_width(ctxt.index);
638638
let r_encoding = right.get_string_character_width(ctxt.index);
639-
let left_length = left.get_size(ctxt.index).bits() / l_encoding.bits();
640-
let right_length = right.get_size(ctxt.index).bits() / r_encoding.bits();
639+
let left_length = left.get_size_in_bits(ctxt.index).unwrap() / l_encoding.bits();
640+
let right_length = right.get_size_in_bits(ctxt.index).unwrap() / r_encoding.bits();
641641
let mut sub_diagnostics = vec![];
642642
(left_length != right_length).then(|| {
643643
sub_diagnostics.push(

‎src/validation/statement.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1223,13 +1223,13 @@ fn is_invalid_pointer_assignment(
12231223
//check if Datatype can hold a Pointer (u64)
12241224
else if (right_type.is_pointer() && !right_type.is_auto_deref())
12251225
&& !left_type.is_pointer()
1226-
&& left_type.get_size_in_bits(index) < POINTER_SIZE
1226+
&& left_type.get_size_in_bits(index).unwrap_or_default() < POINTER_SIZE
12271227
{
12281228
validator.push_diagnostic(
12291229
Diagnostic::new(format!(
12301230
"The type {} {} is too small to hold a Pointer",
12311231
left_type.get_name(),
1232-
left_type.get_size_in_bits(index)
1232+
left_type.get_size_in_bits(index).unwrap_or_default()
12331233
))
12341234
.with_error_code("E065")
12351235
.with_location(location),
@@ -1239,13 +1239,13 @@ fn is_invalid_pointer_assignment(
12391239
//check if size allocated to Pointer is standart pointer size (u64)
12401240
else if left_type.is_pointer()
12411241
&& !right_type.is_pointer()
1242-
&& right_type.get_size_in_bits(index) < POINTER_SIZE
1242+
&& right_type.get_size_in_bits(index).unwrap_or_default() < POINTER_SIZE
12431243
{
12441244
validator.push_diagnostic(
12451245
Diagnostic::new(format!(
12461246
"The type {} {} is too small to be stored in a Pointer",
12471247
right_type.get_name(),
1248-
right_type.get_size_in_bits(index)
1248+
right_type.get_size_in_bits(index).unwrap_or_default()
12491249
))
12501250
.with_error_code("E065")
12511251
.with_location(location),
@@ -1632,10 +1632,10 @@ fn validate_assignment_type_sizes<T: AnnotationMap>(
16321632
}
16331633

16341634
let lhs = left.get_type_information();
1635-
let lhs_size = lhs.get_size(context.index);
1635+
let Ok(lhs_size) = lhs.get_size(context.index) else { return };
16361636
let results_in_truncation = |rhs: &DataType| {
16371637
let rhs = rhs.get_type_information();
1638-
let rhs_size = rhs.get_size(context.index);
1638+
let Ok(rhs_size) = rhs.get_size(context.index) else { return false };
16391639
lhs_size < rhs_size
16401640
|| (lhs_size == rhs_size
16411641
&& ((lhs.is_signed_int() && rhs.is_unsigned_int()) || (lhs.is_int() && rhs.is_float())))
@@ -1713,7 +1713,7 @@ pub(crate) mod helper {
17131713
data_type: &DataTypeInformation,
17141714
index: &Index,
17151715
) -> bool {
1716-
(access.get_bit_width() * access_index) < data_type.get_size_in_bits(index) as u64
1716+
(access.get_bit_width() * access_index) < data_type.get_size_in_bits(index).unwrap_or_default() as u64
17171717
}
17181718

17191719
/// Returns the range from 0 for the given data type
@@ -1722,7 +1722,7 @@ pub(crate) mod helper {
17221722
data_type: &DataTypeInformation,
17231723
index: &Index,
17241724
) -> Range<u64> {
1725-
0..((data_type.get_size_in_bits(index) as u64 / access.get_bit_width()) - 1)
1725+
0..((data_type.get_size_in_bits(index).unwrap_or_default() as u64 / access.get_bit_width()) - 1)
17261726
}
17271727

17281728
/// Returns true if the direct access can be used for the given type

0 commit comments

Comments
 (0)