bugmake - Bugs: bug #63016, Recursive variable references...

 
 

bug #63016: Recursive variable references itself (eventually) when exporting to $(shell)

Submitter:  Martin Dorey <mdorey>
Submitted:  Sun 04 Sep 2022 11:28:29 PM UTC
   
 
Severity:  3 - Normal Item Group:  Bug
Status:  Fixed Privacy:  Public
Assigned to:  psmith Open/Closed:  Closed
Component Version:  4.4 Operating System:  POSIX-Based
Fixed Release:  4.4 Triage Status:  Small Effort
* Mandatory Fields

Add a New Comment Rich Markup
   

Sun 13 Oct 2024 01:39:21 AM UTC, comment #2: 

I may come to regret posting this, as the current git code is working well for me here.  I did ask for this change.  That's why I feel duty-bound to report that it bit me today.

martind@stormy:~/tmp/D160959$ cat Makefile
generate = $(or $(1),$(call generate,flag))
FLAGS += $(or $(call generate),$(error why did my recursion produce the empty string?))
UNRELATED := $(shell :)
all:;
martind@stormy:~/tmp/D160959$ make
make: 'all' is up to date.
martind@stormy:~/tmp/D160959$

All good... until one particular call causes FLAGS to be inherited from the environment:

martind@stormy:~/tmp/D160959$ FLAGS= make --debug=v
GNU Make 4.4.90
Built for x86_64-pc-linux-gnu
Copyright (C) 1988-2024 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Reading makefiles...
Reading makefile 'Makefile'...
Makefile:1: not recursively expanding generate to export to shell function
Makefile:3: *** why did my recursion produce the empty string?.  Stop.
martind@stormy:~/tmp/D160959$

I could work around it, no problem, but it took me hours to understand what was going on... and I'm one of the two principals here.

The shell invocation that bit me today was the same "2004" (actually 2005) one that bit me in my original post here, but it could easily have been something more compelling.

I can try to explain my use case for the recursion if that would help, but I lean towards thinking there's nothing to be done here.  There is already a diagnostic, as we see demonstrated here, though I only thought of asking for it when I found my way back to this bug report.

Martin Dorey <mdorey>
Thu 08 Sep 2022 07:09:44 AM UTC, comment #1: 

I pushed a fix.  I think it might be possible to do better: one can imagine a situation where you wouldn't treat the expansion of a variable to put into the environment as a recursive expansion of a variable used in a recipe.  But to do that would require more work and I couldn't convince myself (or find an example makefile) where it would make a real difference.

In this change, variables that are recursively expanded into the environment of a shell function will be set as the empty string rather than throwing an error.

Paul D. Smith <psmith>
Group administrator
Sun 04 Sep 2022 11:28:29 PM UTC, original submission:  

With this Makefile:


martind@sirius:~/tmp/make-2022-09-04$ cat Makefile
A = $(shell :)
export B = $(A)
default:; $(A)
martind@sirius:~/tmp/make-2022-09-04$


... which bears a certain resemblance to the "export HI" example added to make.texi in:

https://git.savannah.gnu.org/cgit/make.git/commit/?id=98da874c43035a490cdca81331724f233a3d0c9a

... under:

[SV 10593] Export variables to $(shell ...) commands

And yesterday's code from git:


martind@sirius:~/tmp/make-2022-09-04$ make --version
GNU Make 4.3.90
Built for x86_64-pc-linux-gnu
Copyright (C) 1988-2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
martind@sirius:~/tmp/make-2022-09-04$


I get the following error:


martind@sirius:~/tmp/make-2022-09-04$ make
Makefile:1: *** Recursive variable 'A' references itself (eventually).  Stop.
martind@sirius:~/tmp/make-2022-09-04$


... where make-4.3 just says:


martind@platinum:~/tmp/make-2022-09-04$ /usr/bin/make
make: 'default' is up to date.
martind@platinum:~/tmp/make-2022-09-04$


The fatal error happens from the following stack:


(gdb) bt
#0  recursively_expand_for_file (v=v@entry=0x5616496613c0, file=file@entry=0x0) at src/expand.c:124
#1  0x0000561647495aa2 in reference_variable (length=1, name=0x561649665cd2 "A)", o=0x561649665c00 "")
    at src/expand.c:177
#2  variable_expand_string (line=<optimized out>, line@entry=0x0, string=<optimized out>,
    length=length@entry=18446744073709551615) at src/expand.c:383
#3  0x0000561647495d90 in variable_expand (line=<optimized out>) at src/expand.c:420
#4  variable_expand_for_file (file=0x0, line=<optimized out>) at src/expand.c:467
#5  allocated_variable_expand_for_file (file=0x0, line=<optimized out>) at src/expand.c:569
#6  recursively_expand_for_file (v=v@entry=0x561649661440, file=file@entry=0x0) at src/expand.c:143
#7  0x00005616474b13b1 in target_environment (file=file@entry=0x0, recursive=recursive@entry=0) at src/variable.c:1114
#8  0x000056164749b7d2 in func_shell_base (o=0x5616496656e0 "", argv=<optimized out>, trim_newlines=1)
    at src/function.c:1879
#9  0x000056164749bcff in handle_function (op=op@entry=0x7ffe9895f2b8, stringp=stringp@entry=0x7ffe9895f2b0)
    at src/function.c:2669
