steakhal created this revision. steakhal added reviewers: NoQ, martong, xazax.hun, Szelethus, ASDenysPetrov. Herald added subscribers: manas, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. Herald added a project: All. steakhal requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.
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 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D124239 Files: clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp clang/test/Analysis/valist-uninitialized-no-undef.c Index: clang/test/Analysis/valist-uninitialized-no-undef.c =================================================================== --- clang/test/Analysis/valist-uninitialized-no-undef.c +++ clang/test/Analysis/valist-uninitialized-no-undef.c @@ -16,11 +16,20 @@ 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@-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_int(struct MyVaList va) { + return va_arg(va.l, int); // no-warning +} void call_vprintf_bad(int isstring, ...) { va_list va; Index: clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp @@ -178,7 +178,7 @@ 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;
Index: clang/test/Analysis/valist-uninitialized-no-undef.c =================================================================== --- clang/test/Analysis/valist-uninitialized-no-undef.c +++ clang/test/Analysis/valist-uninitialized-no-undef.c @@ -16,11 +16,20 @@ 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@-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_int(struct MyVaList va) { + return va_arg(va.l, int); // no-warning +} void call_vprintf_bad(int isstring, ...) { va_list va; Index: clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp @@ -178,7 +178,7 @@ 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;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits