From d86448fe5f7b8195790de1bc34e7fa07ab974d4a Mon Sep 17 00:00:00 2001
From: Paul Smith <psmith@gnu.org>
Date: Sat, 6 Jan 2024 17:42:40 -0500
Subject: [PATCH] [SV 64402] Correct locating "," in ifeq/ifneq conditionals

Ensure that we correctly skip the entirety of a macro or function
reference when searching for the "," separator in an ifeq/ifneq
conditional, including using "$," and also "${foo,bar}".  Note that
this change means that parenthesis OTHER than those used for variable
expansion are not considered special, any longer.

* NEWS: Announce the change.
* src/read.c (conditional_line): Skip variable references when looking
  for "," and ensure that we match closing parens/braces properly.
* tests/scripts/features/conditionals: Add tests for this behavior.
---
 NEWS                                | 18 ++++++++++----
 src/read.c                          | 28 +++++++++++++++------
 tests/scripts/features/conditionals | 38 +++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+), 12 deletions(-)

diff --git a/NEWS b/NEWS
index ea81d397..4844cf63 100644
--- a/NEWS
+++ b/NEWS
@@ -9,11 +9,6 @@ which is contained in this distribution as the file doc/make.texi.
 See the README file and the GNU Make manual for instructions for
 reporting bugs.
 
-* 'make --print-data-base' (or 'make -p') now outputs time of day
-  using the same form as for file timestamps, e.g., "2023-05-10
-  10:43:57.570558743".  Previously it used the form "Wed May 10
-  10:43:57 2023", which has less detail and is harder to compare.
-
 
 Version 4.4.90 (26 Feb 2023)
 
@@ -34,6 +29,14 @@ https://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=111&se
   your initialization function can check the provided ABI version to verify
   it's being loaded correctly.
 
+* WARNING: Backward-incompatibility!
+  Parsing of the first argument in ifeq/ifneq with () has been cleaned up.
+  When locating the separating "," any variable reference (single char as well
+  as using $() or ${}) is skipped.  However parentheses that are not part of
+  or contained in variable references will not be counted.  This means that
+  things like "ifeq ((foo,bar),)" are now syntax errors.  Use a variable to
+  hide the comma if needed: "COMMA = ," / "ifeq ((foo$(COMMA)bar),)".
+
 * New feature: Unload function for loaded objects
   When a loaded object needs to be unloaded by GNU Make, it will invoke an
   unload function (if one is defined) beforehand that allows the object to
@@ -53,6 +56,11 @@ https://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=111&se
   invoked recursively, warnings can be controlled only for the current
   instance of make using the .WARNINGS variable.
 
+* 'make --print-data-base' (or 'make -p') now outputs time of day
+  using the same form as for file timestamps, e.g., "2023-05-10
+  10:43:57.570558743".  Previously it used the form "Wed May 10
+  10:43:57 2023", which has less detail and is harder to compare.
+
 * Conditional statements starting with the recipe prefix were sometimes
   interpreted in previous versions.  As per the documentation, lines starting
   with the recipe prefix are now never considered conditional statements.
diff --git a/src/read.c b/src/read.c
index 6748486e..a5754a95 100644
--- a/src/read.c
+++ b/src/read.c
@@ -1672,13 +1672,27 @@ conditional_line (char *line, size_t len, const floc *flocp)
       if (termin == ',')
         {
           int count = 0;
-          for (; *line != '\0'; ++line)
-            if (*line == '(')
-              ++count;
-            else if (*line == ')')
-              --count;
-            else if (*line == ',' && count <= 0)
-              break;
+          char *delim = xmalloc (strlen (line));
+          while (*line != '\0')
+            {
+              if (*line == '$')
+                {
+                  ++line;
+                  if (*line == '(')
+                    delim[count++] = ')';
+                  else if (*line == '{')
+                    delim[count++] = '}';
+                }
+              else if (count == 0)
+                {
+                  if (*line == ',')
+                    break;
+                }
+              else if (*line == delim[count-1])
+                --count;
+              ++line;
+            }
+          free (delim);
         }
       else
         while (*line != '\0' && *line != termin)
diff --git a/tests/scripts/features/conditionals b/tests/scripts/features/conditionals
index c1eee95c..c8f14215 100644
--- a/tests/scripts/features/conditionals
+++ b/tests/scripts/features/conditionals
@@ -215,6 +215,44 @@ endif
 !,
     '', "#MAKE#: 'all' is up to date.");
 
+# SV 64402: parse braces in ifeq/ifneq properly
+
+run_make_test(q!
+ifeq ($(and a,b),)
+endif
+ifeq (${and a,b},)
+endif
+ifeq (${and $(x),${or $(z),${q}}},)
+endif
+ifeq (${and ),(},)
+endif
+ifeq ($(and },{),)
+endif
+# We can hide the comma in a variable
+C = ,
+ifeq ((and a$Cb),)
+endif
+all:;
+!,
+    '', "#MAKE#: 'all' is up to date.");
+
+# The "," in "$," is a variable name not a comma
+run_make_test(q!
+ifeq ($,,)
+$(info ok)
+endif
+all:;
+!,
+    '', "ok\n#MAKE#: 'all' is up to date.");
+
+# Without variable references we don't do anything special with parens
+run_make_test(q!
+ifeq ((and a,b),)
+endif
+all:;
+!,
+    '', "#MAKEFILE#:2: extraneous text after 'ifeq' directive\n#MAKE#: 'all' is up to date.");
+
 # This tells the test driver that the perl test script executed properly.
 1;