#10 0x00005616474954d7 in variable_expand_string (line=<optimized out>, line@entry=0x0, string=<optimized out>,
    length=length@entry=18446744073709551615) at src/expand.c:261
#11 0x0000561647495d90 in variable_expand (line=<optimized out>) at src/expand.c:420
#12 variable_expand_for_file (file=0x0, line=<optimized out>) at src/expand.c:467
#13 allocated_variable_expand_for_file (file=0x0, line=<optimized out>) at src/expand.c:569
#14 recursively_expand_for_file (v=v@entry=0x5616496613c0, file=file@entry=0x0) at src/expand.c:143
#15 0x0000561647495aa2 in reference_variable (length=1, name=0x5616496656c3 "A)",
    o=0x5616496655f1 "\333\353%\267\177") at src/expand.c:177
#16 variable_expand_string (line=<optimized out>, line@entry=0x0, string=<optimized out>,
    length=length@entry=18446744073709551615) at src/expand.c:383
#17 0x0000561647495edb in variable_expand (line=<optimized out>) at src/expand.c:420
#18 variable_expand_for_file (line=<optimized out>, file=<optimized out>) at src/expand.c:478
#19 0x0000561647495f44 in allocated_variable_expand_for_file (line=<optimized out>, file=file@entry=0x5616496614a0)
    at src/expand.c:569
#20 0x00005616474a1f11 in new_job (file=0x5616496614a0) at src/job.c:1825
#21 0x00005616474adc40 in remake_file (file=0x5616496614a0) at src/remake.c:1258
#22 update_file_1 (depth=<optimized out>, file=<optimized out>) at src/remake.c:856
#23 update_file (file=file@entry=0x5616496614a0, depth=<optimized out>) at src/remake.c:343
#24 0x00005616474adeb8 in update_goal_chain (goaldeps=<optimized out>) at src/remake.c:160
#25 0x0000561647491283 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at src/main.c:2860
(gdb)


Comparison with similar lines in the surrounding source suggests there's a missing newline on the end of the warning that I think I was hoped to get in this situation:


+              DB (DB_VERBOSE, (_("%s:%lu: Skipping export of %s to shell function due to recursive expansion"),
+                               v->fileinfo.filenm, v->fileinfo.lineno, v->name));


I didn't bother generating a minimal example for it but I've seen the message run into the next piece of debugging output.

I'm not sure I'm really suggesting this hack:


martind@sirius:~/download/make$ git diff | cat
diff --git a/src/expand.c b/src/expand.c
index 6bc2ac59..ee0a45f5 100644
--- a/src/expand.c
+++ b/src/expand.c
@@ -19,6 +19,7 @@ this program.  If not, see <http://www.gnu.org/licenses/>.  */
 #include <assert.h>

 #include "filedef.h"
+#include "debug.h"
 #include "job.h"
 #include "commands.h"
 #include "variable.h"
@@ -120,10 +121,18 @@ recursively_expand_for_file (struct variable *v, struct file *file)
   if (v->expanding)
     {
       if (!v->exp_count)
-        /* Expanding V causes infinite recursion.  Lose.  */
-        OS (fatal, *expanding_var,
-            _("Recursive variable '%s' references itself (eventually)"),
-            v->name);
+        {
+          if (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));
+              return strdup("skipped export to shell function due to recursive expansion");
+            }
+          /* Expanding V causes infinite recursion.  Lose.  */
+          OS (fatal, *expanding_var,
+              _("Recursive variable '%s' references itself (eventually)"),
+              v->name);
+        }
       --v->exp_count;
     }

diff --git a/src/variable.c b/src/variable.c
index 30f4424a..26aed5db 100644
--- a/src/variable.c
+++ b/src/variable.c
@@ -19,7 +19,6 @@ this program.  If not, see <http://www.gnu.org/licenses/>.  */
 #include <assert.h>

 #include "filedef.h"
-#include "debug.h"
 #include "dep.h"
 #include "job.h"
 #include "commands.h"
@@ -1102,15 +1101,6 @@ target_environment (struct file *file, int recursive)
            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"),
-                                 v->fileinfo.filenm, v->fileinfo.lineno, v->name));
-                continue;
-              }
-
             value = cp = recursively_expand_for_file (v, file);
           }

martind@sirius:~/download/make$


... but it does fix this test case and doesn't regress make check, which was already failing one test in functions/wildcard for me.

The $(shell) invocation which is provoking this for real was committed in 2004 as an alleged work around for:

[SV 443] $(wildcard) function uses directory cache: gives incorrect results sometimes

I wish I could find the full story of that bug, because what little Savannah has makes it look like it came from the same part of the same source base as I'm now contemplating returning to $(wildcard) to avoid this, on the prematurely pessimistic assumption that Paul won't want to fix this.  If anyone has the original correspondence, I'd be grateful.  It's not in https://lists.gnu.org/archive/html/bug-make/2002-05/index.html.

Martin Dorey <mdorey>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by psmith (Posted a comment)
  • -email is unavailable- added by mdorey (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-09-08 psmith StatusNone Fixed
        Assigned toNone psmith
        Open/ClosedOpen Closed
        Fixed ReleaseNone 4.4
        Triage StatusNone Small Effort

    Back to the top

    Powered by Savane 3.13-da57.
    Corresponding source code