Skip to content

Commit 558ef84

Browse files
author
6d7a
committed
fix(linker): build reference graph to stop at circular references in recursion detection
1 parent 6a0ab6c commit 558ef84

File tree

8 files changed

+153
-44
lines changed

8 files changed

+153
-44
lines changed

‎rasn-compiler-tests/tests/structured_types.rs‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,3 +486,104 @@ e2e_pdu!(
486486
}
487487
"#
488488
);
489+
490+
e2e_pdu!(
491+
nested_recursion_ping_pong,
492+
r#"
493+
TypeDescription ::= CHOICE {
494+
array [1] IMPLICIT SEQUENCE {
495+
elementType [2] TypeSpecification
496+
},
497+
structure [2] IMPLICIT SEQUENCE {
498+
components [1] IMPLICIT SEQUENCE OF SEQUENCE {
499+
componentType [1] TypeSpecification
500+
}
501+
}
502+
}
503+
504+
TypeSpecification ::= CHOICE {
505+
typeDescription TypeDescription
506+
}
507+
508+
VariableSpecification ::= CHOICE {
509+
variableDescription [2] IMPLICIT SEQUENCE {
510+
typeSpecification TypeSpecification
511+
}
512+
}
513+
"#,
514+
r#"
515+
#[doc = " Inner type "]
516+
#[derive(AsnType, Debug, Clone, Decode, Encode, PartialEq, Eq, Hash)]
517+
#[rasn(tag(context, 1))]
518+
pub struct TypeDescriptionArray {
519+
#[rasn(tag(context, 2), identifier = "elementType")]
520+
pub element_type: TypeSpecification,
521+
}
522+
impl TypeDescriptionArray {
523+
pub fn new(element_type: TypeSpecification) -> Self {
524+
Self { element_type }
525+
}
526+
}
527+
#[doc = " Anonymous SEQUENCE OF member "]
528+
#[derive(AsnType, Debug, Clone, Decode, Encode, PartialEq, Eq, Hash)]
529+
#[rasn(identifier = "SEQUENCE")]
530+
pub struct AnonymousTypeDescriptionStructureComponents {
531+
#[rasn(tag(context, 1), identifier = "componentType")]
532+
pub component_type: TypeSpecification,
533+
}
534+
impl AnonymousTypeDescriptionStructureComponents {
535+
pub fn new(component_type: TypeSpecification) -> Self {
536+
Self { component_type }
537+
}
538+
}
539+
#[doc = " Inner type "]
540+
#[derive(AsnType, Debug, Clone, Decode, Encode, PartialEq, Eq, Hash)]
541+
#[rasn(delegate, tag(context, 1))]
542+
pub struct TypeDescriptionStructureComponents(
543+
pub SequenceOf<AnonymousTypeDescriptionStructureComponents>,
544+
);
545+
#[doc = " Inner type "]
546+
#[derive(AsnType, Debug, Clone, Decode, Encode, PartialEq, Eq, Hash)]
547+
#[rasn(tag(context, 2))]
548+
pub struct TypeDescriptionStructure {
549+
#[rasn(tag(context, 1))]
550+
pub components: TypeDescriptionStructureComponents,
551+
}
552+
impl TypeDescriptionStructure {
553+
pub fn new(components: TypeDescriptionStructureComponents) -> Self {
554+
Self { components }
555+
}
556+
}
557+
#[derive(AsnType, Debug, Clone, Decode, Encode, PartialEq, Eq, Hash)]
558+
#[rasn(choice)]
559+
pub enum TypeDescription {
560+
#[rasn(tag(context, 1))]
561+
array(TypeDescriptionArray),
562+
#[rasn(tag(context, 2))]
563+
structure(TypeDescriptionStructure),
564+
}
565+
#[derive(AsnType, Debug, Clone, Decode, Encode, PartialEq, Eq, Hash)]
566+
#[rasn(choice, automatic_tags)]
567+
pub enum TypeSpecification {
568+
typeDescription(Box<TypeDescription>),
569+
}
570+
#[doc = " Inner type "]
571+
#[derive(AsnType, Debug, Clone, Decode, Encode, PartialEq, Eq, Hash)]
572+
#[rasn(tag(context, 2))]
573+
pub struct VariableSpecificationVariableDescription {
574+
#[rasn(identifier = "typeSpecification")]
575+
pub type_specification: TypeSpecification,
576+
}
577+
impl VariableSpecificationVariableDescription {
578+
pub fn new(type_specification: TypeSpecification) -> Self {
579+
Self { type_specification }
580+
}
581+
}
582+
#[derive(AsnType, Debug, Clone, Decode, Encode, PartialEq, Eq, Hash)]
583+
#[rasn(choice)]
584+
pub enum VariableSpecification {
585+
#[rasn(tag(context, 2))]
586+
variableDescription(VariableSpecificationVariableDescription),
587+
}
588+
"#
589+
);

