From f9d10b2b6314ea2a80515112498aaa919ad81c97 Mon Sep 17 00:00:00 2001 From: Ron Yorston Date: Fri, 31 May 2024 10:27:18 +0100 Subject: make: fix detection of target rules (take 2) Commit d6b764116 (make: fix detection of target rules) checked for target rules before macro assignments. This failed for some Makefiles generated by autotools because partially defined macros were expanded while testing for a target rule. Revert to checking for macro assignments first, but try to detect if the proposed left hand side of the assignment might form part of a target rule with an inline command. Also handle the case where the ';' separator of the inline command has been obfuscated by putting it in a macro. Saves 128-160 bytes. (GitHub pdpmake issues 31, 44) --- miscutils/make.c | 338 +++++++++++++++++++++++---------------------------- testsuite/make.tests | 23 ++++ 2 files changed, 177 insertions(+), 184 deletions(-) diff --git a/miscutils/make.c b/miscutils/make.c index 8d5664500..02da3611c 100644 --- a/miscutils/make.c +++ b/miscutils/make.c @@ -1222,40 +1222,6 @@ modify_words(const char *val, int modifier, size_t lenf, size_t lenr, return buf; } -/* - * Try to detect a target rule by searching for a colon that isn't part - * of a macro assignment. Macros must have been expanded already. Return - * a pointer to the colon or NULL. - */ -static char * -find_colon(char *p) -{ - char *q; - -#if ENABLE_PLATFORM_MINGW32 - for (q = p; (q = strchr(q, ':')); ++q) { - if (posix && !(pragma & P_WINDOWS)) - break; - if (q == p || !isalpha(q[-1]) || q[1] != '/') - break; - } - if (q != NULL) { -#else - if ((q = strchr(p, ':')) != NULL) { -#endif - // Skip ':=', '::=' and ':::=' macro assignments - if ( - // '::=' and ':::=' are from POSIX 202X. - !(!POSIX_2017 && q[1] == ':' && q[2] == ':' && q[3] == '=') && - !(!POSIX_2017 && q[1] == ':' && q[2] == '=') && - // ':=' is a non-POSIX extension - !(!posix && q[1] == '=') - ) - return q; - } - return NULL; -} - /* * Return a pointer to the next instance of a given character. Macro * expansions are skipped so the ':' and '=' in $(VAR:.s1=.s2) aren't @@ -2032,159 +1998,22 @@ input(FILE *fd, int ilevel) goto end_loop; } - // Check for target rule - a = p = expanded = expand_macros(str, FALSE); - if ((q = find_colon(p)) != NULL) { - // All tokens before ':' must be valid targets - *q = '\0'; - while ((a = gettok(&p)) != NULL && is_valid_target(a)) - ; - } - free(expanded); - - if (a == NULL) { - // Looks like a target rule - p = expanded = expand_macros(str, FALSE); - - // Look for colon separator - q = find_colon(p); - if (q == NULL) - error("expected separator"); - - *q++ = '\0'; // Separate targets and prerequisites - - // Double colon - dbl = !posix && *q == ':'; - if (dbl) - q++; - - // Look for semicolon separator - cp = NULL; - s = strchr(q, ';'); - if (s) { - *s = '\0'; - // Retrieve command from copy of line - if ((p = find_char(copy, ':')) && (p = strchr(p, ';'))) - newcmd(&cp, process_command(p + 1)); - } - semicolon_cmd = cp != NULL; - - // Create list of prerequisites - dp = NULL; - while (((p = gettok(&q)) != NULL)) { - char *newp = NULL; - - if (!posix) { - // Allow prerequisites of form library(member1 member2). - // Leading and trailing spaces in the brackets are skipped. - if (!lib) { - s = strchr(p, '('); - if (s && !ends_with_bracket(s) && strchr(q, ')')) { - // Looks like an unterminated archive member - // with a terminator later on the line. - lib = p; - if (s[1] != '\0') { - p = newp = auto_concat(lib, ")"); - s[1] = '\0'; - } else { - continue; - } - } - } else if (ends_with_bracket(p)) { - if (*p != ')') - p = newp = auto_concat(lib, p); - lib = NULL; - if (newp == NULL) - continue; - } else { - p = newp = auto_string(xasprintf("%s%s)", lib, p)); - } - } - - // If not in POSIX mode expand wildcards in the name. - nfile = 1; - files = &p; - if (!posix && wildcard(p, &gd)) { - nfile = gd.gl_pathc; - files = gd.gl_pathv; - } - for (i = 0; i < nfile; ++i) { - if (!POSIX_2017 && strcmp(files[i], ".WAIT") == 0) - continue; - np = newname(files[i]); - newdep(&dp, np); - } - if (files != &p) - globfree(&gd); - free(newp); - } - lib = NULL; - - // Create list of commands - startno = dispno; - while ((str2 = readline(fd)) && *str2 == '\t') { - newcmd(&cp, process_command(str2)); - free(str2); - } - dispno = startno; - - // Create target names and attach rule to them - q = expanded; - count = 0; - seen_inference = FALSE; - while ((p = gettok(&q)) != NULL) { - // If not in POSIX mode expand wildcards in the name. - nfile = 1; - files = &p; - if (!posix && wildcard(p, &gd)) { - nfile = gd.gl_pathc; - files = gd.gl_pathv; - } - for (i = 0; i < nfile; ++i) { - int ttype = target_type(files[i]); - - np = newname(files[i]); - if (ttype != T_NORMAL) { - if (ttype == T_INFERENCE && posix) { - if (semicolon_cmd) - error_in_inference_rule("'; command'"); - seen_inference = TRUE; - } - np->n_flag |= N_SPECIAL; - } else if (!firstname) { - firstname = np; - } - addrule(np, dp, cp, dbl); - count++; - } - if (files != &p) - globfree(&gd); - } - if (seen_inference && count != 1) - error_in_inference_rule("multiple targets"); - - // Prerequisites and commands will be unused if there were - // no targets. Avoid leaking memory. - if (count == 0) { - freedeps(dp); - freecmds(cp); - } - goto end_loop; - } - - // If we get here it must be a macro definition - q = find_char(str, '='); - if (q != NULL) { + // Check for a macro definition + if (find_char(str, '=') != NULL) { int level = (useenv || fd == NULL) ? 4 : 3; + // Use a copy of the line: we might need the original + // if this turns out to be a target rule. + char *copy2 = xstrdup(str); char *newq = NULL; char eq = '\0'; + q = find_char(copy2, '='); // q can't be NULL - if (q - 1 > str) { + if (q - 1 > copy2) { switch (q[-1]) { case ':': // '::=' and ':::=' are from POSIX 202X. - if (!POSIX_2017 && q - 2 > str && q[-2] == ':') { - if (q - 3 > str && q[-3] == ':') { + if (!POSIX_2017 && q - 2 > copy2 && q[-2] == ':') { + if (q - 3 > copy2 && q[-3] == ':') { eq = 'B'; // BSD-style ':=' q[-3] = '\0'; } else { @@ -2216,8 +2045,19 @@ input(FILE *fd, int ilevel) *p = '\0'; // Expand left-hand side of assignment - p = expanded = expand_macros(str, FALSE); - if ((a = gettok(&p)) == NULL || gettok(&p)) + p = expanded = expand_macros(copy2, FALSE); + if ((a = gettok(&p)) == NULL) + error("invalid macro assignment"); + + // If the expanded LHS contains ':' and ';' it can't be a + // macro assignment but it might be a target rule. + if ((s = strchr(a, ':')) != NULL && strchr(s, ';') != NULL) { + free(expanded); + free(copy2); + goto try_target; + } + + if (gettok(&p)) error("invalid macro assignment"); if (eq == ':') { @@ -2257,8 +2097,138 @@ input(FILE *fd, int ilevel) } setmacro(a, q, level); free(newq); - } else { - error("missing separator"); + free(copy2); + goto end_loop; + } + + // If we get here it must be a target rule + try_target: + p = expanded = expand_macros(str, FALSE); + + // Look for colon separator + q = find_char(p, ':'); + if (q == NULL) + error("expected separator"); + + *q++ = '\0'; // Separate targets and prerequisites + + // Double colon + dbl = !posix && *q == ':'; + if (dbl) + q++; + + // Look for semicolon separator + cp = NULL; + s = strchr(q, ';'); + if (s) { + // Retrieve command from expanded copy of line + char *copy3 = expand_macros(copy, FALSE); + if ((p = find_char(copy3, ':')) && (p = strchr(p, ';'))) + newcmd(&cp, process_command(p + 1)); + free(copy3); + *s = '\0'; + } + semicolon_cmd = cp != NULL; + + // Create list of prerequisites + dp = NULL; + while (((p = gettok(&q)) != NULL)) { + char *newp = NULL; + + if (!posix) { + // Allow prerequisites of form library(member1 member2). + // Leading and trailing spaces in the brackets are skipped. + if (!lib) { + s = strchr(p, '('); + if (s && !ends_with_bracket(s) && strchr(q, ')')) { + // Looks like an unterminated archive member + // with a terminator later on the line. + lib = p; + if (s[1] != '\0') { + p = newp = auto_concat(lib, ")"); + s[1] = '\0'; + } else { + continue; + } + } + } else if (ends_with_bracket(p)) { + if (*p != ')') + p = newp = auto_concat(lib, p); + lib = NULL; + if (newp == NULL) + continue; + } else { + p = newp = auto_string(xasprintf("%s%s)", lib, p)); + } + } + + // If not in POSIX mode expand wildcards in the name. + nfile = 1; + files = &p; + if (!posix && wildcard(p, &gd)) { + nfile = gd.gl_pathc; + files = gd.gl_pathv; + } + for (i = 0; i < nfile; ++i) { + if (!POSIX_2017 && strcmp(files[i], ".WAIT") == 0) + continue; + np = newname(files[i]); + newdep(&dp, np); + } + if (files != &p) + globfree(&gd); + free(newp); + } + lib = NULL; + + // Create list of commands + startno = dispno; + while ((str2 = readline(fd)) && *str2 == '\t') { + newcmd(&cp, process_command(str2)); + free(str2); + } + dispno = startno; + + // Create target names and attach rule to them + q = expanded; + count = 0; + seen_inference = FALSE; + while ((p = gettok(&q)) != NULL) { + // If not in POSIX mode expand wildcards in the name. + nfile = 1; + files = &p; + if (!posix && wildcard(p, &gd)) { + nfile = gd.gl_pathc; + files = gd.gl_pathv; + } + for (i = 0; i < nfile; ++i) { + int ttype = target_type(files[i]); + + np = newname(files[i]); + if (ttype != T_NORMAL) { + if (ttype == T_INFERENCE && posix) { + if (semicolon_cmd) + error_in_inference_rule("'; command'"); + seen_inference = TRUE; + } + np->n_flag |= N_SPECIAL; + } else if (!firstname) { + firstname = np; + } + addrule(np, dp, cp, dbl); + count++; + } + if (files != &p) + globfree(&gd); + } + if (seen_inference && count != 1) + error_in_inference_rule("multiple targets"); + + // Prerequisites and commands will be unused if there were + // no targets. Avoid leaking memory. + if (count == 0) { + freedeps(dp); + freecmds(cp); } end_loop: diff --git a/testsuite/make.tests b/testsuite/make.tests index 0397ab4de..6438c90c9 100755 --- a/testsuite/make.tests +++ b/testsuite/make.tests @@ -103,6 +103,29 @@ a = a target:;@echo a = $(a) ' +# Ensure an inline command on a target rule can be detected even if +# the semicolon is obfuscated. +testing "make equal sign in obfuscated inline command" \ + "make -f -" "a = a\n" "" ' +a = a +semi = ; +target:$(semi)@echo a = $(a) +' + +# The fix for the above test broke a complex chain of macro assignments +# generated by autotools. +testing "make complex chain of macro assignments" \ + "make -f -" "flag 1\n" "" ' +FLAG_ = $(FLAG_$(VALUE)) +FLAG_0 = flag 0 +FLAG_1 = flag 1 +MYFLAG = $(FLAG_$(VALUE)) +VALUE = 1 + +target: + @echo $(MYFLAG) +' + # When a build command fails and the '-k' option has been provided # (continue execution on error) no further commands should be executed # for the current target. -- cgit v1.2.3-55-g6feb