[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

Tue Aug 18 05:29:36 PDT 2020

Szelethus created this revision.
Szelethus added reviewers: NoQ, vsavchenko, xazax.hun, martong, balazske, baloghadamsoftware, gamesh411.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, whisperity.
Szelethus requested review of this revision.

In short, macro expansions handled the case where a variadic //parameter// mapped to multiple //arguments//, but not the other way around. An internal ticket was submitted that demonstrated that we fail an assertion. Macro expansion so far worked by lexing the source code token-by-token and using the `Preprocessor` to turn these tokens into identifiers or just get their proper spelling, but what this counter intuitively doesn't do, is actually expand these macros, so we have to do the heavy lifting -- in this case, figure out what `__VA_ARGS__` expands into. Since this case can only occur in a nested macro, the information we gathered from the containing macro does contain this information. If a parameter resolves to `__VA_ARGS__`, we need to temporarily stop getting our tokens from the lexer, and get the tokens from what `__VA_ARGS__` maps to.

I also found a few more deficiencies I'll have to handle sooner rather then later. I also have 3 commits about to land this is based on, some miscellaneous renames, clarification in the documentation, prettifying some tests.

An educational rant:

For those that didn't have the displeasure of following my macro expansion project, here's why this is so annoying: We really, really suck at understanding macros. D74473 <https://reviews.llvm.org/D74473> is a recent example where we attempt to get the definition of the `EOF` macro, but give up rather fast if it isn't something trivial. D54349#1294765 <https://reviews.llvm.org/D54349#1294765> is also memorable for me. Indeed, whenever we hit a macro, we are in deep trouble.

Since CodeChecker isn't an IDE, what we really wanted to achieve is when the bug path goes through a macro, show what it would expand into.

The fundamental problem is, we simply can't ask `Preprocessor` what a macro expands into without hacking really hard. The HTML output commits about every sin under the sun; hiding under `clang/lib/Rewrite/HTMLRewrite.cpp`, it `const_cast`s a `Preprocessor` object, stores most of its inner state (you can't guarantee you got them all) such as the `DiagnosticsEngine`, some of the user configurations in temporary variables, replaces them with new ones, practically reruns the entire lexing and preprocessing stage of the compiler, and at the end, puts the original inner state back in. This enables it to not have to literally reimplement the preprocessor, but creates a large preprocessed version of the source code, which it is almost impossible to reliably link the two together (answering the question that what a specific macro usage expands into).

Pp-trace, a Clang tool, would be great, but unfortunately it still isn't able to do macro expansions in a single go, just step by step (think about nested macros and variadic arguments).

So, I chose to practically reimplement the entire preprocessor with the goal of taking a single source location of a macro usage, and spitting the entire expansion out. This has a few advantages, namely that it leaves the Preprocessor const, and provides a rather deep understanding of the process of the macro expansion. The problem is, its a nightmare due to the extreme primitiveness of the available tools, and will have to be updated as time goes on.

You can learn a bit more from these patches and discussions:
D52742 <https://reviews.llvm.org/D52742> (check the patch stack)
http://lists.llvm.org/pipermail/cfe-dev/2018-September/059226.html
http://lists.llvm.org/pipermail/cfe-dev/2017-August/055077.html

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86135

Files:
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  clang/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
  clang/test/Analysis/plist-macros-with-expansion.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D86135.286252.patch
Type: text/x-patch
Size: 30246 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200818/684f6330/attachment-0001.bin>