[clang] [clang] Bounds checking on unclosed parentheses, brackets or braces in Expanded Tokens (PR #69849)

Nathan Ridge via cfe-commits cfe-commits at lists.llvm.org
Sun Dec 3 00:34:58 PST 2023


HighCommander4 wrote:

I spent a bit more time looking at this. I'd say I'm still quite far from having a complete understanding of this code, but two things occur to me:

 1. `TokenBuffer` operates at the level of the preprocessor. "spelledForExpanded" is basically an operation which takes as input a range of tokens in the preprocessed token stream ("expanded tokens"), and maps them back to a range of tokens in the un-preprocessed token stream ("spelled tokens").
    * The preprocessor has no notion of matching or balancing braces. Braces only become relevant later, during parsing.
    * So, I don't understand why an input token stream that contains unbalanced braces would pose a problem for the "spelledForExpanded" operation.
    * I'm hesitant to approve a patch that adds an early exit to "spelledForExpanded" to prevent an out-of-bounds array access, without understanding in more detail **how** the out-of-bounds array access arises / how it's connected to the unbalanced braces. It's possible that the out-of-bounds access is indicative of a deeper logic error which we'd be covering up.
 2. Since we've observed the problem arise during the `dumpAST` operation (frame 10 of the stack trace [here](https://github.com/clangd/clangd/issues/1559#issue-1646293852)), it would be good to (also) add a test that exercises the `dumpAST` operation as a whole.
    * We have such tests in [DumpASTTests.cpp](https://searchfox.org/llvm/source/clang-tools-extra/clangd/unittests/DumpASTTests.cpp).
    * I think it would be preferable if the AST dump for an input with unbalanced braces, such as `int main() {`, was **not** empty. (I suspect the current patch may make it empty.) `clang -Xclang -ast-dump` manages to print AST nodes in this case, I think so should we.

https://github.com/llvm/llvm-project/pull/69849


More information about the cfe-commits mailing list