Skip to content

Commit

Permalink
Fix infinite loops with recursive types.
Browse files Browse the repository at this point in the history
Closes #1867

There are two different cases where infinite loops happen with recursive
types. First, a type may reference itself (`type Data = Data`). Second,
a type may reference itself inside some other type (`type Data =
vector<Data>`).

The first is fixed with a recursion limit. Since the type simply cannot
resolve, it doesn't get anywhere near codegen. You could detect cycles,
but that introduces some extra overhead and complexity that shouldn't
be needed in a "simple" function.

The second is fixed with an ad-hoc "occurs" check in type unification.
That just detects cycles and aborts if one is present. This could be
placed at some other place in the "resolve until convergence" loop, but
it seems best put closest to the source of the issue.
  • Loading branch information
evantypanski committed Oct 3, 2024
1 parent cd4bde3 commit cdd66ee
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 9 deletions.
9 changes: 5 additions & 4 deletions hilti/toolchain/include/ast/types/name.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

#pragma once

#include <memory>
#include <string>
#include <utility>

#include <hilti/ast/ast-context.h>
Expand All @@ -19,13 +17,16 @@ class Name : public UnqualifiedType {
bool isBuiltIn() const { return _builtin; }

// resolves recursively
UnqualifiedType* resolvedType() const {
UnqualifiedType* resolvedType(size_t recursion_depth = 0) const {
if ( ! _resolved_type_index )
return nullptr;

if ( recursion_depth > 1000 )
return nullptr;

auto t = context()->lookup(_resolved_type_index);
if ( auto n = t->tryAs<type::Name>() )
return n->resolvedType();
return n->resolvedType(recursion_depth + 1);
else
return t;
}
Expand Down
7 changes: 5 additions & 2 deletions hilti/toolchain/include/compiler/type-unifier.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <string>

#include <hilti/ast/forward.h>
#include <hilti/ast/node.h>

namespace hilti::type_unifier {

Expand Down Expand Up @@ -55,12 +56,14 @@ class Unifier {
/** Resets all state to start a new unification. */
void reset() {
_serial.clear();
_cd.clear();
_abort = false;
}

private:
std::string _serial; // builds up serialization incrementally
bool _abort = false; // if true, cannot compute serialization yet
std::string _serial; // builds up serialization incrementally
node::CycleDetector _cd; // used to check for invalid cycles
bool _abort = false; // if true, cannot compute serialization yet
};

namespace detail {
Expand Down
5 changes: 4 additions & 1 deletion hilti/toolchain/src/compiler/resolver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,10 @@ struct VisitorPass1 : visitor::MutatingPostOrder {
}

if ( n->resolvedTypeIndex() ) {
if ( auto resolved = n->resolvedType(); resolved->isOnHeap() ) {
auto resolved = n->resolvedType();
if ( ! resolved )
n->addError(util::fmt("type %s cannot be resolved by its name", n->id()));
else if ( resolved->isOnHeap() ) {
if ( auto qtype = n->parent()->tryAs<QualifiedType>() ) {
auto replace = false;

Expand Down
11 changes: 9 additions & 2 deletions hilti/toolchain/src/compiler/type-unifier.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
// Copyright (c) 2020-2023 by the Zeek Project. See LICENSE for details.

#include <optional>

#include <hilti/ast/ast-context.h>
#include <hilti/ast/builder/builder.h>
#include <hilti/ast/type.h>
Expand Down Expand Up @@ -203,9 +201,18 @@ class VisitorTypeUnifier : public visitor::MutatingPostOrder {
} // namespace

void type_unifier::Unifier::add(UnqualifiedType* t) {
// Occurs check: We cannot handle recursive types. Error out if we see the same
// node twice.
if ( _cd.haveSeen(t) ) {
t->addError("Invalid cycle detected in type");
abort();
}

if ( _abort )
return;

_cd.recordSeen(t);

if ( auto name = t->tryAs<type::Name>() ) {
t = name->resolvedType();
if ( ! t ) {
Expand Down
7 changes: 7 additions & 0 deletions tests/Baseline/hilti.types.id.recursive/output
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
[error] <...>/recursive.hlt:6:15-6:20: type Direct cannot be resolved by its name
[error] <...>/recursive.hlt:7:19-7:40: Invalid cycle detected in type
[error] <...>/recursive.hlt:8:17-8:32: Invalid cycle detected in type
[error] <...>/recursive.hlt:10:14-10:19: type Second cannot be resolved by its name
[error] <...>/recursive.hlt:11:15-11:19: type First cannot be resolved by its name
[error] hiltic: aborting after errors
13 changes: 13 additions & 0 deletions tests/hilti/types/id/recursive.hlt
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# @TEST-EXEC-FAIL: ${HILTIC} -p %INPUT > output 2>&1
# @TEST-EXEC: btest-diff output

module Test {

type Direct = Direct;
type Referenced = strong_ref<Referenced>;
type InVector = vector<InVector>;

type First = Second;
type Second = First;

}

0 comments on commit cdd66ee

Please sign in to comment.