From 897ce5596a6f706a97150c4c365b04427b2ec71f Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Thu, 31 Mar 2011 13:08:37 +0100 Subject: [PATCH] elflink: Detect circular dependencies during module load It's possible for the 'modules.dep' file to contain circular dependencies where module A has a dependency on module B, and module B has a depedency on module A. Currently what happens in the face of circular dependencies when trying to parse and load the modules in 'modules.dep' is that we get stuck in a loop in module_load_dependencies(), presumably eventually blowing the stack. To fix this we maintain a singly-linked list of module names that are in the process of having their dependencies loaded, but that have not completed the loading. Everytime we begin loading the dependencies for a new module we check to see if that module name is already on the linked list. If it is, we've discovered a circular reference so we refuse to load the dependencies and print a helpful message to the user which includes the circular dependency chain. It would be possible to cope with these circular references if we were to skip loading the dependencies for any modules that are on the linked list and defer resolving any module relocs until after all module images have been loaded into memory. However, it's more than likely that a circular reference is indicative of a bug and that the correct fix would be to extract the symbols causing the circular dependency into their own module. So we treat circular dependencies as an error case for now. Signed-off-by: Matt Fleming --- com32/lib/sys/module/exec.c | 84 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 80 insertions(+), 4 deletions(-) diff --git a/com32/lib/sys/module/exec.c b/com32/lib/sys/module/exec.c index 54182cc..fbe165d 100644 --- a/com32/lib/sys/module/exec.c +++ b/com32/lib/sys/module/exec.c @@ -343,11 +343,45 @@ int spawn_load(const char *name,const char **argv) */ } +/* + * Avoid circular dependencies. + * + * It's possible that someone messed up the modules.dep file and that + * it includes circular dependencies, so we need to take steps here to + * avoid looping in module_load_dependencies() forever. + * + * We build a singly-linked list of modules that are in the middle of + * being loaded. When they have completed loading their entry is + * removed from this list in LIFO order (new entries are always added + * to the head of the list). + */ +struct loading_dep { + const char *name; + struct module_dep *next; +}; +static struct loading_dep *loading_deps; + +/* + * Remember that because we insert elements in a LIFO order we need to + * start from the end of the list and work towards the front so that + * we print the modules in the order in which we tried to load them. + * + * Yay for recursive function calls. + */ +static void print_loading_dep(struct loading_dep *dep) +{ + if (dep) { + print_loading_dep(dep->next); + printf("\t\t\"%s\"\n", dep->name); + } +} + int module_load_dependencies(const char *name,const char *dep_file) { FILE *d_file=fopen(dep_file,"r"); char line[2048],aux[2048],temp_name[MODULE_NAME_SIZE],slbz[24]; int i=0,j=0,res=0; + struct loading_dep *dep; if(d_file==NULL) { @@ -355,6 +389,40 @@ int module_load_dependencies(const char *name,const char *dep_file) return -1; } + /* + * Are we already in the middle of loading this module's + * dependencies? + */ + for (dep = loading_deps; dep; dep = dep->next) { + if (!strcasecmp(dep->name, name)) + break; /* found */ + } + + if (dep) { + struct loading_dep *last, *prev; + + /* Dup! */ + printf("\t\tCircular depedency detected when loading " + "modules!\n"); + printf("\t\tModules dependency chain looks like this,\n\n"); + print_loading_dep(loading_deps); + printf("\n\t\t... and we tried to load \"%s\" again\n", name); + + return -1; + } else { + dep = malloc(sizeof(*dep)); + if (!dep) { + printf("Failed to alloc memory for loading_dep\n"); + return -1; + } + + dep->name = name; + dep->next = loading_deps; + + /* Insert at the head of the list */ + loading_deps = dep; + } + /* Note from feng: * new modues.dep has line like this: * a.c32: b.32 c.c32 d.c32 @@ -401,13 +469,21 @@ int module_load_dependencies(const char *name,const char *dep_file) if (strlen(temp_name)) { char *argv[2] = { NULL, NULL }; - - module_load_dependencies(temp_name, MODULES_DEP); - if (spawn_load(temp_name, argv) < 0) - continue; + int ret; + + ret = module_load_dependencies(temp_name, + MODULES_DEP); + if (!ret) { + if (spawn_load(temp_name, argv) < 0) + continue; + } } } + /* Remove our entry from the head of loading_deps */ + loading_deps = loading_deps->next; + free(dep); + return 0; } -- 2.7.4