Merge pull request #2532 from EylonKrause/fix/preprocess-macro-call-eof

preprocessor: stop macro-argument scan on an unexpected/EOF token
diff --git a/verible/verilog/preprocessor/verilog-preprocess.cc b/verible/verilog/preprocessor/verilog-preprocess.cc
index 1d847fb..39e816a 100644
--- a/verible/verilog/preprocessor/verilog-preprocess.cc
+++ b/verible/verilog/preprocessor/verilog-preprocess.cc
@@ -272,6 +272,16 @@
     if ((*token_iter)->text() == ")") {
       break;
     }
+    // Any other token -- in particular the EOF token from an unterminated
+    // macro call -- would otherwise leave token_iter and parameters_size
+    // unchanged and spin this loop forever. An early ')' (handled above) is the
+    // legal way to pass fewer arguments than parameters; anything else here is
+    // a malformed call, so reject it rather than silently accepting it. The
+    // caller records the returned message as a preprocessor error.
+    return absl::InvalidArgumentError(absl::StrCat(
+        "unexpected token while scanning arguments of macro call `",
+        macro_name_str,
+        ": expected ',' or ')', but got: ", (*token_iter)->ToString()));
   }
   if (parameters_size > 0) {
     while (parameters_size--) {
@@ -305,8 +315,15 @@
 
   if (config_.expand_macros) {
     verible::MacroCall macro_call;
-    RETURN_IF_ERROR(
-        ConsumeAndParseMacroCall(iter, generator, &macro_call, *found));
+    absl::Status call_status =
+        ConsumeAndParseMacroCall(iter, generator, &macro_call, *found);
+    if (!call_status.ok()) {
+      // Surface a malformed macro call (e.g. an unterminated argument list) as
+      // a preprocessor error at the call site instead of silently accepting it.
+      std::string message(call_status.message());
+      preprocess_data_.errors.emplace_back(**iter, message);
+      return call_status;
+    }
     RETURN_IF_ERROR(ExpandMacro(macro_call, found));
   }
   auto &lexed = preprocess_data_.lexed_macros_backup.back();
diff --git a/verible/verilog/preprocessor/verilog-preprocess_test.cc b/verible/verilog/preprocessor/verilog-preprocess_test.cc
index a8e41d9..c5d8eaa 100644
--- a/verible/verilog/preprocessor/verilog-preprocess_test.cc
+++ b/verible/verilog/preprocessor/verilog-preprocess_test.cc
@@ -139,6 +139,17 @@
   }
 }
 
+// A function-like macro invoked with no closing ')' (EOF reached mid-call) must
+// not spin ConsumeAndParseMacroCall forever. It is a malformed call, so the
+// preprocessor rejects it with an error rather than silently accepting it.
+// (Completing the test at all also proves the scan terminated instead of
+// hanging.)
+TEST(VerilogPreprocessTest, UnterminatedMacroCallIsRejected) {
+  PreprocessorTester tester("`define FOO(a, b) a\n`FOO(x",
+                            VerilogPreprocess::Config({.expand_macros = true}));
+  EXPECT_FALSE(tester.PreprocessorData().errors.empty());
+}
+
 #define EXPECT_PARSE_OK()                                                \
   do {                                                                   \
     EXPECT_TRUE(tester.Status().ok()) << "Unexpected analyzer failure."; \