[clang] be744da - [analyzer] Fix ValistChecker false-positive involving symbolic pointers

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2022-04-26T08:49:05+02:00
New Revision: be744da01f9da0675ba5a3958c03bcd1fdc8ad60

URL: https://github.com/llvm/llvm-project/commit/be744da01f9da0675ba5a3958c03bcd1fdc8ad60
DIFF: https://github.com/llvm/llvm-project/commit/be744da01f9da0675ba5a3958c03bcd1fdc8ad60.diff

LOG: [analyzer] Fix ValistChecker false-positive involving symbolic pointers

In the following example:

  int va_list_get_int(va_list *va) {
    return va_arg(*va, int); // FP
  }

The `*va` expression will be something like `Element{SymRegion{va}, 0, va_list}`.
We use `ElementRegions` for representing the result of the dereference.
In this case, the `IsSymbolic` was set to `false` in the
`getVAListAsRegion()`.

Hence, before checking if the memregion is a SymRegion, we should take
the base of that region.

Analogously to the previous example, one can craft other cases:

  struct MyVaList {
    va_list l;
  };
  int va_list_get_int(struct MyVaList va) {
    return va_arg(va.l, int); // FP
  }

But it would also work if the `va_list` would be in the base or derived
part of a class. `ObjCIvarRegions` are likely also susceptible.
I'm not explicitly demonstrating these cases.

PS: Check the `MemRegion::getBaseRegion()` definition.

Fixes #55009

Reviewed By: xazax.hun

Differential Revision: https://reviews.llvm.org/D124239

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
    clang/test/Analysis/valist-uninitialized-no-undef.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
index 2ce95a9d47685..fbefd5f9ffdc9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
@@ -178,7 +178,7 @@ const MemRegion *ValistChecker::getVAListAsRegion(SVal SV, const Expr *E,
     if (isa<ParmVarDecl>(DeclReg->getDecl()))
       Reg = C.getState()->getSVal(SV.castAs<Loc>()).getAsRegion();
   }
-  IsSymbolic = Reg && Reg->getAs<SymbolicRegion>();
+  IsSymbolic = Reg && Reg->getBaseRegion()->getAs<SymbolicRegion>();
   // Some VarRegion based VA lists reach here as ElementRegions.
   const auto *EReg = dyn_cast_or_null<ElementRegion>(Reg);
   return (EReg && VaListModelledAsArray) ? EReg->getSuperRegion() : Reg;

diff  --git a/clang/test/Analysis/valist-uninitialized-no-undef.c b/clang/test/Analysis/valist-uninitialized-no-undef.c
index 528ac86c14213..6d6542a6acf99 100644
--- a/clang/test/Analysis/valist-uninitialized-no-undef.c
+++ b/clang/test/Analysis/valist-uninitialized-no-undef.c
@@ -16,11 +16,20 @@ void call_inlined_uses_arg(int fst, ...) {
 
 void f6(va_list *fst, ...) {
   va_start(*fst, fst);
-  // 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 at -1{{va_arg() is called on an uninitialized va_list}}
+  (void)va_arg(*fst, int);
   va_end(*fst);
-} 
+}
+
+int va_list_get_int(va_list *va) {
+  return va_arg(*va, int); // no-warning
+}
+
+struct MyVaList {
+  va_list l;
+};
+int va_list_get_int2(struct MyVaList *va) {
+  return va_arg(va->l, int); // no-warning
+}
 
 void call_vprintf_bad(int isstring, ...) {
   va_list va;