commit 7893737232abef49ccf10192063da88772257b52
Author: Dmitry Goncharov <dgoncharov@users.sf.net>
Date:   Sat Nov 12 21:18:04 2022 -0500

    [SV 63347] Command line variable definitions in MAKEFLAGS at parse time.
    
    This commit introduces two visible changes.
    1. Have command line variable definitions in MAKEFLAGS at parse time.
    2. Define makeflags at parse time, each time a makefile modifies MAKEFLAGS.
    
    The new MAKEFLAGS and MAKEOVERRIDES initialization procedure.
    
    1. decode_switches (argc, argv, o_command) is called.
    This first call adds intact command line variable definitions to the make variable
    database.
    2. All command line variable definitions go through quote_for_env.
    Internal variable -*-command-variables-*- is initialized to contain quoted
    definitions.
    3. MAKEOVERRIDES is initialized to refer to -*-command-variables-*- with origin
    equals o_env. Origin has to be o_env to keep the definitions in the database
    intact.
    4. define_makeflags is called. This adds MAKEOVERRIDES to MAKEFLAGS.
    5. makefiles are parsed. If a makefile modifies MAKEFLAGS, the new value of
    MAKEFLAGS is defined right away.
    6. env switches are decoded again as o_env. The original definitions set in the
    database by decode_switches at step 1 stay intact, because o_command beats
    o_env.
    
    The database has to contain the original intact definitions in order to detect
    incorrect input in cases like
    $ cat makefile
    all:; $(hello)
    $ make hello='$(world'
    makefile:1: *** unterminated variable reference.  Stop.
    
    * makeint.h: Declare enum variable_origin.
    Declare struct variable.
    Declare define_makeflags.
    Add parameter origin to decode_env_switches.
    * main.c (define_makeflags): Remove parameter all. If a variable is defined on the
    command line, then append MAKEOVERRIDES to MAKEFLAGS.
    (decode_env_switches): Replace parameter env with origin.
    (decode_switches): Replace parameter env with origin.
    Treat origin == o_command as env == 0.
    Pass origin to handle_non_switch_argument
    (handle_non_switch_argument): Replace parameter env with origin.
    Treat origin == o_command as env == 0.
    Take origin from decode_switches and pass it to try_variable_definition.
    (main): Pass o_command as origin to decode_switches before parsing makefiles.
    Pass o_env as origin to decode_switches (through decode_env_switches) after
    parsing makefiles.
    * variable.c (set_special_var): Take parameter origin and pass it to
    decode_env_switches.
    (set_special_var): Define makeflags at parse time, each time a makefile
    modifies MAKEFLAGS.
    (do_variable_definition): If appending to MAKEFLAGS, ensure command line
    variable definitions are listed after all switches.
    This is needed, because with the change in main.c MAKEFLAGS can carry command
    line variable definitions at parse time. When the makefile contains code that
    appends to MAKEFLAGS, the newly appended value needs to be inserted before the
    command line variable definitions.
    * ../tests/scripts/variables/MAKEFLAGS: Add tests.

diff --git a/src/main.c b/src/main.c
index eec93656..37f506d7 100644
--- a/src/main.c
+++ b/src/main.c
@@ -105,8 +105,8 @@ double atof ();
 static void clean_jobserver (int status);
 static void print_data_base (void);
 static void print_version (void);
-static void decode_switches (int argc, const char **argv, int env);
-static struct variable *define_makeflags (int all, int makefile);
+static void decode_switches (int argc, const char **argv,
+                             enum variable_origin origin);
 static char *quote_for_env (char *out, const char *in);
 static void initialize_global_hash_tables (void);
 
@@ -1574,13 +1574,13 @@ main (int argc, char **argv, char **envp)
   /* Decode the switches.  */
   if (lookup_variable (STRING_SIZE_TUPLE (GNUMAKEFLAGS_NAME)))
     {
-      decode_env_switches (STRING_SIZE_TUPLE (GNUMAKEFLAGS_NAME));
+      decode_env_switches (STRING_SIZE_TUPLE (GNUMAKEFLAGS_NAME), o_command);
 
       /* Clear GNUMAKEFLAGS to avoid duplication.  */
       define_variable_cname (GNUMAKEFLAGS_NAME, "", o_env, 0);
     }
 
-  decode_env_switches (STRING_SIZE_TUPLE (MAKEFLAGS_NAME));
+  decode_env_switches (STRING_SIZE_TUPLE (MAKEFLAGS_NAME), o_command);
 
 #if 0
   /* People write things like:
@@ -1601,7 +1601,7 @@ main (int argc, char **argv, char **envp)
     int env_slots = arg_job_slots;
     arg_job_slots = INVALID_JOB_SLOTS;
 
-    decode_switches (argc, (const char **)argv, 0);
+    decode_switches (argc, (const char **)argv, o_command);
     argv_slots = arg_job_slots;
 
     if (arg_job_slots == INVALID_JOB_SLOTS)
@@ -2021,7 +2021,7 @@ main (int argc, char **argv, char **envp)
 
   /* Set up the MAKEFLAGS and MFLAGS variables for makefiles to see.
      Initialize it to be exported but allow the makefile to reset it.  */
-  define_makeflags (0, 0)->export = v_export;
+  define_makeflags (0)->export = v_export;
 
   /* Define the default variables.  */
   define_default_variables ();
@@ -2071,12 +2071,12 @@ main (int argc, char **argv, char **envp)
     arg_job_slots = INVALID_JOB_SLOTS;
 
     /* Decode switches again, for variables set by the makefile.  */
-    decode_env_switches (STRING_SIZE_TUPLE (GNUMAKEFLAGS_NAME));
+    decode_env_switches (STRING_SIZE_TUPLE (GNUMAKEFLAGS_NAME), o_env);
 
     /* Clear GNUMAKEFLAGS to avoid duplication.  */
     define_variable_cname (GNUMAKEFLAGS_NAME, "", o_override, 0);
 
-    decode_env_switches (STRING_SIZE_TUPLE (MAKEFLAGS_NAME));
+    decode_env_switches (STRING_SIZE_TUPLE (MAKEFLAGS_NAME), o_env);
 #if 0
     decode_env_switches (STRING_SIZE_TUPLE ("MFLAGS"));
 #endif
@@ -2259,7 +2259,7 @@ main (int argc, char **argv, char **envp)
 
   /* Set up MAKEFLAGS and MFLAGS again, so they will be right.  */
 
-  define_makeflags (1, 0);
+  define_makeflags (0);
 
   /* Make each 'struct goaldep' point at the 'struct file' for the file
      depended on.  Also do magic for special targets.  */
@@ -2419,7 +2419,7 @@ main (int argc, char **argv, char **envp)
       }
 
       /* Set up 'MAKEFLAGS' specially while remaking makefiles.  */
-      define_makeflags (1, 1);
+      define_makeflags (1);
 
       {
         int orig_db_level = db_level;
@@ -2811,7 +2811,7 @@ main (int argc, char **argv, char **envp)
     }
 
   /* Set up 'MAKEFLAGS' again for the normal targets.  */
-  define_makeflags (1, 0);
+  define_makeflags (0);
 
   /* Set always_make_flag if -B was given.  */
   always_make_flag = always_make_set;
@@ -2999,7 +2999,7 @@ init_switches (void)
 
 /* Non-option argument.  It might be a variable definition.  */
 static void
-handle_non_switch_argument (const char *arg, int env)
+handle_non_switch_argument (const char *arg, enum variable_origin origin)
 {
   struct variable *v;
 
@@ -3032,7 +3032,7 @@ handle_non_switch_argument (const char *arg, int env)
       }
   }
 #endif
