Merge remote-tracking branch \'aosp/upstream\'

am: fa3234caba

Change-Id: I1eb4edbfe2f5d4fbfe07e1eb15884f0df2e3b0b9

23 files changed, 297 insertions, 81 deletions

diff --git a/.gitignore b/.gitignore
index b907c3d..19d2723 100644
--- a/.gitignore
+++ b/.gitignore
@@ -11,8 +11,10 @@ repo/maloader/
 testcase_parse_benchmark_test.go
 bench-old.out
 bench-new.out
+find_test
 ninja_test
 string_piece_test
+strutil_bench
 strutil_test
 go_src_stamp
 version.cc
diff --git a/.travis.yml b/.travis.yml
index f045dd3..2fd6fa8 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -11,7 +11,10 @@ before_script:
   - sudo apt-get install -y libstdc++-4.8-dev clang-3.5 ninja-build realpath
 
 script:
-  - make -j4 ckati
+  - make -j4 ckati ckati_tests
   - ruby runtest.rb -c
   - ruby runtest.rb -c -n
   - ruby runtest.rb -c -n -a
+  - ./ninja_test
+  - ./string_piece_test
+  - ./strutil_test
diff --git a/AUTHORS b/AUTHORS
index 787d767..3ca8fe5 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -8,5 +8,6 @@
 #
 # Please keep the list sorted.
 
-Kouhei Sutou <kou@cozmixng.org>
 Google Inc.
+Kouhei Sutou <kou@cozmixng.org>
+Po Hu <hupo1985@gmail.com>
diff --git a/CONTRIBUTORS b/CONTRIBUTORS
index 1d81fb2..c153787 100644
--- a/CONTRIBUTORS
+++ b/CONTRIBUTORS
@@ -26,6 +26,7 @@ Colin Cross <ccross@google.com>
 Dan Willemsen <dwillemsen@google.com>
 Fumitoshi Ukai <ukai@google.com>
 Kouhei Sutou <kou@cozmixng.org>
+Po Hu <hupo1985@gmail.com>
 Ryo Hashimoto <hashimoto@google.com>
 Shinichiro Hamaji <hamaji@google.com>
 Stefan Becker <stefanb@gpartner-nvidia.com>
