[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

2022-05-12 Thread Aaron Ballman via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc67104172034: Check for resource exhaustion when recursively parsing declarators (authored by aaron.ballman). Repository: rG LLVM Github Monorepo

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

2022-05-12 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. >> Another concern here: Do we properly initialize "Actions" in the case where >> we don't have a semantic Action? That is, if we are just preprocessing? Doh! Forgot we don't do 'Parse

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

2022-05-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 428987. aaron.ballman marked an inline comment as done. aaron.ballman added a comment. Simplify the implementation by using the facilities Sema already exposes. The behavior remains the same: F:\source\llvm-project>llvm\out\build\x64-Debug\bin\clang.

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

2022-05-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang/include/clang/Parse/Parser.h:802 + /// more in that case. Use this in code that may recurse deeply (for example, + /// in template instantiation) to avoid stack overflow. + v

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

2022-05-12 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: clang/lib/Parse/Parser.cpp:2618 + // Only warn about this once. + if (!Actions.WarnedStackExhausted) { +Diag(Loc, diag::warn_stack_exhausted); Another concern here: Do we properly initialize "Actions" in the cas

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

2022-05-12 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. I don't see how you can test this behavior without figuring out how to get a 'perfect' number to warn but not crash... The only way to validate that I would expect would be to do some bizarre flag that has us just 'don't run' in the case where the warning is emitted,

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

2022-05-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 428969. aaron.ballman added a comment. Rebased. I removed the test coverage because it turned out to not be testing anything. If we exhaust resources, then we don't run the `-verify` to test the diagnostic behavior, and we're relying on the crash bein

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

2022-05-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Two things to note: 1. I'm not convinced that adding a test case is the right thing to do here. On the one hand, we want to verify that we now emit the diagnostic before crashing. But on the other hand, this test is extremely expensive to run (because it exhausts

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

2022-05-04 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment. I think I've seen this one before Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124915/new/ https://reviews.llvm.org/D124915 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

2022-05-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision. aaron.ballman added reviewers: erichkeane, rsmith, rjmccall. Herald added a project: All. aaron.ballman requested review of this revision. Herald added a project: clang. With sufficiently tortured code, it's possible to cause a stack overflow when parsing decl