Skip to content

preprocessor: stop macro-argument scan on an unexpected/EOF token - #2532

Open
EylonKrause wants to merge 3 commits into
chipsalliance:masterfrom
EylonKrause:fix/preprocess-macro-call-eof
Open

preprocessor: stop macro-argument scan on an unexpected/EOF token#2532
EylonKrause wants to merge 3 commits into
chipsalliance:masterfrom
EylonKrause:fix/preprocess-macro-call-eof

Conversation

@EylonKrause

Copy link
Copy Markdown
Contributor

Problem

VerilogPreprocess::ConsumeAndParseMacroCall hangs (infinite loop) on an unterminated macro call.

Root cause

The while (parameters_size > 0) argument-scan loop (verilog/preprocessor/verilog-preprocess.cc:252) only handles three token kinds: MacroArg, ,, and ). An unterminated call such as `FOO( at end of input leaves the current token as the EOF token, which matches none of the three branches — so neither token_iter nor parameters_size advances and the loop spins forever. This is reached from untrusted/truncated .sv via AnalyzeAutomaticPreprocessFallback (expand_macros=true) in both the linter and the LSP.

Fix

Break out of the loop on any other token; the existing trailing loop already back-fills the remaining parameters with default TokenInfo (the same terminal state an early ) produces), so the fix routes into a path the tests already exercise.

Related (not fixed here)

On the macro-expansion paths (ExpandMacro/ExpandText), the lexed sequence is built without a trailing EOF sentinel, so GenerateBypassWhiteSpaces can dereference an exhausted generator (crash) on similar input. That needs a separate structural fix (append an EOF sentinel / add an end-guard) — happy to follow up in a second PR.

Verification

Static reasoning; the `MACRO3 () test already exercises the all-defaults back-fill path this fix routes into. (Bazel build not set up locally.)

ConsumeAndParseMacroCall's `while (parameters_size > 0)` argument loop only
handled MacroArg, ',' and ')'. An unterminated macro call (e.g. `\`FOO(` at
end of file) leaves the current token as EOF, which matches none of the
three branches, so neither token_iter nor parameters_size advances and the
loop spins forever -- a hang when linting or serving a truncated/untrusted
.sv (both reach this via AnalyzeAutomaticPreprocessFallback with
expand_macros=true). Break out on any other token; the existing trailing
loop already back-fills the remaining parameters with default TokenInfo,
the same terminal state an early ')' produces.

Note: a related path (GenerateBypassWhiteSpaces dereferencing an exhausted
generator when a macro-expansion token sequence lacks a trailing EOF
sentinel) can crash on similar input; happy to follow up separately.

Signed-off-by: Eylon Krause <eylon1909@gmail.com>
@linux-foundation-easycla

linux-foundation-easycla Bot commented Jul 5, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

@EylonKrause

Copy link
Copy Markdown
Contributor Author

Disclosure: this contribution was authored with an AI coding assistant (Claude) and reviewed before submission.

Signed-off-by: Eylon Krause <eylon1909@gmail.com>
// unchanged and spin this loop forever. Stop scanning; the loop below
// back-fills the remaining parameters with default TokenInfo (the same
// terminal state produced by an early ')').
break;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if instead of silently accepting the issue, we should may return absl::InvalidArgumentError() ?

…ting

Address review feedback: when the macro-argument scan reaches an
unexpected token (in particular the EOF from an unterminated call),
ConsumeAndParseMacroCall now returns an InvalidArgumentError and the
caller records it as a preprocessor error at the call site, rather than
silently back-filling the arguments. An early ')' remains the legal way
to pass fewer arguments than parameters.

Signed-off-by: Eylon Krause <eylon1909@gmail.com>
@EylonKrause

Copy link
Copy Markdown
Contributor Author

Good idea — done. ConsumeAndParseMacroCall now returns absl::InvalidArgumentError when the argument scan hits an unexpected token (in particular the EOF of an unterminated call), and the caller (HandleMacroIdentifier) records it as a preprocessor error at the call site, so a malformed call surfaces a real diagnostic instead of being silently back-filled. An early ) remains the legal way to pass fewer arguments than parameters. The test now asserts the error is reported.

(ConsumeAndParseMacroCall is static, so it can't push to preprocess_data_.errors itself; the caller records the returned message — mirroring the existing undefined-macro path.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants