From 8c2aa889bbe7fbd59d2655d6dc5a26cf86dfecf7 Mon Sep 17 00:00:00 2001 From: Dmitry Goncharov Date: Sat, 23 Apr 2022 20:34:18 -0400 Subject: [PATCH] [SV 62324] Simplify set_file_variables by passing in the stem Previously we always used the file->stem value as our stem in set_file_variables(); when that wasn't correct we had to temporarily set that value while the function was called, then reset it afterward. This led to issues (for example when we assumed the stem was a cached string but it wasn't). Avoid this by passing in the stem as an argument so that different values can be provided. Add tests to verify this. * src/commands.c (set_file_variables): Take second parameter stem to relieve the callers of set_file_variables() from setting/restoring file->stem. * src/commands.h (set_file_variables): Ditto. (execute_file_commands): Pass file->stem to set_file_variables(). * src/file.c (expand_deps): Pass d->stem to set_file_variables() and remove set and restore of file->stem. * src/implicit.c (pattern_search): Pass stem to set_file_variables() and remove set and restore of file->stem. * tests/scripts/features/se_explicit: Add new tests. * tests/scripts/features/se_implicit: Ditto. * tests/scripts/features/se_statpat: Ditto. * tests/scripts/variables/automatic: Ditto. --- src/commands.c | 17 ++++++++++------- src/commands.h | 2 +- src/file.c | 9 +-------- src/implicit.c | 12 ++---------- tests/scripts/features/se_explicit | 7 +++++++ tests/scripts/features/se_implicit | 7 +++++++ tests/scripts/features/se_statpat | 7 +++++++ tests/scripts/variables/automatic | 24 ++++++++++++++++++++++++ 8 files changed, 59 insertions(+), 26 deletions(-) diff --git a/src/commands.c b/src/commands.c index ee9c5367..644fe43b 100644 --- a/src/commands.c +++ b/src/commands.c @@ -58,10 +58,13 @@ dep_hash_cmp (const void *x, const void *y) return strcmp (dep_name (dx), dep_name (dy)); } -/* Set FILE's automatic variables up. */ +/* Set FILE's automatic variables up. + * Use STEM to set $*. + * If STEM is 0, then set FILE->STEM and $* to the target name with any + * suffix in the .SUFFIXES list stripped off. */ void -set_file_variables (struct file *file) +set_file_variables (struct file *file, const char *stem) { struct dep *d; const char *at, *percent, *star, *less; @@ -95,7 +98,7 @@ set_file_variables (struct file *file) } /* $* is the stem from an implicit or static pattern rule. */ - if (file->stem == 0) + if (stem == 0) { /* In Unix make, $* is set to the target name with any suffix in the .SUFFIXES list stripped off for @@ -121,14 +124,14 @@ set_file_variables (struct file *file) size_t slen = strlen (dep_name (d)); if (len > slen && strneq (dep_name (d), name + (len - slen), slen)) { - file->stem = strcache_add_len (name, len - slen); + file->stem = stem = strcache_add_len (name, len - slen); break; } } if (d == 0) - file->stem = ""; + file->stem = stem = ""; } - star = file->stem; + star = stem; /* $< is the first not order-only dependency. */ less = ""; @@ -464,7 +467,7 @@ execute_file_commands (struct file *file) initialize_file_variables (file, 0); - set_file_variables (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. */ diff --git a/src/commands.h b/src/commands.h index d9d8af65..48df68c7 100644 --- a/src/commands.h +++ b/src/commands.h @@ -42,4 +42,4 @@ void execute_file_commands (struct file *file); void print_commands (const struct commands *cmds); void delete_child_targets (struct child *child); void chop_commands (struct commands *cmds); -void set_file_variables (struct file *file); +void set_file_variables (struct file *file, const char *stem); diff --git a/src/file.c b/src/file.c index 5aba3b6d..6862397a 100644 --- a/src/file.c +++ b/src/file.c @@ -571,7 +571,6 @@ expand_deps (struct file *f) { struct dep *d; struct dep **dp; - const char *file_stem = f->stem; const char *fstem; int initialized = 0; @@ -646,19 +645,13 @@ expand_deps (struct file *f) initialized = 1; } - if (d->stem != 0) - f->stem = d->stem; - - set_file_variables (f); + set_file_variables (f, d->stem ? d->stem : f->stem); p = variable_expand_for_file (d->name, f); /* Free the un-expanded name. */ free ((char*)d->name); - if (d->stem != 0) - f->stem = file_stem; - /* Parse the prerequisites and enter them into the file database. */ new = split_prereqs (p); diff --git a/src/implicit.c b/src/implicit.c index b7f5d9ce..a6fd222c 100644 --- a/src/implicit.c +++ b/src/implicit.c @@ -521,10 +521,6 @@ pattern_search (struct file *file, int archive, if (rule->deps == 0) break; - /* Temporary assign STEM to file->stem (needed to set file - variables below). */ - file->stem = stem_str; - /* Mark this rule as in use so a recursive pattern_search won't try to use it. */ rule->in_use = 1; @@ -666,14 +662,14 @@ pattern_search (struct file *file, int archive, if (!file_vars_initialized) { initialize_file_variables (file, 0); - set_file_variables (file); + set_file_variables (file, stem_str); file_vars_initialized = 1; } /* Update the stem value in $* for this rule. */ else if (!file_variables_set) { define_variable_for_file ( - "*", 1, file->stem, o_automatic, 0, file); + "*", 1, stem_str, o_automatic, 0, file); file_variables_set = 1; } @@ -913,10 +909,6 @@ pattern_search (struct file *file, int archive, break; } - /* Reset the stem in FILE. */ - - file->stem = 0; - /* This rule is no longer 'in use' for recursive searches. */ rule->in_use = 0; diff --git a/tests/scripts/features/se_explicit b/tests/scripts/features/se_explicit index 450aaf7f..c0b1f29a 100644 --- a/tests/scripts/features/se_explicit +++ b/tests/scripts/features/se_explicit @@ -291,5 +291,12 @@ all: 2.x 1.x #MAKE#: Nothing to be done for 'all'.\n"); +# sv 62324. +# Integrity self check. +run_make_test(q! +.SECONDEXPANSION: +all: bye.x +bye.x: $$(firstword bye.1; +!, '', "#MAKEFILE#:4: *** unterminated call to function 'firstword': missing ')'. Stop.", 512); 1; diff --git a/tests/scripts/features/se_implicit b/tests/scripts/features/se_implicit index 712f3bfd..65b62501 100644 --- a/tests/scripts/features/se_implicit +++ b/tests/scripts/features/se_implicit @@ -434,6 +434,13 @@ all: 2.x "@=2.x,<=,^=,+=,|=,?=,*=2 #MAKE#: Nothing to be done for 'all'.\n"); +# sv 62324. +# Integrity self check. +run_make_test(q! +.SECONDEXPANSION: +all: bye.x +%.x: $$(firstword %.1; +!, '', "#MAKE#: *** unterminated call to function 'firstword': missing ')'. Stop.", 512); # This tells the test driver that the perl test script executed properly. 1; diff --git a/tests/scripts/features/se_statpat b/tests/scripts/features/se_statpat index b1e59e1f..17f9c348 100644 --- a/tests/scripts/features/se_statpat +++ b/tests/scripts/features/se_statpat @@ -102,6 +102,13 @@ oo$ba.1 oo$ba '); +# sv 62324. +# Integrity self check. +run_make_test(q! +.SECONDEXPANSION: +all: bye.x +bye.x: %.x: $$(firstword %.1; +!, '', "#MAKEFILE#:4: *** unterminated call to function 'firstword': missing ')'. Stop.", 512); # This tells the test driver that the perl test script executed properly. 1; diff --git a/tests/scripts/variables/automatic b/tests/scripts/variables/automatic index efd039c8..771bbda8 100644 --- a/tests/scripts/variables/automatic +++ b/tests/scripts/variables/automatic @@ -91,6 +91,30 @@ mbr.src: ; @:', '', 'mbr'); +# Same as above with second expansion. +# +run_make_test(' +.SECONDEXPANSION: +.SUFFIXES: .b .src + +p:=mbr.src +mbr.b: $$p + @echo $* + +mbr.src: ; @:', + '', + 'mbr'); + +# Test that $* is set to empty string for unknown suffixes. + +run_make_test(' +mbr.b: mbr.src + @echo star=$* + +mbr.src: ; @:', + '', + "star=\n"); + # TEST #3 -- test for Savannah bug #8154 # Make sure that nonexistent prerequisites are listed in $?, since they are # considered reasons for the target to be rebuilt.