From: Dmitry Goncharov Date: Mon, 12 Sep 2022 05:58:52 +0000 (-0400) Subject: [SV 63045] Reload each intact unloaded shared object X-Git-Tag: 4.3.90~8 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ee861a4e9f523d06d26ed612ad1a93b6ecb408de;p=thirdparty%2Fmake.git [SV 63045] Reload each intact unloaded shared object If makefile rules do not update an unloaded shared object, load it again. Avoid double loading of the same object if the setup function returns -1. * src/filedef.h (struct file): Add "unloaded" flag. * src/makeint.h (load_file): Take struct file *. (unload_file): Return int. * src/main.c (main): Reload unloaded shared objects if they weren't updated. * src/commands.c (execute_file_commands): Set "unloaded" and reset "loaded" when a shared object is unloaded. * src/read.c (eval): Set "loaded" and reset "unloaded" when a shared object is loaded. Add successfully loaded files to the db. * src/load.c (load_file): Check "loaded" to avoid double loading the same object. Fix a memory leak of string loaded. Return -1, rather than 1, if the object is already loaded. This fixes double loading of the same object when the setup routine returns -1. (load_object): Add a log message. (unload_file): Return an error on dlclose failure. Log a message. * tests/scripts/features/loadapi: Add new tests. --- diff --git a/src/commands.c b/src/commands.c index 3dd1953f..e7c0cc87 100644 --- a/src/commands.c +++ b/src/commands.c @@ -462,10 +462,15 @@ execute_file_commands (struct file *file) set_file_variables (file, file->stem); - /* If this is a loaded dynamic object, unload it before remaking. - Some systems don't support overwriting a loaded object. */ - if (file->loaded) - unload_file (file->name); + /* Some systems don't support overwriting a loaded object so if this one + unload it before remaking. Keep its name in .LOADED: it will be rebuilt + and loaded again. If rebuilding or loading again fail, then we'll exit + anyway and it won't matter. */ + if (file->loaded && unload_file (file->name) == 0) + { + file->loaded = 0; + file->unloaded = 1; + } /* Start the commands running. */ new_job (file); diff --git a/src/filedef.h b/src/filedef.h index 1eb8f61c..304a07f6 100644 --- a/src/filedef.h +++ b/src/filedef.h @@ -84,6 +84,7 @@ struct file unsigned int builtin:1; /* True if the file is a builtin rule. */ unsigned int precious:1; /* Non-0 means don't delete file on quit */ unsigned int loaded:1; /* True if the file is a loaded object. */ + unsigned int unloaded:1; /* True if this loaded object was unloaded. */ unsigned int low_resolution_time:1; /* Nonzero if this file's time stamp has only one-second resolution. */ unsigned int tried_implicit:1; /* Nonzero if have searched diff --git a/src/load.c b/src/load.c index 701ca4da..81cc3011 100644 --- a/src/load.c +++ b/src/load.c @@ -90,6 +90,8 @@ load_object (const floc *flocp, int noerror, const char *ldname, return NULL; } + DB (DB_VERBOSE, (_("Loaded shared object %s\n"), ldname)); + /* Assert that the GPL license symbol is defined. */ symp = (load_func_t) dlsym (dlp, "plugin_is_GPL_compatible"); if (! symp) @@ -119,19 +121,19 @@ load_object (const floc *flocp, int noerror, const char *ldname, } int -load_file (const floc *flocp, const char **ldname, int noerror) +load_file (const floc *flocp, struct file *file, int noerror) { - size_t nmlen = strlen (*ldname); + const char *ldname = file->name; + size_t nmlen = strlen (ldname); char *new = alloca (nmlen + CSTRLEN (SYMBOL_EXTENSION) + 1); char *symname = NULL; - char *loaded; const char *fp; int r; load_func_t symp; /* Break the input into an object file name and a symbol name. If no symbol name was provided, compute one from the object file name. */ - fp = strchr (*ldname, '('); + fp = strchr (ldname, '('); if (fp) { const char *ep; @@ -142,16 +144,16 @@ load_file (const floc *flocp, const char **ldname, int noerror) ep = strchr (fp+1, ')'); if (ep && ep[1] == '\0') { - size_t l = fp - *ldname;; + size_t l = fp - ldname; ++fp; if (fp == ep) - OS (fatal, flocp, _("Empty symbol name for load: %s"), *ldname); + OS (fatal, flocp, _("Empty symbol name for load: %s"), ldname); /* Make a copy of the ldname part. */ - memcpy (new, *ldname, l); + memcpy (new, ldname, l); new[l] = '\0'; - *ldname = new; + ldname = new; nmlen = l; /* Make a copy of the symbol name part. */ @@ -161,22 +163,22 @@ load_file (const floc *flocp, const char **ldname, int noerror) } } - /* Add this name to the string cache so it can be reused later. */ - *ldname = strcache_add (*ldname); + /* Make sure this name is in the string cache. */ + ldname = file->name = strcache_add (ldname); - /* If this object has been loaded, we're done. */ - loaded = allocated_variable_expand ("$(.LOADED)"); - fp = strstr (loaded, *ldname); - r = fp && (fp==loaded || fp[-1]==' ') && (fp[nmlen]=='\0' || fp[nmlen]==' '); - if (r) - goto exit; + /* If this object has been loaded, we're done: return -1 to ensure make does + not rebuild again. If a rebuild is allowed it was set up when this + object was initially loaded. */ + file = lookup_file (ldname); + if (file && file->loaded) + return -1; /* If we didn't find a symbol name yet, construct it from the ldname. */ if (! symname) { char *p = new; - fp = strrchr (*ldname, '/'); + fp = strrchr (ldname, '/'); #ifdef HAVE_DOS_PATHS if (fp) { @@ -186,13 +188,13 @@ load_file (const floc *flocp, const char **ldname, int noerror) fp = fp2; } else - fp = strrchr (*ldname, '\\'); + fp = strrchr (ldname, '\\'); /* The (improbable) case of d:foo. */ if (fp && *fp && fp[1] == ':') fp++; #endif if (!fp) - fp = *ldname; + fp = ldname; else ++fp; while (isalnum ((unsigned char) *fp) || *fp == '_') @@ -201,56 +203,48 @@ load_file (const floc *flocp, const char **ldname, int noerror) symname = new; } - DB (DB_VERBOSE, (_("Loading symbol %s from %s\n"), symname, *ldname)); + DB (DB_VERBOSE, (_("Loading symbol %s from %s\n"), symname, ldname)); /* Load it! */ - symp = load_object (flocp, noerror, *ldname, symname); + symp = load_object (flocp, noerror, ldname, symname); if (! symp) return 0; /* Invoke the symbol. */ r = (*symp) (flocp); - /* If it succeeded, add the load file to the loaded variable. - Anything other than 0, including -1, is a success. */ + /* If the load didn't fail, add the file to the .LOADED variable. */ if (r) - { - size_t loadlen = strlen (loaded); - char *newval = alloca (loadlen + strlen (*ldname) + 2); - /* Don't add a space if it's empty. */ - if (loadlen) - { - memcpy (newval, loaded, loadlen); - newval[loadlen++] = ' '; - } - strcpy (&newval[loadlen], *ldname); - do_variable_definition (flocp, ".LOADED", newval, o_default, f_simple, 0); - } + do_variable_definition(flocp, ".LOADED", ldname, o_file, f_append_value, 0); - exit: - free (loaded); return r; } -void +int unload_file (const char *name) { + int rc = 0; struct load_list *d; for (d = loaded_syms; d != NULL; d = d->next) if (streq (d->name, name) && d->dlp) { - if (dlclose (d->dlp)) + DB (DB_VERBOSE, (_("Unloading shared object %s\n"), name)); + rc = dlclose (d->dlp); + if (rc) perror_with_name ("dlclose: ", d->name); - d->dlp = NULL; + else + d->dlp = NULL; break; } + + return rc; } #else int -load_file (const floc *flocp, const char **ldname UNUSED, int noerror) +load_file (const floc *flocp, struct file *file UNUSED, int noerror) { if (! noerror) O (fatal, flocp, @@ -259,7 +253,7 @@ load_file (const floc *flocp, const char **ldname UNUSED, int noerror) return 0; } -void +int unload_file (const char *name UNUSED) { O (fatal, NILF, "INTERNAL: Cannot unload when load is not supported"); diff --git a/src/main.c b/src/main.c index 1deb926d..91eae580 100644 --- a/src/main.c +++ b/src/main.c @@ -2429,6 +2429,24 @@ main (int argc, char **argv, char **envp) break; case us_none: + { + /* Reload any unloaded shared objects. Do not re-exec to have + that shared object loaded: a re-exec would cause an infinite + loop, because the shared object was not updated. */ + struct goaldep *d; + + for (d = read_files; d; d = d->next) + if (d->file->unloaded) + { + struct file *f = d->file; + /* Load the file. 0 means failure. */ + if (load_file (&d->floc, f, 0) == 0) + OS (fatal, &d->floc, _("%s: failed to load"), f->name); + f->unloaded = 0; + f->loaded = 1; + } + } + /* No makefiles needed to be updated. If we couldn't read some included file that we care about, fail. */ if (0) diff --git a/src/makeint.h b/src/makeint.h index dd45218d..d941ff3d 100644 --- a/src/makeint.h +++ b/src/makeint.h @@ -494,6 +494,8 @@ extern struct rlimit stack_limit; +struct file; + /* Specify the location of elements read from makefiles. */ typedef struct { @@ -616,8 +618,8 @@ int guile_gmake_setup (const floc *flocp); /* Loadable object support. Sets to the strcached name of the loaded file. */ typedef int (*load_func_t)(const floc *flocp); -int load_file (const floc *flocp, const char **filename, int noerror); -void unload_file (const char *name); +int load_file (const floc *flocp, struct file *file, int noerror); +int unload_file (const char *name); /* Maintainer mode support */ #ifdef MAKE_MAINTAINER_MODE diff --git a/src/read.c b/src/read.c index 97e71bac..94e1ce90 100644 --- a/src/read.c +++ b/src/read.c @@ -954,28 +954,37 @@ eval (struct ebuffer *ebuf, int set_default) struct nameseq *next = files->next; const char *name = files->name; struct goaldep *deps; + struct file *f; int r; - /* Load the file. 0 means failure. */ - r = load_file (&ebuf->floc, &name, noerror); - if (! r && ! noerror) - OS (fatal, &ebuf->floc, _("%s: failed to load"), name); + { + struct file file = {0}; + file.name = name; + /* Load the file. 0 means failure. */ + r = load_file (&ebuf->floc, &file, noerror); + if (! r && ! noerror) + OS (fatal, &ebuf->floc, _("%s: failed to load"), name); + name = file.name; + } + + f = lookup_file (name); + if (!f) + f = enter_file (name); + f->loaded = 1; + f->unloaded = 0; free_ns (files); files = next; - /* Return of -1 means a special load: don't rebuild it. */ + /* Return of -1 means don't ever try to rebuild. */ if (r == -1) continue; - /* It succeeded, so add it to the list "to be rebuilt". */ + /* Otherwise add it to the list to be rebuilt. */ deps = alloc_goaldep (); deps->next = read_files; read_files = deps; - deps->file = lookup_file (name); - if (deps->file == 0) - deps->file = enter_file (name); - deps->file->loaded = 1; + deps->file = f; } continue; diff --git a/tests/scripts/features/loadapi b/tests/scripts/features/loadapi index ba149281..f4d335a7 100644 --- a/tests/scripts/features/loadapi +++ b/tests/scripts/features/loadapi @@ -24,8 +24,12 @@ print $F <<'EOF' ; #include "gnumake.h" +char* getenv (const char*); + int plugin_is_GPL_compatible; +int testapi_gmk_setup (); + static char * test_eval (const char *buf) { @@ -73,6 +77,13 @@ testapi_gmk_setup () gmk_add_function ("test-noexpand", func_test, 1, 1, GMK_FUNC_NOEXPAND); gmk_add_function ("test-eval", func_test, 1, 1, GMK_FUNC_DEFAULT); gmk_add_function ("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_.", func_test, 0, 0, 0); + + if (getenv ("TESTAPI_VERBOSE")) + printf("testapi_gmk_setup\n"); + + if (getenv ("TESTAPI_KEEP")) + return -1; + return 1; } EOF @@ -121,6 +132,77 @@ all:;@echo '$(test-noexpand $(TEST))' !, '', "\$(TEST)\n"); + +# During all subsequent tests testapi.so exists. +# +my @loads = ('', q! +load testapi.so +load testapi.so +-load testapi.so +-load testapi.so +$(eval load testapi.so) +$(eval -load testapi.so) +!); + +for my $extra_loads (@loads) { +my $n = 5; +if ($extra_loads) { + $n = 12; +} +# sv 63045. +# Test that having unloaded a shared object make loads it again, even if the +# shared object is not updated. +$ENV{TESTAPI_VERBOSE} = 1; +run_make_test(" +load testapi.so +$extra_loads +all:; \$(info \$(test-expand hello)) +testapi.so: force; \$(info \$@) +force:; +.PHONY: force +", '', "testapi_gmk_setup\ntestapi.so\ntestapi_gmk_setup\nhello\n#MAKE#: 'all' is up to date.\n"); + +# sv 63045. +# Same as above, but testapi_gmk_setup returned -1. +$ENV{TESTAPI_KEEP} = 1; +$ENV{TESTAPI_VERBOSE} = 1; +run_make_test(" +load testapi.so +$extra_loads +all:; \$(info \$(test-expand hello)) +testapi.so: force; \$(info \$@) +force:; +.PHONY: force +", '', "testapi_gmk_setup\nhello\n#MAKE#: 'all' is up to date.\n"); + +# sv 63045. +# Test that make exits, unless make can successfully update an unloaded shared +# object. +$ENV{TESTAPI_VERBOSE} = 1; +run_make_test(" +load testapi.so +$extra_loads +all:; \$(info \$(test-expand hello)) +testapi.so: force; false +force:; +.PHONY: force +", '', "testapi_gmk_setup\nfalse\n#MAKE#: *** [#MAKEFILE#:$n: testapi.so] Error 1\n", 512); + +# sv 63045. +# Same as above, but testapi_gmk_setup returned -1. +$ENV{TESTAPI_KEEP} = 1; +$ENV{TESTAPI_VERBOSE} = 1; +run_make_test(" +load testapi.so +$extra_loads +all:; \$(info \$(test-expand hello)) +testapi.so: force; false +force:; +.PHONY: force +", '', "testapi_gmk_setup\nhello\n#MAKE#: 'all' is up to date.\n"); + +} + unlink(qw(testapi.c testapi.so)) unless $keep; # This tells the test driver that the perl test script executed properly.