[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.

diff --git a/src/commands.c b/src/commands.c
index ee9c53679db4ea38702d021a3f535a0849802463..644fe43be6b73d4baaf356190b4c08fac7c1922e 100644 (file)
--- 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 d9d8af658a9f7350b3e1258cc1a6dbff381cb997..48df68c769f228e6da289814014a4f300368a52c 100644 (file)
--- 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 5aba3b6dc7de8742c7663aae8ba100f9363b8d2e..6862397ac4c8867a96ea72a811bf14c6e9c9918c 100644 (file)
--- 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 b7f5d9ce440bc02f6839d03b6110a0aefe449f75..a6fd222cb15a3450336f62741184c68aa6393bcb 100644 (file)
--- 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 450aaf7f7c464d31083fc61265aedc476872d424..c0b1f29aabdc74df93cae46e061f65c919f10832 100644 (file)
--- 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 712f3bfd77fd5f64cc3a7d00b0c05deeec379bc7..65b62501b6d2bef763265640a3eb21158a628a00 100644 (file)
--- 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 b1e59e1ffc4b006d8bcd993b1c2cbf7d4dc4fae9..17f9c3484e8f3d74231643ccb3ed7a505d57f24c 100644 (file)
--- 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 efd039c81ca78922be1422e61b107876e84bb86b..771bbda8f7beebdffebe82c13e7f3ef4a25f1a97 100644 (file)
--- 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.