‎rasn-compiler/src/intermediate/encoding_rules/per_visible.rs‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,10 @@ impl PerVisibleAlphabetConstraints {
160160
let mut permitted_alphabet =
161161
PerVisibleAlphabetConstraints::default_for(string_type);
162162
for c in &c_string.constraints {
163-
PerVisibleAlphabetConstraints::try_new(c, c_string.ty)?
164-
.map(|mut p| permitted_alphabet += &mut p);
163+
if let Some(mut p) = PerVisibleAlphabetConstraints::try_new(c, c_string.ty)?
164+
{
165+
permitted_alphabet += &mut p
166+
}
165167
}
166168
Ok(Some(permitted_alphabet))
167169
} else {
@@ -195,10 +197,7 @@ impl PerVisibleAlphabetConstraints {
195197
}
196198
}
197199

198-
fn find_string_index(
199-
value: &String,
200-
char_set: &BTreeMap<usize, char>,
201-
) -> Result<usize, GrammarError> {
200+
fn find_string_index(value: &str, char_set: &BTreeMap<usize, char>) -> Result<usize, GrammarError> {
202201
let as_char = value.chars().next().unwrap();
203202
find_char_index(char_set, as_char)
204203
}
@@ -362,7 +361,7 @@ impl TryFrom<Option<&SubtypeElement>> for PerVisibleRangeConstraints {
362361
extensible: _,
363362
}) => per_visible_range_constraints(
364363
matches!(subtype, ASN1Type::Integer(_)),
365-
subtype.constraints().unwrap_or(&mut vec![]),
364+
subtype.constraints().unwrap_or(&vec![]),
366365
),
367366
x => {
368367
println!("{x:?}");
@@ -404,7 +403,7 @@ impl PerVisible for SubtypeElement {
404403
extensible: _,
405404
} => s
406405
.constraints()
407-
.map_or(false, |c| c.iter().any(|c| c.per_visible())),
406+
.is_some_and(|c| c.iter().any(|c| c.per_visible())),
408407
SubtypeElement::ValueRange {
409408
min: _,
410409
max: _,
@@ -419,7 +418,7 @@ impl PerVisible for SubtypeElement {
419418

420419
pub fn per_visible_range_constraints(
421420
signed: bool,
422-
constraint_list: &Vec<Constraint>,
421+
constraint_list: &[Constraint],
423422
) -> Result<PerVisibleRangeConstraints, GrammarError> {
424423
let mut constraints = if signed {
425424
PerVisibleRangeConstraints::default()

‎rasn-compiler/src/intermediate/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1077,7 +1077,7 @@ impl IntegerType {
10771077
/// Returns the Integer type with more restrictions
10781078
/// - an IntegerType with a smaller set of values is considered more restrictive
10791079
/// - an unsigned IntegerType is considered more restrictive if the size of the set of values is equal
1080-
/// if equal, `self` is returned
1080+
/// if equal, `self` is returned
10811081
pub fn max_restrictive(self, rhs: IntegerType) -> IntegerType {
10821082
match (self, rhs) {
10831083
(x, y) if x == y => x,

‎rasn-compiler/src/lexer/enumerated.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ pub fn enumerated_value(input: Input<'_>) -> ParserResult<'_, ToplevelValueDefin
3030
}),
3131
name: n.to_string(),
3232
parameterization: params,
33-
associated_type: p.clone().into(),
33+
associated_type: p.clone(),
3434
value: ASN1Value::EnumeratedValue {
3535
enumerated: p.as_str().into_owned(),
3636
enumerable: e.to_string(),

‎rasn-compiler/src/lexer/error.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ impl<'a> ErrorTree<'a> {
229229
match self {
230230
ErrorTree::Leaf { kind, .. } => kind == &ErrorKind::Nom(nom::error::ErrorKind::Eof),
231231
ErrorTree::Branch { tip, .. } => tip.is_eof_error(),
232-
ErrorTree::Fork { branches } => branches.back().map_or(false, |b| b.is_eof_error()),
232+
ErrorTree::Fork { branches } => branches.back().is_some_and(|b| b.is_eof_error()),
233233
}
234234
}
235235
}

‎rasn-compiler/src/validator/linking/constraints.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,8 @@ impl SubtypeElement {
9797
max,
9898
extensible: _,
9999
} => {
100-
min.as_ref().map_or(false, |s| s.is_elsewhere_declared())
101-
|| max.as_ref().map_or(false, |s| s.is_elsewhere_declared())
100+
min.as_ref().is_some_and(|s| s.is_elsewhere_declared())
101+
|| max.as_ref().is_some_and(|s| s.is_elsewhere_declared())
102102
}
103103
SubtypeElement::SizeConstraint(s) => s.has_cross_reference(),
104104
SubtypeElement::TypeConstraint(t) => t.references_class_by_name(),

‎rasn-compiler/src/validator/linking/mod.rs‎

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ impl ToplevelDefinition {
6767
| ToplevelDefinition::Type(ToplevelTypeDefinition {
6868
ty: ASN1Type::SetOf(s),
6969
..
70-
}) => s.element_type.constraints().map_or(false, |constraints| {
70+
}) => s.element_type.constraints().is_some_and(|constraints| {
7171
constraints
7272
.iter()
7373
.any(|c| matches!(c, Constraint::Parameter(_)))
@@ -87,15 +87,14 @@ impl ToplevelDefinition {
8787
}
8888
match &t.ty {
8989
ASN1Type::Enumerated(e) => {
90-
return e
91-
.members
92-
.iter()
93-
.find_map(|m| (&m.name == identifier).then(|| ASN1Value::Integer(m.index)))
90+
return e.members.iter().find_map(|m| {
91+
(&m.name == identifier).then_some(ASN1Value::Integer(m.index))
92+
})
9493
}
9594
ASN1Type::Integer(i) => {
9695
return i.distinguished_values.as_ref().and_then(|dv| {
9796
dv.iter().find_map(|d| {
98-
(&d.name == identifier).then(|| ASN1Value::Integer(d.value))
97+
(&d.name == identifier).then_some(ASN1Value::Integer(d.value))
9998
})
10099
})
101100
}
@@ -117,9 +116,16 @@ impl ToplevelDefinition {
117116

118117
/// Checks if at any depth down the arbitrarily nested `self`, an elsewhere declared type with the name `name` exists.
119118
/// Sequence Ofs and Set Ofs break the recursion tree, because they use heap-based data structures.
120-
pub fn recurses(&self, name: &str, tlds: &BTreeMap<String, ToplevelDefinition>) -> bool {
119+
pub fn recurses(
120+
&self,
121+
name: &str,
122+
tlds: &BTreeMap<String, ToplevelDefinition>,
123+
reference_graph: Vec<&str>,
124+
) -> bool {
121125
match self {
122-
ToplevelDefinition::Type(ToplevelTypeDefinition { ty, .. }) => ty.recurses(name, tlds),
126+
ToplevelDefinition::Type(ToplevelTypeDefinition { ty, .. }) => {
127+
ty.recurses(name, tlds, reference_graph)
128+
}
123129
_ => false, // TODO: Check recursion for values and information objects
124130
}
125131
}
@@ -176,7 +182,7 @@ impl ToplevelDefinition {
176182
/// will be represented in the generated rust bindings.
177183
/// ### Params
178184
/// * `tlds` - vector of other top-level declarations that will be searched as the method resolves a reference
179-
/// returns `true` if the reference was resolved successfully.
185+
/// returns `true` if the reference was resolved successfully.
180186
pub fn link_constraint_reference(
181187
&mut self,
182188
tlds: &BTreeMap<String, ToplevelDefinition>,
@@ -276,16 +282,10 @@ impl ASN1Type {
276282
pub fn has_choice_selection_type(&self) -> bool {
277283
match self {
278284
ASN1Type::ChoiceSelectionType(_) => true,
279-
ASN1Type::Sequence(s) | ASN1Type::Set(s) => s
280-
.members
281-
.iter()
282-
.map(|m| m.ty.has_choice_selection_type())
283-
.any(|b| b),
284-
ASN1Type::Choice(c) => c
285-
.options
286-
.iter()
287-
.map(|o| o.ty.has_choice_selection_type())
288-
.any(|b| b),
285+
ASN1Type::Sequence(s) | ASN1Type::Set(s) => {
286+
s.members.iter().any(|m| m.ty.has_choice_selection_type())
287+
}
288+
ASN1Type::Choice(c) => c.options.iter().any(|o| o.ty.has_choice_selection_type()),
289289
ASN1Type::SequenceOf(s) | ASN1Type::SetOf(s) => {
290290
s.element_type.has_choice_selection_type()
291291
}
@@ -566,24 +566,33 @@ impl ASN1Type {
566566

567567
/// Checks if at any depth down the arbitrarily nested `self`, an elsewhere declared type with the name `name` exists.
568568
/// Sequence Ofs and Set Ofs break the recursion tree, because they use heap-based data structures.
569-
pub fn recurses(&self, name: &str, tlds: &BTreeMap<String, ToplevelDefinition>) -> bool {
569+
/// The `reference_graph` serves to detect circular references in the recursion tree. If a circular reference is detected,
570+
/// recursion detection is stopped. The circular reference will be marked recursive once the respective type is captured mutably in `mark_recursive`.
571+
pub fn recurses(
572+
&self,
573+
name: &str,
574+
tlds: &BTreeMap<String, ToplevelDefinition>,
575+
mut reference_graph: Vec<&str>,
576+
) -> bool {
570577
match self {
571578
ASN1Type::ElsewhereDeclaredType(DeclarationElsewhere { identifier, .. }) => {
572-
identifier == name
573-
|| tlds
574-
.get(identifier)
575-
.is_some_and(|tld| tld.recurses(name, tlds))
579+
!reference_graph.contains(&identifier.as_str())
580+
&& (identifier == name
581+
|| tlds.get(identifier).is_some_and(|tld| {
582+
reference_graph.push(identifier);
583+
tld.recurses(name, tlds, reference_graph)
584+
}))
576585
}
577586
ASN1Type::Choice(c) => c.options.iter().any(|opt|
578587
// if an option is already marked recursive,
579588
// it will be boxed and constitute a recursion
580589
// boundary between `self` and the option type
581-
!opt.is_recursive && opt.ty.recurses(name, tlds)),
590+
!opt.is_recursive && opt.ty.recurses(name, tlds, reference_graph.clone())),
582591
ASN1Type::Sequence(s) | ASN1Type::Set(s) => s.members.iter().any(|m|
583592
// if a member is already marked recursive,
584593
// it will be boxed and thus constitutes a recursion
585594
// boundary between `self` and the member type
586-
!m.is_recursive && m.ty.recurses(name, tlds)),
595+
!m.is_recursive && m.ty.recurses(name, tlds, reference_graph.clone())),
587596
_ => false,
588597
}
589598
}
@@ -598,7 +607,7 @@ impl ASN1Type {
598607
ASN1Type::Choice(choice) => {
599608
let mut children = Vec::new();
600609
for option in &mut choice.options {
601-
option.is_recursive = option.ty.recurses(name, tlds);
610+
option.is_recursive = option.ty.recurses(name, tlds, Vec::new());
602611
let opt_ty_name = option.ty.as_str().into_owned();
603612
let mut opt_children = option.ty.mark_recursive(&opt_ty_name, tlds)?;
604613
if opt_children.iter().any(|id: &Cow<'_, str>| id == name) {
@@ -612,7 +621,7 @@ impl ASN1Type {
612621
ASN1Type::Set(s) | ASN1Type::Sequence(s) => {
613622
let mut children = Vec::new();
614623
for member in &mut s.members {
615-
member.is_recursive = member.ty.recurses(name, tlds);
624+
member.is_recursive = member.ty.recurses(name, tlds, Vec::new());
616625
let mem_ty_name = member.ty.as_str().into_owned();
617626
let mut mem_children = member.ty.mark_recursive(&mem_ty_name, tlds)?;
618627
if mem_children.iter().any(|id: &Cow<'_, str>| id == name) {
@@ -776,7 +785,7 @@ impl ASN1Type {
776785
m.ty.contains_constraint_reference()
777786
|| m.default_value
778787
.as_ref()
779-
.map_or(false, |d| d.is_elsewhere_declared())
788+
.is_some_and(|d| d.is_elsewhere_declared())
780789
|| m.constraints.iter().any(|c| c.has_cross_reference())
781790
})
782791
}

‎rasn-compiler/src/validator/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ impl Validator {
334334
.get(key)
335335
.map(|t| match t {
336336
ToplevelDefinition::Type(t) => t.ty.references_class_by_name(),
337-
ToplevelDefinition::Information(i) => i.class.as_ref().map_or(false, |c| match c {
337+
ToplevelDefinition::Information(i) => i.class.as_ref().is_some_and(|c| match c {
338338
ClassLink::ByReference(_) => false,
339339
ClassLink::ByName(_) => true,
340340
}),

0 commit comments

Comments
 (0)