Skip to content

Commit 75488d3

Browse files
peter-bpeter-b
committed
[Bug 14654] libscript: Detect and reject cyclic module dependencies.
1 parent 179bfdb commit 75488d3

2 files changed

Lines changed: 33 additions & 21 deletions

File tree

‎libscript/src/script-module.cpp‎

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -492,22 +492,28 @@ bool MCScriptEnsureModuleIsUsable(MCScriptModuleRef self)
492492
// If the module has already been ensured as usable, we are done.
493493
if (self -> is_usable)
494494
return true;
495+
496+
/* If the module is already marked as being checked for usability,
497+
* then there must be a cyclic module dependency. */
498+
if (self -> is_in_usable_check)
499+
return false;
500+
self -> is_in_usable_check = true;
495501

496502
// First ensure we can resolve all its external dependencies.
497503
for(uindex_t i = 0; i < self -> dependency_count; i++)
498504
{
499505
MCScriptModuleRef t_module;
500506
if (!MCScriptLookupModule(self -> dependencies[i] . name, t_module))
501-
return false;
507+
goto error_cleanup;
502508

503509
// A used module must not be a widget.
504510
if (t_module -> module_kind == kMCScriptModuleKindWidget)
505-
return false;
511+
goto error_cleanup;
506512

507513
// A used module must be usable - do this before resolving imports so
508514
// chained imports work.
509515
if (!MCScriptEnsureModuleIsUsable(t_module))
510-
return false;
516+
goto error_cleanup;
511517

512518
// Check all the imported definitions from the module, and compute indicies.
513519
for(uindex_t t_import = 0; t_import < self -> imported_definition_count; t_import++)
@@ -519,7 +525,7 @@ bool MCScriptEnsureModuleIsUsable(MCScriptModuleRef self)
519525

520526
MCScriptDefinition *t_def;
521527
if (!MCScriptLookupDefinitionInModule(t_module, t_import_def -> name, t_def))
522-
return false;
528+
goto error_cleanup;
523529

524530
MCScriptModuleRef t_mod;
525531
if (t_def -> kind == kMCScriptDefinitionKindExternal)
@@ -536,7 +542,7 @@ bool MCScriptEnsureModuleIsUsable(MCScriptModuleRef self)
536542
{
537543
if (t_import_def -> kind != kMCScriptDefinitionKindHandler ||
538544
t_def -> kind != kMCScriptDefinitionKindForeignHandler)
539-
return false;
545+
goto error_cleanup;
540546
}
541547

542548
// Check that signatures match.
@@ -547,7 +553,7 @@ bool MCScriptEnsureModuleIsUsable(MCScriptModuleRef self)
547553

548554
// Now create the instance we need.
549555
if (!MCScriptCreateInstanceOfModule(t_module, self -> dependencies[i] . instance))
550-
return false;
556+
goto error_cleanup;
551557
}
552558

