Charusso marked 2 inline comments as done.
Charusso added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/DynamicSize.cpp:85
+ if (CI->getValue().isUnsigned())
+ Size = SVB.makeIntVal(CI->getValue(), /*IsUnsigned=*/false);
+
----------------
That one is interesting. Some of the checkers / SValBuilder(?) generate
unsigned integers which should not happen, I believe. May we need a FIXME and
an assertion about signedness. What do you think?
================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:698
+ DefinedOrUnknownSVal Size = UnknownVal();
+ if (const Expr *SizeExpr = CNE->getArraySize().getValueOr(nullptr))
+ Size = State->getSVal(SizeExpr, LCtx).castAs<DefinedOrUnknownSVal>();
----------------
NoQ wrote:
> Same. I guess we should add a test for both cases, given that nothing failed
> when we screwed up the extent.
Well, it was equally wrong everywhere, so that it works... I have noticed it
like 5 months ago, but I was lazy to fix.
================
Comment at: clang/test/Analysis/explain-svals.cpp:52
// Sic! What gets computed is the extent of the element-region.
- clang_analyzer_explain(clang_analyzer_getExtent(x)); //
expected-warning-re{{{{^signed 32-bit integer '4'$}}}}
+ clang_analyzer_explain(clang_analyzer_getExtent(x)); //
expected-warning-re{{{{^\(argument 'ext'\) \* 4$}}}}
delete[] x;
----------------
Yea, that is the fact: The size is the size of the parameter, which is unknown.
================
Comment at: clang/test/Analysis/memory-model.cpp:108
+ free(c);
+}
----------------
Here I wanted to put more, but I am not that cool with other MemRegions. Some
wise words about the followups of this test file:
> Some day
- The 11 years old comment in ExprEngine.cpp:
https://github.com/llvm/llvm-project/blame/master/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp#L237
================
Comment at: clang/test/Analysis/misc-ps-region-store.m:1190
+ tmp2[x] = am; // expected-warning \
+ {{Access out-of-bound array element (buffer overflow)}}
}
----------------
That is the single regression which I do not get.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69726/new/
https://reviews.llvm.org/D69726
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
