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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to