From 1748e6641419e8a48f830caad072ed5b298577af Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Sun, 7 May 2023 16:51:12 -0400 Subject: [SV 63219] Support an "unload" function for loaded objects If a loaded object defines a symbol _gmk_unload, assume it's a function and invoke it whenever the loaded object is unloaded. Original implementation by Dmitry Goncharov * NEWS: Announce the change. * doc/make.texi: Describe the behavior. * src/gnumake.h: Add information to the comments. * src/makeint.h (unload_all): Declare a new function. * src/main.c (die): Invoke unload_all(). * src/load.c (unload_func_t): Declare a new type for unload. (struct load_list): Remember the unload symbol if it exists. (load_object): Move the parsing of the object name from load_file. Check for the _gmk_unload symbol and if found, remember it. (load_file): Allow load_object to do object filename parsing. (unload_file): Remove the load_list entry when unloading the object. (unload_all): Unload all the loaded objects. * tests/scripts/features/loadapi: Test the unload function. --- NEWS | 8 +- doc/make.texi | 78 ++++++++++-- src/gnumake.h | 11 +- src/load.c | 264 ++++++++++++++++++++++++----------------- src/main.c | 4 + src/makeint.h | 1 + tests/scripts/features/loadapi | 89 ++++++++++++++ 7 files changed, 337 insertions(+), 118 deletions(-) diff --git a/NEWS b/NEWS index 159d228e..295a2a4e 100644 --- a/NEWS +++ b/NEWS @@ -29,6 +29,12 @@ https://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=111&se your initialization function can check the provided ABI version to verify it's being loaded correctly. +* New feature: Unload function for loaded objects + When a loaded object needs to be unloaded by GNU Make, it will invoke an + unload function (if one is defined) beforehand that allows the object to + perform cleanup operations. + Original idea and implementation: Dmitry Goncharov + * New feature: Makefile warning reporting control A new option "--warn" controls reporting of warnings for makefiles. Actions can be set to "ignore", "warn", or "error". Two new warnings are reported: @@ -38,7 +44,7 @@ https://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=111&se deprecated, and is translated to "--warn=undefined-vars" internally. * New feature: Control warnings with the .WARNINGS variable - In addition to --warn from the command line, which take effect for make + In addition to --warn from the command line, which takes effect for make invoked recursively, warnings can be controlled only for the current instance of make using the .WARNINGS variable. diff --git a/doc/make.texi b/doc/make.texi index 2e76e900..57c13077 100644 --- a/doc/make.texi +++ b/doc/make.texi @@ -11990,7 +11990,7 @@ If the load succeeds @code{make} will invoke an initializing function. If @var{symbol-name} is provided, it will be used as the name of the initializing function. -If no @var{symbol-name} is provided, the initializing function name is created +If @var{symbol-name} is not provided, the initializing function name is created by taking the base file name of @var{object-file}, up to the first character which is not a valid symbol name character (alphanumerics and underscores are valid symbol name characters). To this prefix will be appended the suffix @@ -12030,6 +12030,31 @@ an error, you can use the @code{-load} directive instead of @code{load}. GNU to load. The failed object is not added to the @code{.LOADED} variable, which can then be consulted to determine if the load was successful. +@subsubheading Unloading Objects +@cindex unloading objects +@cindex unload function for loaded objects + +When GNU Make needs to unload a loaded object, either because it is exiting or +because the loaded object has been rebuilt, it will invoke an unload function. +The unload function name is created by taking the base file name of the object +file, up to the first character which is not a valid symbol name character +(alphanumerics and underscores are valid symbol name characters), then +appending the suffix @code{_gmk_unload}. + +If that function exists it will be called using the signature: + +@example +void _gmk_unload (void); +@end example + +If the function does not exist, it will not be called. + +Note that only one unload function may be defined per loaded object, +regardless of how many different setup methods are provided in that loaded +object. If your loaded object provides multiple setup methods that require +unload support it's up to you to coordinate which setups have been invoked in +the unload function. + @node Initializing Functions, Remaking Loaded Objects, load Directive, Loading Objects @subsection Initializing Functions @cindex loaded object initializing function @@ -12275,8 +12300,16 @@ take a prefix as an argument. First we can write the function in a file int plugin_is_GPL_compatible; -char * -gen_tmpfile(const char *nm, int argc, char **argv) +struct tmpfile @{ + struct tmpfile *next; + char *name; +@}; +static struct tmpfile *files = NULL; +@end group + +@group +static char * +gen_tmpfile(const char *nm, unsigned int argc, char **argv) @{ int fd; @@ -12290,6 +12323,11 @@ gen_tmpfile(const char *nm, int argc, char **argv) fd = mkstemp(buf); if (fd >= 0) @{ + struct tmpfile *new = malloc (sizeof (struct tmpfile)); + new->name = strdup (buf); + new->next = files; + files = new; + /* Don't leak the file descriptor. */ close (fd); return buf; @@ -12300,7 +12338,9 @@ gen_tmpfile(const char *nm, int argc, char **argv) gmk_free (buf); return NULL; @} +@end group +@group int mk_temp_gmk_setup (unsigned int abi, const gmk_floc *floc) @{ @@ -12311,6 +12351,23 @@ mk_temp_gmk_setup (unsigned int abi, const gmk_floc *floc) return 1; @} @end group + +@group +void +mk_temp_gmk_close () +@{ + while (files) + @{ + struct tmpfile *f = files; + files = f->next; + printf ("mk_temp removing %s\n", f->name); + remove (f->name); + free (f->name); + free (f); + @} + printf ("mk_temp plugin closed\n"); +@} +@end group @end example Next, we will write a @file{Makefile} that can build this shared object, load @@ -12320,8 +12377,9 @@ it, and use it: @group all: @@echo Temporary file: $(mk-temp tmpfile.) + @@echo Temporary file: $(mk-temp tmpfile.) -load mk_temp.so +-load mk_temp.so mk_temp.so: mk_temp.c $(CC) -shared -fPIC -o $@@ $< @@ -12332,7 +12390,7 @@ On MS-Windows, due to peculiarities of how shared objects are produced, the compiler needs to scan the @dfn{import library} produced when building @code{make}, typically called @file{libgnumake-@var{version}.dll.a}, where @var{version} is the version of the load object API. So the recipe to produce -a shared object will look on Windows like this (assuming the API version is +a shared object will look like this on Windows (assuming the API version is 1): @example @@ -12345,10 +12403,16 @@ mk_temp.dll: mk_temp.c Now when you run @code{make} you'll see something like: @example +@group $ make -mk_temp abi 1 plugin loaded from Makefile:4 cc -shared -fPIC -o mk_temp.so mk_temp.c -Temporary filename: tmpfile.A7JEwd +mk_temp abi 1 plugin loaded from Makefile:5 +Temporary file: tmpfile.OYkGMT +Temporary file: tmpfile.sYsJO0 +mk_temp removing tmpfile.sYsJO0 +mk_temp removing tmpfile.OYkGMT +mk_temp plugin closed +@end group @end example @node Integrating make, Features, Extending make, Top diff --git a/src/gnumake.h b/src/gnumake.h index 3ebe6621..b448a3c1 100644 --- a/src/gnumake.h +++ b/src/gnumake.h @@ -35,7 +35,16 @@ typedef char *(*gmk_func_ptr)(const char *nm, unsigned int argc, char **argv); int (unsigned int abi_version, const gmk_floc *flocp); - The abi_version will be set to GMK_ABI_VERSION. */ + The abi_version will be set to GMK_ABI_VERSION. + + When an object is unloaded by GNU Make, an unload method will be invoked. + The name of the method is derived from the filename of the object, with + _gmk_unload appended. It has the signature: + + void _gmk_unload (void); + + There will only be one unload method invoked regardless of the number of + setup methods within the object. */ #ifdef _WIN32 # ifdef GMK_BUILDING_MAKE diff --git a/src/load.c b/src/load.c index 519ec010..ab55feaf 100644 --- a/src/load.c +++ b/src/load.c @@ -24,8 +24,6 @@ this program. If not, see . */ #include #include -#define SYMBOL_EXTENSION "_gmk_setup" - #include "debug.h" #include "filedef.h" #include "variable.h" @@ -35,22 +33,32 @@ this program. If not, see . */ # define RTLD_GLOBAL 0 #endif +#define GMK_SETUP "_gmk_setup" +#define GMK_UNLOAD "_gmk_unload" + +typedef int (*setup_func_t)(unsigned int abi, const floc *flocp); +typedef void (*unload_func_t)(void); + struct load_list { struct load_list *next; const char *name; void *dlp; + unload_func_t unload; }; static struct load_list *loaded_syms = NULL; -typedef int (*setup_func_t)(unsigned int abi, const floc *flocp); - static setup_func_t load_object (const floc *flocp, int noerror, const char *ldname, - const char *symname) + const char *setupnm) { static void *global_dl = NULL; + char *buf; + const char *fp; + char *endp; + void *dlp; + struct load_list *new; setup_func_t symp; if (! global_dl) @@ -63,62 +71,97 @@ load_object (const floc *flocp, int noerror, const char *ldname, } } - symp = (setup_func_t) dlsym (global_dl, symname); - if (! symp) + /* Find the prefix of the ldname. */ + fp = strrchr (ldname, '/'); +#ifdef HAVE_DOS_PATHS + if (fp) { - struct load_list *new; - void *dlp = NULL; + const char *fp2 = strchr (fp, '\\'); - /* If the path has no "/", try the current directory first. */ - if (! strchr (ldname, '/') -#ifdef HAVE_DOS_PATHS - && ! strchr (ldname, '\\') + if (fp2 > fp) + fp = fp2; + } + else + fp = strrchr (ldname, '\\'); + /* The (improbable) case of d:foo. */ + if (fp && *fp && fp[1] == ':') + fp++; #endif - ) - dlp = dlopen (concat (2, "./", ldname), RTLD_LAZY|RTLD_GLOBAL); + if (!fp) + fp = ldname; + else + ++fp; - /* If we haven't opened it yet, try the default search path. */ - if (! dlp) - dlp = dlopen (ldname, RTLD_LAZY|RTLD_GLOBAL); + endp = buf = alloca (strlen (fp) + CSTRLEN (GMK_UNLOAD) + 1); + while (isalnum ((unsigned char) *fp) || *fp == '_') + *(endp++) = *(fp++); - /* Still no? Then fail. */ - if (! dlp) - { - const char *err = dlerror (); - if (noerror) - DB (DB_BASIC, ("%s\n", err)); - else - OS (error, flocp, "%s", err); - return NULL; - } + /* If we didn't find a symbol name yet, construct it from the prefix. */ + if (! setupnm) + { + memcpy (endp, GMK_SETUP, CSTRLEN (GMK_SETUP) + 1); + setupnm = buf; + } - DB (DB_VERBOSE, (_("Loaded shared object %s\n"), ldname)); + DB (DB_VERBOSE, (_("Loading symbol %s from %s\n"), setupnm, ldname)); - /* Assert that the GPL license symbol is defined. */ - symp = (setup_func_t) dlsym (dlp, "plugin_is_GPL_compatible"); - if (! symp) - OS (fatal, flocp, - _("loaded object %s is not declared to be GPL compatible"), - ldname); + symp = (setup_func_t) dlsym (global_dl, setupnm); + if (symp) + return symp; - symp = (setup_func_t) dlsym (dlp, symname); - if (! symp) - { - const char *err = dlerror (); - OSSS (fatal, flocp, _("failed to load symbol %s from %s: %s"), - symname, ldname, err); - } + /* If the path has no "/", try the current directory first. */ + dlp = NULL; + if (! strchr (ldname, '/') +#ifdef HAVE_DOS_PATHS + && ! strchr (ldname, '\\') +#endif + ) + dlp = dlopen (concat (2, "./", ldname), RTLD_LAZY|RTLD_GLOBAL); + + /* If we haven't opened it yet, try the default search path. */ + if (! dlp) + dlp = dlopen (ldname, RTLD_LAZY|RTLD_GLOBAL); + + /* Still no? Then fail. */ + if (! dlp) + { + const char *err = dlerror (); + if (noerror) + DB (DB_BASIC, ("%s\n", err)); + else + OS (error, flocp, "%s", err); + return NULL; + } - /* Add this symbol to a trivial lookup table. This is not efficient but - it's highly unlikely we'll be loading lots of objects, and we only - need it to look them up on unload, if we rebuild them. */ - new = xmalloc (sizeof (struct load_list)); - new->name = xstrdup (ldname); - new->dlp = dlp; - new->next = loaded_syms; - loaded_syms = new; + DB (DB_VERBOSE, (_("Loaded shared object %s\n"), ldname)); + + /* Assert that the GPL license symbol is defined. */ + symp = (setup_func_t) dlsym (dlp, "plugin_is_GPL_compatible"); + if (! symp) + OS (fatal, flocp, + _("loaded object %s is not declared to be GPL compatible"), ldname); + + symp = (setup_func_t) dlsym (dlp, setupnm); + if (! symp) + { + const char *err = dlerror (); + OSSS (fatal, flocp, _("failed to load symbol %s from %s: %s"), + setupnm, ldname, err); } + new = xcalloc (sizeof (struct load_list)); + new->next = loaded_syms; + loaded_syms = new; + new->name = ldname; + new->dlp = dlp; + + /* Compute the name of the unload function and look it up. */ + memcpy (endp, GMK_UNLOAD, CSTRLEN (GMK_UNLOAD) + 1); + + new->unload = (unload_func_t) dlsym (dlp, buf); + if (new->unload) + DB (DB_VERBOSE, (_("Detected symbol %s in %s\n"), buf, ldname)); + return symp; } @@ -126,9 +169,8 @@ int load_file (const floc *flocp, struct file *file, int noerror) { const char *ldname = file->name; - size_t nmlen = strlen (ldname); - char *new = alloca (nmlen + CSTRLEN (SYMBOL_EXTENSION) + 1); - char *symname = NULL; + char *buf; + char *setupnm = NULL; const char *fp; int r; setup_func_t symp; @@ -153,15 +195,15 @@ load_file (const floc *flocp, struct file *file, int noerror) OS (fatal, flocp, _("empty symbol name for load: %s"), ldname); /* Make a copy of the ldname part. */ - memcpy (new, ldname, l); - new[l] = '\0'; - ldname = new; - nmlen = l; + buf = alloca (strlen (ldname) + 1); + memcpy (buf, ldname, l); + buf[l] = '\0'; + ldname = buf; /* Make a copy of the symbol name part. */ - symname = new + l + 1; - memcpy (symname, fp, ep - fp); - symname[ep - fp] = '\0'; + setupnm = buf + l + 1; + memcpy (setupnm, fp, ep - fp); + setupnm[ep - fp] = '\0'; } } @@ -175,40 +217,8 @@ load_file (const floc *flocp, struct file *file, int noerror) 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, '/'); -#ifdef HAVE_DOS_PATHS - if (fp) - { - const char *fp2 = strchr (fp, '\\'); - - if (fp2 > fp) - fp = fp2; - } - else - fp = strrchr (ldname, '\\'); - /* The (improbable) case of d:foo. */ - if (fp && *fp && fp[1] == ':') - fp++; -#endif - if (!fp) - fp = ldname; - else - ++fp; - while (isalnum ((unsigned char) *fp) || *fp == '_') - *(p++) = *(fp++); - strcpy (p, SYMBOL_EXTENSION); - symname = new; - } - - 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, setupnm); if (! symp) return 0; @@ -228,22 +238,53 @@ load_file (const floc *flocp, struct file *file, int noerror) 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) - { - DB (DB_VERBOSE, (_("Unloading shared object %s\n"), name)); - rc = dlclose (d->dlp); - if (rc) - perror_with_name ("dlclose: ", d->name); - else - d->dlp = NULL; - break; - } - - return rc; + struct load_list **dp = &loaded_syms; + + /* Unload and remove the entry for this file. */ + while (*dp != NULL) + { + struct load_list *d = *dp; + + if (streq (d->name, name)) + { + int rc; + + DB (DB_VERBOSE, (_("Unloading shared object %s\n"), name)); + + if (d->unload) + (*d->unload) (); + + rc = dlclose (d->dlp); + if (rc) + perror_with_name ("dlclose: ", d->name); + + *dp = d->next; + free (d); + return rc; + } + + dp = &d->next; + } + + return 0; +} + +void +unload_all () +{ + while (loaded_syms) + { + struct load_list *d = loaded_syms; + loaded_syms = loaded_syms->next; + + if (d->unload) + (*d->unload) (); + + if (dlclose (d->dlp)) + perror_with_name ("dlclose: ", d->name); + + free (d); + } } #else @@ -264,4 +305,9 @@ unload_file (const char *name UNUSED) O (fatal, NILF, "INTERNAL: cannot unload when load is not supported"); } +void +unload_all () +{ +} + #endif /* MAKE_LOAD */ diff --git a/src/main.c b/src/main.c index 8215ed78..5343d47b 100644 --- a/src/main.c +++ b/src/main.c @@ -3815,6 +3815,10 @@ die (int status) if (verify_flag) verify_file_data_base (); + /* Unload plugins before jobserver integrity check in case a plugin + * participates in jobserver. */ + unload_all (); + clean_jobserver (status); if (output_context) diff --git a/src/makeint.h b/src/makeint.h index ce5c8452..d55ccb6f 100644 --- a/src/makeint.h +++ b/src/makeint.h @@ -674,6 +674,7 @@ int guile_gmake_setup (const floc *flocp); /* Loadable object support. Sets to the strcached name of the loaded file. */ int load_file (const floc *flocp, struct file *file, int noerror); int unload_file (const char *name); +void unload_all (void); /* Maintainer mode support */ #ifdef MAKE_MAINTAINER_MODE diff --git a/tests/scripts/features/loadapi b/tests/scripts/features/loadapi index 311260f9..50e5d05f 100644 --- a/tests/scripts/features/loadapi +++ b/tests/scripts/features/loadapi @@ -93,6 +93,32 @@ testapi_gmk_setup (unsigned int abi, const gmk_floc *floc) return 1; } + +int +alternative_setup () +{ + gmk_add_function ("test-expand", func_test, 1, 1, GMK_FUNC_DEFAULT); + 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 ("alternative_setup\n"); + + if (getenv ("TESTAPI_KEEP")) + return -1; + + return 1; +} + +void +testapi_gmk_unload () +{ + const char *s = getenv ("TESTAPI_VERBOSE"); + if (s && *s == '3') + printf ("testapi_gmk_unload\n"); +} + EOF close($F) or die "close: testapi.c: $!\n"; @@ -222,7 +248,70 @@ force:; ", '', "testapi_gmk_setup\n#MAKEFILE#:2\ntestapi.so\ntestapi_gmk_setup\n#MAKEFILE#:2\nhello\n#MAKE#: 'all' is up to date.\n"); } +my @names = ('testapi.so', './testapi.so', '#PWD#/testapi.so'); + +for my $name (@names) { + +# Test the make correctly figures out the name of the close function and runs +# the close function. +$ENV{TESTAPI_VERBOSE} = 3; +run_make_test(" +load $name +all:; \$(info \$(test-expand hello)) +", '', "testapi_gmk_setup\nhello\n#MAKE#: 'all' is up to date.\ntestapi_gmk_unload\n"); +} + +# Same as above, but the setup function is custom. +@names = ('testapi.so(alternative_setup)', './testapi.so(alternative_setup)', + '#PWD#/testapi.so(alternative_setup)'); +for my $name (@names) { + +# Test the close function. +$ENV{TESTAPI_VERBOSE} = 3; +run_make_test(" +load $name +all:; \$(info \$(test-expand hello)) +", '', "alternative_setup\nhello\n#MAKE#: 'all' is up to date.\ntestapi_gmk_unload\n"); +} + +# Test that makes runs the close function on failure. +$ENV{TESTAPI_VERBOSE} = 3; +run_make_test(q! +load testapi.so +all: bad_preqreq; : +!, '', "testapi_gmk_setup\n#MAKE#: *** No rule to make target 'bad_preqreq', needed by 'all'. Stop.\ntestapi_gmk_unload\n", 512); + +# Test that make unloads a shared object, calls the close function, loads +# the plugin again, and then calls the close function again on exit. +&utouch(-10, 'testapi.so'); +$ENV{TESTAPI_VERBOSE} = 3; +run_make_test(" +load testapi.so +all:; \$(info \$(test-expand hello)) +testapi.so: testapi.c; $sobuild +", '', "testapi_gmk_setup\ntestapi_gmk_unload\n$sobuild\ntestapi_gmk_setup\nhello\n#MAKE#: 'all' is up to date.\ntestapi_gmk_unload"); + +# Test that make unloads a shared object, calls the close function, loads +# the plugin again, and then calls the close function again on exit. +$ENV{TESTAPI_VERBOSE} = 3; +run_make_test(q! +load testapi.so +all:; $(info $(test-expand hello)) +testapi.so: force; $(info $@) +force:; +.PHONY: force +!, '', "testapi_gmk_setup\ntestapi_gmk_unload\ntestapi.so\ntestapi_gmk_setup\nhello\n#MAKE#: 'all' is up to date.\ntestapi_gmk_unload\n"); + unlink(qw(testapi.c testapi.so)) unless $keep; +# Test that make does not run the close function, unless the shared object +# loaded successfully. +unlink('testapi.so'); +$ENV{TESTAPI_VERBOSE} = 3; +run_make_test(q! +load testapi.so +all:; : +!, '', "#MAKEFILE#:2: testapi.so: cannot open shared object file: $ERR_no_such_file\n#MAKEFILE#:2: *** testapi.so: failed to load. Stop.\n", 512); + # This tells the test driver that the perl test script executed properly. 1; -- cgit v1.2.1