553559
// Now build all the typeinfo's
@@ -581,12 +587,11 @@ bool MCScriptEnsureModuleIsUsable(MCScriptModuleRef self)
581587
{
582588
MCAutoStringRef t_name_string;
583589
if (!MCStringFormat(&t_name_string, "%@.%@", self -> name, t_public_name))
584-
return false;
590+
goto error_cleanup;
585591

586592
MCNewAutoNameRef t_name;
587593
if (!MCNameCreate(*t_name_string, &t_name))
588-
return false;
589-
594+
goto error_cleanup;
590595
// If the target type is an alias, named type or optional type then
591596
// we just create an alias. Otherwise it must be a record, foreign,
592597
// handler type - this means it must be named.
@@ -597,21 +602,21 @@ bool MCScriptEnsureModuleIsUsable(MCScriptModuleRef self)
597602
MCTypeInfoIsOptional(t_target_type))
598603
{
599604
if (!MCAliasTypeInfoCreate(*t_name, t_target_type, &t_typeinfo))
600-
return false;
605+
goto error_cleanup;
601606
}
602607
else
603608
{
604609
if (!MCNamedTypeInfoCreate(*t_name, &t_typeinfo))
605-
return false;
610+
goto error_cleanup;
606611

607612
if (!MCNamedTypeInfoBind(*t_typeinfo, t_target_type))
608-
return false;
613+
goto error_cleanup;
609614
}
610615
}
611616
else
612617
{
613618
if (!MCAliasTypeInfoCreate(kMCEmptyName, self -> types[t_type_def -> type] -> typeinfo, &t_typeinfo))
614-
return false;
619+
goto error_cleanup;
615620
}
616621
}
617622
else if (t_def -> kind == kMCScriptDefinitionKindExternal)
@@ -627,15 +632,15 @@ bool MCScriptEnsureModuleIsUsable(MCScriptModuleRef self)
627632
t_typeinfo = t_module -> types[static_cast<MCScriptTypeDefinition *>(t_import -> resolved_definition) -> type] -> typeinfo;
628633
}
629634
else
630-
return false;
635+
goto error_cleanup;
631636
}
632637
break;
633638
case kMCScriptTypeKindOptional:
634639
{
635640
MCScriptOptionalType *t_type;
636641
t_type = static_cast<MCScriptOptionalType *>(self -> types[i]);
637642
if (!MCOptionalTypeInfoCreate(self -> types[t_type -> type] -> typeinfo, &t_typeinfo))
638-
return false;
643+
goto error_cleanup;
639644
}
640645
break;
641646
case kMCScriptTypeKindForeign:
@@ -652,7 +657,7 @@ bool MCScriptEnsureModuleIsUsable(MCScriptModuleRef self)
652657
if (t_symbol == nil)
653658
{
654659
MCLog("Unable to resolve foreign type '%@'", t_type -> binding);
655-
return false;
660+
goto error_cleanup;
656661
}
657662

658663
t_typeinfo = MCValueRetain(*(MCTypeInfoRef *)t_symbol);
@@ -670,11 +675,11 @@ bool MCScriptEnsureModuleIsUsable(MCScriptModuleRef self)
670675
t_field . name = t_type -> fields[i] . name;
671676
t_field . type = self -> types[t_type -> fields[i] . type] -> typeinfo;
672677
if (!t_fields . Push(t_field))
673-
return false;
678+
goto error_cleanup;
674679
}
675680

676681
if (!MCRecordTypeInfoCreate(t_fields . Ptr(), t_type -> field_count, self -> types[t_type -> base_type] -> typeinfo, &t_typeinfo))
677-
return false;
682+
goto error_cleanup;
678683
}
679684
break;
680685
case kMCScriptTypeKindHandler:
@@ -689,11 +694,11 @@ bool MCScriptEnsureModuleIsUsable(MCScriptModuleRef self)
689694
t_parameter . mode = (MCHandlerTypeFieldMode)t_type -> parameters[i] . mode;
690695
t_parameter . type = self -> types[t_type -> parameters[i] . type] -> typeinfo;
691696
if (!t_parameters . Push(t_parameter))
692-
return false;
697+
goto error_cleanup;
693698
}
694699

695700
if (!MCHandlerTypeInfoCreate(t_parameters . Ptr(), t_type -> parameter_count, self -> types[t_type -> return_type] -> typeinfo, &t_typeinfo))
696-
return false;
701+
goto error_cleanup;
697702
}
698703
break;
699704
}
@@ -703,13 +708,18 @@ bool MCScriptEnsureModuleIsUsable(MCScriptModuleRef self)
703708

704709
// First validate the module - if this fails we do nothing more.
705710
if (!MCScriptValidateModule(self))
706-
return false;
711+
goto error_cleanup;
707712

708713
// Now bind all the public types.
709714

710715
self -> is_usable = true;
716+
self -> is_in_usable_check = false;
711717

712718
return true;
719+
720+
error_cleanup:
721+
self -> is_in_usable_check = false;
722+
return false;
713723
}
714724

715725
////////////////////////////////////////////////////////////////////////////////

‎libscript/src/script-private.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,8 @@ struct MCScriptModule: public MCScriptObject
382382
// After a module has been validated and had its dependencies resolved, this
383383
// var is true - not pickled
384384
bool is_usable : 1;
385+
// During the check for usability, this var is true - not pickled
386+
bool is_in_usable_check : 1;
385387

386388
// (computed) The number of slots needed by an instance - not pickled
387389
uindex_t slot_count;

0 commit comments

Comments
 (0)