diff --git a/eval.cc b/eval.cc
index 5c9055c..da183f7 100644
--- a/eval.cc
+++ b/eval.cc
@@ -122,7 +122,10 @@ void Evaluator::EvalRule(const RuleStmt* stmt) {
 
   Rule* rule;
   RuleVarAssignment rule_var;
-  ParseRule(loc_, expr, stmt->term, &rule, &rule_var);
+  function<string()> after_term_fn = [this, stmt](){
+    return stmt->after_term ? stmt->after_term->Eval(this) : "";
+  };
+  ParseRule(loc_, expr, stmt->term, after_term_fn, &rule, &rule_var);
 
   if (rule) {
     if (stmt->term == ';') {
@@ -201,8 +204,7 @@ void Evaluator::EvalIf(const IfStmt* stmt) {
       if (lhs.str().find_first_of(" \t") != string::npos)
         Error("*** invalid syntax in conditional.");
       Var* v = LookupVarInCurrentScope(lhs);
-      const string&& s = v->Eval(this);
-      is_true = (s.empty() == (stmt->op == CondOp::IFNDEF));
+      is_true = (v->String().empty() == (stmt->op == CondOp::IFNDEF));
       break;
     }
     case CondOp::IFEQ:
diff --git a/expr.cc b/expr.cc
index ad7d19e..5e7afe6 100644
--- a/expr.cc
+++ b/expr.cc
@@ -537,6 +537,8 @@ Value* ParseExprImpl(const Loc& loc,
         r->AddValue(new Literal(StringPiece(" ")));
         // Skip the current escaped newline
         i += 2;
+        if (n == '\r' && s.get(i) == '\n')
+          i++;
         // Then continue skipping escaped newlines, spaces, and tabs
         for (; i < s.size(); i++) {
           if (s[i] == '\\' && (s.get(i+1) == '\r' || s.get(i+1) == '\n')) {
diff --git a/flags.cc b/flags.cc
index 876e855..1ed9afd 100644
--- a/flags.cc
+++ b/flags.cc
@@ -16,6 +16,7 @@
 
 #include "flags.h"
 
+#include <stdlib.h>
 #include <unistd.h>
 
 #include "log.h"
@@ -52,6 +53,13 @@ void Flags::Parse(int argc, char** argv) {
   num_jobs = num_cpus = sysconf(_SC_NPROCESSORS_ONLN);
   const char* num_jobs_str;
 
+  if (const char* makeflags = getenv("MAKEFLAGS")) {
+    for (StringPiece tok : WordScanner(makeflags)) {
+      if (!HasPrefix(tok, "-") && tok.find('=') != string::npos)
+        cl_vars.push_back(tok);
+    }
+  }
+
   for (int i = 1; i < argc; i++) {
     const char* arg = argv[i];
     bool should_propagate = true;
diff --git a/func.cc b/func.cc
index a7d1c8c..9aaa9a2 100644
--- a/func.cc
+++ b/func.cc
@@ -299,13 +299,18 @@ void JoinFunc(const vector<Value*>& args, Evaluator* ev, string* s) {
   WordScanner ws1(list1);
   WordScanner ws2(list2);
   WordWriter ww(s);
-  for (WordScanner::Iterator iter1 = ws1.begin(), iter2 = ws2.begin();
+  WordScanner::Iterator iter1, iter2;
+  for (iter1 = ws1.begin(), iter2 = ws2.begin();
        iter1 != ws1.end() && iter2 != ws2.end();
        ++iter1, ++iter2) {
     ww.Write(*iter1);
     // Use |AppendString| not to append extra ' '.
     AppendString(*iter2, s);
   }
+  for (; iter1 != ws1.end(); ++iter1)
+    ww.Write(*iter1);
+  for (; iter2 != ws2.end(); ++iter2)
+    ww.Write(*iter2);
 }
 
 void WildcardFunc(const vector<Value*>& args, Evaluator* ev, string* s) {
@@ -458,10 +463,6 @@ void EvalFunc(const vector<Value*>& args, Evaluator* ev, string*) {
   //const string text = args[0]->Eval(ev);
   string* text = new string;
   args[0]->Eval(ev, text);
-  if ((*text)[0] == '#') {
-    delete text;
-    return;
-  }
   if (ev->avoid_io()) {
     KATI_WARN("%s:%d: *warning*: $(eval) in a recipe is not recommended: %s",
               LOCF(ev->loc()), text->c_str());
diff --git a/rule.cc b/rule.cc
index f3f5203..68ec7f5 100644
--- a/rule.cc
+++ b/rule.cc
@@ -54,6 +54,7 @@ Rule::Rule()
 }
 
 void ParseRule(Loc& loc, StringPiece line, char term,
+               function<string()> after_term_fn,
                Rule** out_rule, RuleVarAssignment* rule_var) {
   size_t index = line.find(':');
   if (index == string::npos) {
@@ -84,15 +85,31 @@ void ParseRule(Loc& loc, StringPiece line, char term,
 
   StringPiece rest = line.substr(index);
   size_t term_index = rest.find_first_of("=;");
+  string buf;
   if ((term_index != string::npos && rest[term_index] == '=') ||
       (term_index == string::npos && term == '=')) {
     if (term_index == string::npos)
       term_index = rest.size();
-    rule_var->outputs.swap(outputs);
-    ParseAssignStatement(rest, term_index,
-                         &rule_var->lhs, &rule_var->rhs, &rule_var->op);
-    *out_rule = NULL;
-    return;
+    // "test: =foo" is questionable but a valid rule definition (not a
+    // target specific variable).
+    // See https://github.com/google/kati/issues/83
+    if (term_index == 0) {
+      KATI_WARN("%s:%d: defining a target which starts with `=', "
+                "which is not probably what you meant", LOCF(loc));
+      buf = line.as_string();
+      if (term)
+        buf += term;
+      buf += after_term_fn();
+      line = buf;
+      rest = line.substr(index);
+      term_index = string::npos;
+    } else {
+      rule_var->outputs.swap(outputs);
+      ParseAssignStatement(rest, term_index,
+                           &rule_var->lhs, &rule_var->rhs, &rule_var->op);
+      *out_rule = NULL;
+      return;
+    }
   }
 
   Rule* rule = new Rule();
diff --git a/rule.h b/rule.h
index 2a67368..4d72bb7 100644
--- a/rule.h
+++ b/rule.h
@@ -15,6 +15,8 @@
 #ifndef RULE_H_
 #define RULE_H_
 
+#include <functional>
+#include <string>
 #include <vector>
 
 #include "loc.h"
@@ -58,8 +60,12 @@ struct RuleVarAssignment {
   AssignOp op;
 };
 
-// If |rule| is not NULL, |rule_var| is filled.
+// If |rule| is not NULL, |rule_var| is filled. If the expression
+// after the terminator |term| is needed (this happens only when
+// |term| is '='), |after_term_fn| will be called to obtain the right
+// hand side.
 void ParseRule(Loc& loc, StringPiece line, char term,
+               function<string()> after_term_fn,
                Rule** rule, RuleVarAssignment* rule_var);
 
 #endif  // RULE_H_
diff --git a/strutil.cc b/strutil.cc
index d3c14d8..8f7ff06 100644
--- a/strutil.cc
+++ b/strutil.cc
@@ -21,6 +21,7 @@
 #include <unistd.h>
 
 #include <algorithm>
+#include <functional>
 #include <stack>
 #include <utility>
 
@@ -38,8 +39,9 @@ static bool isSpace(char c) {
 static int SkipUntilSSE42(const char* s, int len,
                           const char* ranges, int ranges_size) {
   __m128i ranges16 = _mm_loadu_si128((const __m128i*)ranges);
+  len &= ~15;
   int i = 0;
-  do {
+  while (i < len) {
     __m128i b16 = _mm_loadu_si128((const __m128i*)(s + i));
     int r = _mm_cmpestri(
         ranges16, ranges_size, b16, len - i,
@@ -48,11 +50,26 @@ static int SkipUntilSSE42(const char* s, int len,
       return i + r;
     }
     i += 16;
-  } while (i < len);
+  }
   return len;
 }
 #endif
 
+template <typename Cond>
+static int SkipUntil(const char* s, int len,
+                     const char* ranges, int ranges_size,
+                     Cond cond) {
+  int i = 0;
+#ifdef __SSE4_2__
+  i += SkipUntilSSE42(s, len, ranges, ranges_size);
+#endif
+  for (; i < len; i++) {
+    if (cond(s[i]))
+      break;
+  }
+  return i;
+}
+
 WordScanner::Iterator& WordScanner::Iterator::operator++() {
   int len = static_cast<int>(in->size());
   for (s = i + 1; s < len; s++) {
@@ -66,17 +83,11 @@ WordScanner::Iterator& WordScanner::Iterator::operator++() {
     return *this;
   }
 
-#ifdef __SSE4_2__
   static const char ranges[] = "\x09\x0d  ";
-  i = s;
-  i += SkipUntilSSE42(in->data() + s, len - s, ranges, 4);
-#else
-  for (i = s; i < len; i++) {
-    if (isSpace((*in)[i]))
-      break;
-  }
-#endif
-
+  // It's intentional we are not using isSpace here. It seems with
+  // lambda the compiler generates better code.
+  i = s + SkipUntil(in->data() + s, len - s, ranges, 4,
+                    [](char c) { return (9 <= c && c <= 13) || c == 32; });
   return *this;
 }
 
@@ -423,10 +434,10 @@ size_t FindThreeOutsideParen(StringPiece s, char c1, char c2, char c3) {
 }
 
 size_t FindEndOfLine(StringPiece s, size_t e, size_t* lf_cnt) {
-#ifdef __SSE4_2__
   static const char ranges[] = "\0\0\n\n\\\\";
   while (e < s.size()) {
-    e += SkipUntilSSE42(s.data() + e, s.size() - e, ranges, 6);
+    e += SkipUntil(s.data() + e, s.size() - e, ranges, 6,
+                   [](char c) { return c == 0 || c == '\n' || c == '\\'; });
     if (e >= s.size()) {
       CHECK(s.size() == e);
       break;
@@ -452,24 +463,6 @@ size_t FindEndOfLine(StringPiece s, size_t e, size_t* lf_cnt) {
     }
   }
   return e;
-#else
-  bool prev_backslash = false;
-  for (; e < s.size(); e++) {
-    char c = s[e];
-    if (c == '\\') {
-      prev_backslash = !prev_backslash;
-    } else if (c == '\n') {
-      ++*lf_cnt;
-      if (!prev_backslash) {
-        return e;
-      }
-      prev_backslash = false;
-    } else if (c != '\r') {
-      prev_backslash = false;
-    }
-  }
-  return e;
-#endif
 }
 
 StringPiece TrimLeadingCurdir(StringPiece s) {
@@ -528,11 +521,14 @@ string EchoEscape(const string str) {
   return buf;
 }
 
+static bool NeedsShellEscape(char c) {
+  return c == 0 || c == '"' || c == '$' || c == '\\' || c == '`';
+}
+
 void EscapeShell(string* s) {
-#ifdef __SSE4_2__
   static const char ranges[] = "\0\0\"\"$$\\\\``";
   size_t prev = 0;
-  size_t i = SkipUntilSSE42(s->c_str(), s->size(), ranges, 10);
+  size_t i = SkipUntil(s->c_str(), s->size(), ranges, 10, NeedsShellEscape);
   if (i == s->size())
     return;
 
@@ -550,37 +546,8 @@ void EscapeShell(string* s) {
     r += c;
     i++;
     prev = i;
-    i += SkipUntilSSE42(s->c_str() + i, s->size() - i, ranges, 10);
+    i += SkipUntil(s->c_str() + i, s->size() - i, ranges, 10, NeedsShellEscape);
   }
   StringPiece(*s).substr(prev).AppendToString(&r);
   s->swap(r);
-#else
-  if (s->find_first_of("$`\\\"") == string::npos)
-    return;
-  string r;
-  bool last_dollar = false;
-  for (char c : *s) {
-    switch (c) {
-      case '$':
-        if (last_dollar) {
-          r += c;
-          last_dollar = false;
-        } else {
-          r += '\\';
-          r += c;
-          last_dollar = true;
-        }
-        break;
-      case '`':
-      case '"':
-      case '\\':
-        r += '\\';
-        // fall through.
-      default:
-        r += c;
-        last_dollar = false;
-    }
-  }
-  s->swap(r);
-#endif
 }
diff --git a/strutil_test.cc b/strutil_test.cc
index a89786f..7a5a47d 100644
--- a/strutil_test.cc
+++ b/strutil_test.cc
@@ -17,6 +17,8 @@
 #include "strutil.h"
 
 #include <assert.h>
+#include <sys/mman.h>
+#include <unistd.h>
 
 #include <string>
 #include <vector>
@@ -138,6 +140,50 @@ void TestFindEndOfLine() {
   ASSERT_EQ(FindEndOfLine(StringPiece(buf, 2), 0, &lf_cnt), 2);
 }
 
+// Take a string, and copy it into an allocated buffer where
+// the byte immediately after the null termination character
+// is read protected. Useful for testing, but doesn't support
+// freeing the allocated pages.
+const char* CreateProtectedString(const char* str) {
+  int pagesize = sysconf(_SC_PAGE_SIZE);
+  void *buffer;
+  char *buffer_str;
+
+  // Allocate two pages of memory
+  if (posix_memalign(&buffer, pagesize, pagesize * 2) != 0) {
+    perror("posix_memalign failed");
+    assert(false);
+  }
+
+  // Make the second page unreadable
+  buffer_str = (char*)buffer + pagesize;
+  if (mprotect(buffer_str, pagesize, PROT_NONE) != 0) {
+    perror("mprotect failed");
+    assert(false);
+  }
+
+  // Then move the test string into the very end of the first page
+  buffer_str -= strlen(str) + 1;
+  strcpy(buffer_str, str);
+
+  return buffer_str;
+}
+
+void TestWordScannerInvalidAccess() {
+  vector<StringPiece> ss;
+  for (StringPiece tok : WordScanner(CreateProtectedString("0123 456789"))) {
+    ss.push_back(tok);
+  }
+  assert(ss.size() == 2LU);
+  ASSERT_EQ(ss[0], "0123");
+  ASSERT_EQ(ss[1], "456789");
+}
+
+void TestFindEndOfLineInvalidAccess() {
+  size_t lf_cnt = 0;
+  ASSERT_EQ(FindEndOfLine(CreateProtectedString("a\\"), 0, &lf_cnt), 2);
+}
+
 }  // namespace
 
 int main() {
@@ -150,5 +196,7 @@ int main() {
   TestNormalizePath();
   TestEscapeShell();
   TestFindEndOfLine();
+  TestWordScannerInvalidAccess();
+  TestFindEndOfLineInvalidAccess();
   assert(!g_failed);
 }
diff --git a/symtab.cc b/symtab.cc
index b47522b..cbaa4b4 100644
--- a/symtab.cc
+++ b/symtab.cc
@@ -69,6 +69,10 @@ void Symbol::SetGlobalVar(Var* v, bool is_override) const {
        orig->Origin() == VarOrigin::ENVIRONMENT_OVERRIDE)) {
     return;
   }
+  if (orig->Origin() == VarOrigin::COMMAND_LINE &&
+         v->Origin() == VarOrigin::FILE) {
+    return;
+  }
   if (orig->Origin() == VarOrigin::AUTOMATIC) {
     ERROR("overriding automatic variable is not implemented yet");
   }
diff --git a/testcase/cmdline_var.sh b/testcase/cmdline_var.sh
new file mode 100755
index 0000000..b892736
--- /dev/null
+++ b/testcase/cmdline_var.sh
@@ -0,0 +1,27 @@
+#!/bin/sh
+#
+# Copyright 2016 Google Inc. All rights reserved
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http:#www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -e
+
+mk="$@"
+
+cat <<EOF > Makefile
+CLVAR := FAIL
+all:
+	@echo \$(CLVAR)
+EOF
+
+${mk} CLVAR:=PASS 2> /dev/null
diff --git a/testcase/cmdline_var_makeflags.sh b/testcase/cmdline_var_makeflags.sh
new file mode 100755
index 0000000..6e38a2d
--- /dev/null
+++ b/testcase/cmdline_var_makeflags.sh
@@ -0,0 +1,36 @@
+#!/bin/sh
+#
+# Copyright 2016 Google Inc. All rights reserved
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http:#www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -e
+
+mk="$@"
+
+cat <<EOF > Makefile
+CLVAR := FAIL
+MFVAR := FAIL
+FILEVAR := PASS
+all:
+	@echo \$(ENVVAR) \$(origin ENVVAR)
+	@echo \$(MFVAR) \$(origin MFVAR)
+	@echo \$(CLVAR) \$(origin CLVAR)
+	@echo \$(FILEVAR) \$(origin FILEVAR)
+EOF
+
+export ENVVAR=PASS
+export FILEVAR=FAIL
+export MAKEFLAGS="MFVAR=PASS CLVAR=FAIL"
+
+${mk} CLVAR=PASS 2> /dev/null
diff --git a/testcase/cmdline_var_modify.sh b/testcase/cmdline_var_modify.sh
new file mode 100755
index 0000000..9616a2b
--- /dev/null
+++ b/testcase/cmdline_var_modify.sh
@@ -0,0 +1,27 @@
+#!/bin/sh
+#
+# Copyright 2016 Google Inc. All rights reserved
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http:#www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -e
+
+mk="$@"
+
+cat <<EOF > Makefile
+CLVAR := FAIL
+all:
+	@echo \$(CLVAR)
+EOF
+
+${mk} CLVAR:=P CLVAR+=A CLVAR+=SS CLVAR?=FAIL 2> /dev/null
diff --git a/testcase/cmdline_var_override.sh b/testcase/cmdline_var_override.sh
new file mode 100755
index 0000000..0a5b3c1
--- /dev/null
+++ b/testcase/cmdline_var_override.sh
@@ -0,0 +1,27 @@
+#!/bin/sh
+#
+# Copyright 2016 Google Inc. All rights reserved
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http:#www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -e
+
+mk="$@"
+
+cat <<EOF > Makefile
+override CLVAR := PASS
+all:
+	@echo \$(CLVAR)
+EOF
+
+${mk} CLVAR:=FAIL 2> /dev/null
diff --git a/testcase/crlf.mk b/testcase/crlf.mk
index bcfc169..2dcfcd7 100644
--- a/testcase/crlf.mk
+++ b/testcase/crlf.mk
@@ -1,4 +1,5 @@
 PASS := \

+  PASS \

   PASS

 

 test:

diff --git a/testcase/empty_target_specific_var.mk b/testcase/empty_target_specific_var.mk
new file mode 100644
index 0000000..fd9e6c1
--- /dev/null
+++ b/testcase/empty_target_specific_var.mk
@@ -0,0 +1,7 @@
+# TODO(go): https://github.com/google/kati/issues/83
+
+test: =foo
+
+var==foo
+$(var):
+	echo PASS
diff --git a/testcase/empty_target_specific_var2.mk b/testcase/empty_target_specific_var2.mk
new file mode 100644
index 0000000..6defb52
--- /dev/null
+++ b/testcase/empty_target_specific_var2.mk
@@ -0,0 +1,11 @@
+# TODO(go): https://github.com/google/kati/issues/83
+
+define var
+VAR:=1
+endef
+
+$(call var)
+
+eq_one:==1
+$(eq_one):
+	echo PASS
diff --git a/testcase/eval_starts_with_comment.mk b/testcase/eval_starts_with_comment.mk
new file mode 100644
index 0000000..c3adca4
--- /dev/null
+++ b/testcase/eval_starts_with_comment.mk
@@ -0,0 +1,9 @@
+.PHONY: test
+
+define _rule
+# comment
+test:
+	echo PASS
+endef
+
+$(eval $(_rule))
diff --git a/testcase/ifdef_rec_var.mk b/testcase/ifdef_rec_var.mk
new file mode 100644
index 0000000..0874d2e
--- /dev/null
+++ b/testcase/ifdef_rec_var.mk
@@ -0,0 +1,8 @@
+empty=$(info FAIL)
+rec=$(empty)
+
+ifdef rec
+$(info PASS)
+else
+$(info FAIL)
+endif
diff --git a/testcase/join.mk b/testcase/join.mk
index cafe395..2772740 100644
--- a/testcase/join.mk
+++ b/testcase/join.mk
@@ -3,4 +3,5 @@ foo:=$(join a b,.c .o)
 # produces `a.c b.o'.
 test:
 	echo $(foo)
-
+	echo $(join a b c, 0 1)
+	echo $(join a b, 0 1 2)