-  v = try_variable_definition (0, arg, o_command, 0);
+  v = try_variable_definition (0, arg, origin, 0);
   if (v != 0)
     {
       /* It is indeed a variable definition.  If we don't already have this
@@ -3052,11 +3052,13 @@ handle_non_switch_argument (const char *arg, int env)
           command_variables = cv;
         }
     }
-  else if (! env)
+  else if (*arg && origin == o_command)
     {
       /* Not an option or variable definition; it must be a goal
          target!  Enter it as a file and add it to the dep chain of
-         goals.  */
+         goals.
+         Check *arg, because if the top makefile resets MAKEOVERRIDES, then ARG
+         points to an empty string in the submake.  */
       struct file *f = enter_file (strcache_add (expand_command_line_file (arg)));
       f->cmd_target = 1;
 
@@ -3104,7 +3106,7 @@ handle_non_switch_argument (const char *arg, int env)
    They came from the environment if ENV is nonzero.  */
 
 static void
-decode_switches (int argc, const char **argv, int env)
+decode_switches (int argc, const char **argv, enum variable_origin origin)
 {
   int bad = 0;
   const struct command_switch *cs;
@@ -3118,7 +3120,7 @@ decode_switches (int argc, const char **argv, int env)
 
   /* Let getopt produce error messages for the command line,
      but not for options from the environment.  */
-  opterr = !env;
+  opterr = origin == o_command;
   /* Reset getopt's state.  */
   optind = 0;
 
@@ -3134,7 +3136,7 @@ decode_switches (int argc, const char **argv, int env)
         break;
       else if (c == 1)
         /* An argument not starting with a dash.  */
-        handle_non_switch_argument (coptarg, env);
+        handle_non_switch_argument (coptarg, origin);
       else if (c == '?')
         /* Bad option.  We will print a usage message and die later.
            But continue to parse the other options so the user can
@@ -3148,7 +3150,7 @@ decode_switches (int argc, const char **argv, int env)
                  this switch.  We test this individually inside the
                  switch below rather than just once outside it, so that
                  options which are to be ignored still consume args.  */
-              int doit = !env || cs->env;
+              int doit = origin == o_command || cs->env;
 
               switch (cs->type)
                 {
@@ -3298,9 +3300,9 @@ decode_switches (int argc, const char **argv, int env)
      to be returned in order, this only happens when there is a "--"
      argument to prevent later arguments from being options.  */
   while (optind < argc)
-    handle_non_switch_argument (argv[optind++], env);
+    handle_non_switch_argument (argv[optind++], origin);
 
-  if (bad && !env)
+  if (bad && origin == o_command)
     print_usage (bad);
 
   /* If there are any options that need to be decoded do it now.  */
@@ -3320,7 +3322,7 @@ decode_switches (int argc, const char **argv, int env)
    decode_switches.  */
 
 void
-decode_env_switches (const char *envar, size_t len)
+decode_env_switches (const char *envar, size_t len, enum variable_origin origin)
 {
   char *varref = alloca (2 + len + 2);
   char *value, *p, *buf;
@@ -3382,7 +3384,7 @@ decode_env_switches (const char *envar, size_t len)
     argv[1] = buf;
 
   /* Parse those words.  */
-  decode_switches (argc, argv, 1);
+  decode_switches (argc, argv, origin);
 }
 
 /* Quote the string IN so that it will be interpreted as a single word with
@@ -3407,11 +3409,11 @@ quote_for_env (char *out, const char *in)
 }
 
 /* Define the MAKEFLAGS and MFLAGS variables to reflect the settings of the
-   command switches.  Include options with args if ALL is nonzero.
+   command switches. Always include options with args.
    Don't include options with the 'no_makefile' flag set if MAKEFILE.  */
 
-static struct variable *
-define_makeflags (int all, int makefile)
+struct variable *
+define_makeflags (int makefile)
 {
   const char ref[] = "MAKEOVERRIDES";
   const char posixref[] = "-*-command-variables-*-";
@@ -3596,25 +3598,24 @@ define_makeflags (int all, int makefile)
       p = mempcpy (p, evalref, CSTRLEN (evalref));
     }
 
-  if (all)
-    {
-      /* If there are any overrides to add, write a reference to
-         $(MAKEOVERRIDES), which contains command-line variable definitions.
-         Separate the variables from the switches with a "--" arg.  */
+  {
+    /* If there are any overrides to add, write a reference to
+       $(MAKEOVERRIDES), which contains command-line variable definitions.
+       Separate the variables from the switches with a "--" arg.  */
 
-      const char *r = posix_pedantic ? posixref : ref;
-      size_t l = strlen (r);
-      v = lookup_variable (r, l);
+    const char *r = posix_pedantic ? posixref : ref;
+    size_t l = strlen (r);
+    v = lookup_variable (r, l);
 
-      if (v && v->value && v->value[0] != '\0')
-        {
-          p = stpcpy (p, " -- ");
-          *(p++) = '$';
-          *(p++) = '(';
-          p = mempcpy (p, r, l);
-          *(p++) = ')';
-        }
-    }
+    if (v && v->value && v->value[0] != '\0')
+      {
+        p = stpcpy (p, " -- ");
+        *(p++) = '$';
+        *(p++) = '(';
+        p = mempcpy (p, r, l);
+        *(p++) = ')';
+      }
+  }
 
   /* If there is a leading dash, omit it.  */
   if (flagstring[0] == '-')
diff --git a/src/makeint.h b/src/makeint.h
index c726abe0..d6ac21b8 100644
--- a/src/makeint.h
+++ b/src/makeint.h
@@ -558,7 +558,11 @@ void out_of_memory (void) NORETURN;
 #define ONS(_t,_a,_f,_n,_s)   _t((_a), INTSTR_LENGTH + strlen (_s), \
                                  (_f), (_n), (_s))
 
-void decode_env_switches (const char*, size_t line);
+enum variable_origin;
+void decode_env_switches (const char*, size_t line,
+                          enum variable_origin origin);
+struct variable;
+struct variable *define_makeflags (int makefile);
 void temp_stdin_unlink (void);
 void die (int) NORETURN;
 void pfatal_with_name (const char *) NORETURN;
diff --git a/src/variable.c b/src/variable.c
index 0bd9963b..007ca3f4 100644
--- a/src/variable.c
+++ b/src/variable.c
@@ -1215,7 +1215,7 @@ target_environment (struct file *file, int recursive)
 }
 
 static struct variable *
-set_special_var (struct variable *var)
+set_special_var (struct variable *var, enum variable_origin origin)
 {
   if (streq (var->name, RECIPEPREFIX_NAME))
     {
@@ -1225,7 +1225,10 @@ set_special_var (struct variable *var)
       cmd_prefix = var->value[0]=='\0' ? RECIPEPREFIX_DEFAULT : var->value[0];
     }
   else if (streq (var->name, MAKEFLAGS_NAME))
-    decode_env_switches (STRING_SIZE_TUPLE(MAKEFLAGS_NAME));
+    {
+      decode_env_switches (STRING_SIZE_TUPLE(MAKEFLAGS_NAME), origin);
+      define_makeflags (rebuilding_makefiles);
+    }
 
   return var;
 }
@@ -1352,7 +1355,7 @@ do_variable_definition (const floc *flocp, const char *varname,
           {
             /* Paste the old and new values together in VALUE.  */
 
-            size_t oldlen, vallen;
+            size_t oldlen, vallen, alloclen;
             const char *val;
             char *tp = NULL;
 
@@ -1378,13 +1381,34 @@ do_variable_definition (const floc *flocp, const char *varname,
               }
 
             oldlen = strlen (v->value);
-            p = alloc_value = xmalloc (oldlen + 1 + vallen + 1);
+            alloclen = oldlen + 1 + vallen + 1;
+            p = alloc_value = xmalloc (alloclen);
 
             if (oldlen)
               {
                 memcpy (alloc_value, v->value, oldlen);
                 alloc_value[oldlen] = ' ';
                 ++oldlen;
+                if (streq (varname, MAKEFLAGS_NAME))
+                  {
+                    /* If appending to MAKEFLAGS, ensure command line variable
+                     * definitions are listed after all switches.  */
+                    char *a;
+                    alloc_value[oldlen] = '\0';
+                    a = strstr (alloc_value, " -- ");
+                    if (a)
+                      {
+                        /* Found command line variable definitions.
+                         * Copy the new value before -- and copy cli
+                         * definitions after the new value.  */
+                        const char *b = strdupa (a);
+                        const size_t blen = oldlen - (a - alloc_value);
+                        memcpy (mempcpy (a + 1, val, vallen), b, blen);
+                        alloc_value[alloclen - 1] = '\0';
+                        free (tp);
+                        break;
+                      }
+                  }
               }
 
             memcpy (&alloc_value[oldlen], val, vallen + 1);
@@ -1546,7 +1570,7 @@ do_variable_definition (const floc *flocp, const char *varname,
 
  done:
   free (alloc_value);
-  return v->special ? set_special_var (v) : v;
+  return v->special ? set_special_var (v, origin) : v;
 }
 
 /* Parse P (a null-terminated string) as a variable definition.
