[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

Wed Apr 15 07:05:17 PDT 2020

xazax.hun requested changes to this revision.
xazax.hun added a comment.
This revision now requires changes to proceed.

I am not an expert when it comes to VLAs but I do see some problems here.

First of all, we do not want to include typedef statements in the CFG as they are noops in terms of the execution. We definitely want to include the evaluation of array size expressions for VLAs. In case the evaluation of that expression is not included in the CFG that is definitely a bug but the name of the revision should reflect this.

My questions so far:

- Where is the size expression actually evaluated? Is it evaluated at the point of the typedef or at the point of the variable definition? It is possible to construct code that behaves differently in those cases. In case the behavior is the latter the currently produced CFG is wrong.
- You mentioned that type aliases are not handled. Why? Is there any technical reason for that? Is it a language restriction? Does it behave differently? I think, unless it is really hard to support it well, I would prefer to introduce full support in one patch. Otherwise rewriting a code to use type aliases instead of typedefs would change the behavior of the analyzer on that code which is quite surprising for the users.

I think this needs more testing, i.e. what about more complex size expressions and multiple typedefs/type aliases? Can we have multidimensional VLAs?

================
Comment at: clang/lib/Analysis/CFG.cpp:2843
+
+  if (const auto *TDD = dyn_cast<TypedefDecl>(DS->getSingleDecl())) {
+    // If we encounter a VLA in typedef, then process its size expressions.
----------------
Can't we have the same situation with type aliases? In that case, maybe we should check for `TypedefNameDecl` instead.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77809/new/

https://reviews.llvm.org/D77809