[preprocessor] Fix crash/hang on truncated callable macro invocation
GenerateBypassWhiteSpaces dereferences the stream iterator (**iterator) with no
end guard, relying on every stream ending in a kept EOF token. The streams
re-lexed for macro expansion in ExpandText, ExpandMacro, HandleInclude and the
standalone tool strip the EOF (loop stops on !isEOF()), so a callable macro
invocation truncated at end-of-stream (e.g. `define A(x) hello `A followed by
`A(1)) makes the streamer return the view's end() iterator and the deref reads
past the end -> SIGSEGV. An included file ending in a callable macro crashes the
same way; a '(' with no ')' spins forever.
Restore the kept-EOF sentinel on each re-lexed stream so the whitespace-skip
loop stops at EOF and callers return a diagnostic instead of dereferencing past
the end: append the EOF sentinel in all four stream builders; break on it in the
two token-pulling loops so it is not forwarded; skip it when splicing an included
child stream into the parent; and break on EOF in the argument-scanning loop
(the one caller that did not handle a mid-scan EOF, which otherwise hangs on a
'(' without ')'). Also record the "callable macro without ()" error in
preprocess_data_.errors instead of silently swallowing it (requires making
ConsumeAndParseMacroCall non-static).
Adds TruncatedCallableMacroDoesNotCrash. Existing preprocessor and analyzer test
suites pass. Unbounded macro self-recursion is a separate pre-existing bug and
is not addressed here.
Signed-off-by: Eylon Krause <eylon1909@gmail.com>
diff --git a/verible/verilog/preprocessor/verilog-preprocess.cc b/verible/verilog/preprocessor/verilog-preprocess.cc
index 2f77111..1d847fb 100644
--- a/verible/verilog/preprocessor/verilog-preprocess.cc
+++ b/verible/verilog/preprocessor/verilog-preprocess.cc
@@ -245,11 +245,15 @@
if ((*token_iter)->text() == "(") {
token_iter = GenerateBypassWhiteSpaces(generator); // skip the "("
} else {
+ preprocess_data_.errors.emplace_back(
+ **token_iter,
+ "Error it is illegal to call a callable macro without ().");
return absl::InvalidArgumentError(
"Error it is illegal to call a callable macro without ().");
}
while (parameters_size > 0) {
+ if ((*token_iter)->isEOF()) break; // truncated call; stop scanning args
if ((*token_iter)->token_enum() == MacroArg) {
macro_call->positional_arguments.emplace_back(**token_iter);
token_iter = GenerateBypassWhiteSpaces(generator);
@@ -342,6 +346,10 @@
lexer.DoNextToken()) {
lexed_sequence.push_back(lexer.GetLastToken());
}
+ // Retain the EOF token as an end sentinel so a truncated callable-macro
+ // invocation stops at EOF in GenerateBypassWhiteSpaces instead of
+ // dereferencing past the end of the stream view.
+ lexed_sequence.push_back(lexer.GetLastToken());
verible::TokenStreamView lexed_streamview;
// Initializing the lexed token stream view.
InitTokenStreamView(lexed_sequence, &lexed_streamview);
@@ -352,6 +360,7 @@
// Token-pulling loop.
for (auto iter = iter_generator(); iter != end; iter = iter_generator()) {
auto &last_token = **iter;
+ if (last_token.isEOF()) break; // end sentinel; nothing to forward
// TODO: handle lexical error
if (lexer.GetLastToken().token_enum() == TK_SPACE) {
continue; // don't forward spaces
@@ -396,6 +405,8 @@
lexer.DoNextToken()) {
lexed_sequence.push_back(lexer.GetLastToken());
}
+ // Retain EOF end sentinel (see ExpandText).
+ lexed_sequence.push_back(lexer.GetLastToken());
verible::TokenStreamView lexed_streamview;
// Initializing the lexed token stream view.
InitTokenStreamView(lexed_sequence, &lexed_streamview);
@@ -407,6 +418,7 @@
for (auto iter = iter_generator(); iter != end; iter = iter_generator()) {
// TODO: handle lexical error
auto &last_token = **iter;
+ if (last_token.isEOF()) break; // end sentinel; nothing to forward
if (last_token.token_enum() == TK_SPACE) continue; // don't forward spaces
// If the expanded token is another macro identifier that needs to be
// expanded.
@@ -635,6 +647,9 @@
lexer.DoNextToken()) {
included_sequence.push_back(lexer.GetLastToken());
}
+ // Retain EOF end sentinel; the child ScanStream expects an EOF-terminated
+ // stream.
+ included_sequence.push_back(lexer.GetLastToken());
// Preprocessing the included file tokens.
verible::TokenStreamView lexed_streamview;
@@ -657,8 +672,11 @@
preprocess_data_.included_text_structure.push_back(std::move(u));
}
- // Forwarding the included preprocessed view.
+ // Forwarding the included preprocessed view. The EOF end sentinel appended
+ // above is consumed by the child ScanStream and must not be spliced into the
+ // middle of the parent's token stream.
for (const auto &u : child_preprocessed_data.preprocessed_token_stream) {
+ if (u->isEOF()) continue;
preprocess_data_.preprocessed_token_stream.push_back(u);
}
diff --git a/verible/verilog/preprocessor/verilog-preprocess.h b/verible/verilog/preprocessor/verilog-preprocess.h
index 2d134ae..0d932f0 100644
--- a/verible/verilog/preprocessor/verilog-preprocess.h
+++ b/verible/verilog/preprocessor/verilog-preprocess.h
@@ -164,9 +164,11 @@
absl::Status HandleElse(TokenStreamView::const_iterator else_pos);
absl::Status HandleEndif(TokenStreamView::const_iterator endif_pos);
- static absl::Status ConsumeAndParseMacroCall(
- TokenStreamView::const_iterator, const StreamIteratorGenerator &,
- verible::MacroCall *, const verible::MacroDefinition &);
+ // Non-static so it can record diagnostics into preprocess_data_.errors.
+ absl::Status ConsumeAndParseMacroCall(TokenStreamView::const_iterator,
+ const StreamIteratorGenerator &,
+ verible::MacroCall *,
+ const verible::MacroDefinition &);
// The following functions return nullptr when there is no error:
absl::Status ConsumeMacroDefinition(const StreamIteratorGenerator &,
diff --git a/verible/verilog/preprocessor/verilog-preprocess_test.cc b/verible/verilog/preprocessor/verilog-preprocess_test.cc
index ae33567..a8e41d9 100644
--- a/verible/verilog/preprocessor/verilog-preprocess_test.cc
+++ b/verible/verilog/preprocessor/verilog-preprocess_test.cc
@@ -1033,5 +1033,30 @@
<< error.error_message;
}
+// Regression: a callable-macro invocation truncated at end-of-stream (no '(',
+// or '(' with no matching ')') must not crash or hang the preprocessor. Before
+// the fix these inputs dereferenced past the end of the token stream view
+// (SIGSEGV) or spun forever scanning arguments. With error-surfacing enabled
+// the no-'(' cases also report a preprocessor diagnostic.
+TEST(VerilogPreprocessTest, TruncatedCallableMacroDoesNotCrash) {
+ constexpr std::string_view kNoParenInputs[] = {
+ "`define A(x) hello `A\n`A(1)\n", // truncated callable ref in macro body
+ "`define A(x) x\n`A\n", // truncated callable ref at top level
+ };
+ for (std::string_view input : kNoParenInputs) {
+ PreprocessorTester tester(
+ input, VerilogPreprocess::Config({.expand_macros = true}));
+ EXPECT_FALSE(tester.Status().ok()) << input;
+ EXPECT_GE(tester.PreprocessorData().errors.size(), 1) << input;
+ }
+
+ // '(' with no matching ')': must terminate (was an infinite loop). The
+ // residue is rejected downstream, so only assert non-OK here.
+ PreprocessorTester open_paren(
+ "`define C(z) z\n`define A(x) hello `C(\n`A(1)\n",
+ VerilogPreprocess::Config({.expand_macros = true}));
+ EXPECT_FALSE(open_paren.Status().ok());
+}
+
} // namespace
} // namespace verilog
diff --git a/verible/verilog/tools/preprocessor/verilog-preprocessor.cc b/verible/verilog/tools/preprocessor/verilog-preprocessor.cc
index c23fdf7..af781f3 100644
--- a/verible/verilog/tools/preprocessor/verilog-preprocessor.cc
+++ b/verible/verilog/tools/preprocessor/verilog-preprocessor.cc
@@ -122,6 +122,7 @@
// source code just like it was, but with conditionals filtered.
lexed_sequence.push_back(lexer.GetLastToken());
}
+ lexed_sequence.push_back(lexer.GetLastToken()); // EOF end sentinel
verible::TokenStreamView lexed_streamview;
// Initializing the lexed token stream view.
InitTokenStreamView(lexed_sequence, &lexed_streamview);
