[SV 63016] Don't fail exporting to $(shell ...)

The fix for SV 10593 caused recursive expansion errors when exporting
a variable that contains a $(shell ...) invocation.  If we see this
type of recursion, ignore it and expand to the empty string rather
than failing.

* src/variable.h (env_recursion): New global variable.
* src/variable.c (target_environment): If creating the environment
for a $(shell ...) function increment env_recursion.  Remove the
check for expansion in a shell function context.
* src/expand.c (recursively_expand_for_file): Check for recursive
expansion in a $(shell ...) environment context and if present,
show the verbose message and return the empty string.
* tests/scripts/functions/shell: Add a test for this situation.

diff --git a/src/expand.c b/src/expand.c
index 6bc2ac59a2108023dbb5069e1ee92ec3e7e43cbc..2acb32c5445d9eb9f0a478aa83c3e89a1b8c2e54 100644 (file)
--- a/src/expand.c
+++ b/src/expand.c
@@ -18,9 +18,10 @@ this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include <assert.h>
 
+#include "commands.h"
+#include "debug.h"
 #include "filedef.h"
 #include "job.h"
-#include "commands.h"
 #include "variable.h"
 #include "rule.h"
 
@@ -101,6 +102,16 @@ recursively_expand_for_file (struct variable *v, struct file *file)
   struct variable_set_list *save = 0;
   int set_reading = 0;
 
+  /* If we're expanding to put into the environment of a shell function then
+     ignore any recursion issues.  */
+  if (v->expanding && env_recursion)
+    {
+      DB (DB_VERBOSE,
+          (_("%s:%lu: not recursively expanding %s to export to shell function\n"),
+           v->fileinfo.filenm, v->fileinfo.lineno, v->name));
+      return xstrdup ("");
+    }
+
   /* Don't install a new location if this location is empty.
      This can happen for command-line variables, builtin variables, etc.  */
   saved_varp = expanding_var;

diff --git a/src/variable.c b/src/variable.c
index 6c3d21250ba9fbfd99e96d00b609d806d730c6ce..b5c43c35c707ea5ec1580145432662b9dfccee8a 100644 (file)
--- a/src/variable.c
+++ b/src/variable.c
@@ -31,12 +31,15 @@ this program.  If not, see <http://www.gnu.org/licenses/>.  */
 #endif
 #include "hash.h"
 
+/* Incremented every time we enter target_environment().  */
+unsigned long long env_recursion = 0;
+
 /* Incremented every time we add or remove a global variable.  */
-static unsigned long variable_changenum;
+static unsigned long variable_changenum = 0;
 
 /* Chain of all pattern-specific variables.  */
 
-static struct pattern_var *pattern_vars;
+static struct pattern_var *pattern_vars = NULL;
 
 /* Pointer to the last struct in the pack of a specific size, from 1 to 255.*/
 
@@ -1038,15 +1041,20 @@ target_environment (struct file *file, int recursive)
   int found_mflags = 0;
   int found_makeflags = 0;
 
+  /* If file is NULL we're creating the target environment for $(shell ...)
+     Remember this so we can just ignore recursion.  */
+  if (!file)
+    ++env_recursion;
+
   /* We need to update makeflags if (a) we're not recurive, (b) jobserver_auth
      is enabled, and (c) we need to add invalidation.  */
   if (!recursive && jobserver_auth)
     invalid = jobserver_get_invalid_auth ();
 
-  if (file == 0)
-    set_list = current_variable_set_list;
-  else
+  if (file)
     set_list = file->variables;
+  else
+    set_list = current_variable_set_list;
 
   hash_init (&table, VARIABLE_BUCKETS,
              variable_hash_1, variable_hash_2, variable_hash_cmp);
@@ -1101,19 +1109,7 @@ target_environment (struct file *file, int recursive)
            expand its value.  If it came from the environment, it should
            go back into the environment unchanged.  */
         if (v->recursive && v->origin != o_env && v->origin != o_env_override)
-          {
-            /* If V is being recursively expanded and this is for a shell
-               function, just skip it.  */
-            if (v->expanding && file == NULL)
-              {
-                DB (DB_VERBOSE,
-                    (_("%s:%lu: Skipping export of %s to shell function due to recursive expansion\n"),
-                     v->fileinfo.filenm, v->fileinfo.lineno, v->name));
-                continue;
-              }
-
-            value = cp = recursively_expand_for_file (v, file);
-          }
+          value = cp = recursively_expand_for_file (v, file);
 
         /* If this is the SHELL variable remember we already added it.  */
         if (!added_SHELL && streq (v->name, "SHELL"))
@@ -1212,6 +1208,9 @@ target_environment (struct file *file, int recursive)
 
   hash_free (&table, 0);
 
+  if (!file)
+    --env_recursion;
+
   return result_0;
 }
 \f

diff --git a/src/variable.h b/src/variable.h
index a28749c60747d76ac2fc4ebcb1320abe6de0ba43..155d9d63721a53ebd727a84d2e8d2dba7d430361 100644 (file)
--- a/src/variable.h
+++ b/src/variable.h
@@ -114,6 +114,7 @@ struct pattern_var
     struct variable variable;
   };
 
+extern unsigned long long env_recursion;
 extern char *variable_buffer;
 extern struct variable_set_list *current_variable_set_list;
 extern struct variable *default_goal_var;

diff --git a/tests/scripts/functions/shell b/tests/scripts/functions/shell
index 1ca1e9abeb7bc6712e90fa7e30eacc731e913e69..a5bd5ddcffb866999bb44d2a1626338196c8db4c 100644 (file)
--- a/tests/scripts/functions/shell
+++ b/tests/scripts/functions/shell
@@ -88,6 +88,16 @@ all: ; @echo $$HI $(bad)
 ',
               '',"hi hi");
 
+# SV 63016: Exported variable that contains a variable containing $(shell...)
+
+run_make_test('
+HI = $(shell echo hi)
+export bad = $(HI)
+.PHONY: all
+all:; : $(HI)
+',
+              '',": hi");
+
 if ($port_type ne 'W32') {
     # Test shell errors in recipes including offset
     # This needs to be ported to Windows, or else Windows error messages