[PATCH] D30157: [analyzer] Improve valist check

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun marked 3 inline comments as done.
xazax.hun added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/ValistChecker.cpp:178
+    VaListModelledAsArray = Cast->getCastKind() == CK_ArrayToPointerDecay;
+  const MemRegion *Reg = SV.getAsRegion();
+  if (const auto *DeclReg = Reg->getAs<DeclRegion>()) {
----------------
NoQ wrote:
> I suspect that UnknownVal should also be handled before that, otherwise we'd have null dereference on the next line.
Indeed. 


================
Comment at: test/Analysis/valist-uninitialized-no-undef.c:5
+
+// This is the same function as the previous one, but it is called in call_inlined_uses_arg(),
+// and the warning is generated during the analysis of call_inlined_uses_arg().
----------------
NoQ wrote:
> Hmm, where's the previous one?
Tha calling function is after this one. 


================
Comment at: test/Analysis/valist-uninitialized-no-undef.c:19
+  // FIXME: There should be no warning for this.
+  (void)va_arg(*fst, int); // expected-warning{{va_arg() is called on an uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized va_list}}
+  va_end(*fst);
----------------
NoQ wrote:
> As the patch tries to handle symbolic va_list regions, i wonder what's so particularly hard about this false positive (apart from its being obviously rare, by the way did you actually see such code?).
What is strange, this case does work with the hexagon AST variant. 


https://reviews.llvm.org/D30157