[PATCH] D71155: [analyzer] CERT: StrChecker: 30.c

[Csaba Dabis via Phabricator via cfe-commits]

Charusso marked 2 inline comments as done.
Charusso added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2109
+  if (const MemRegion *MR = SrcV.getAsRegion()) {
+    if (const auto *SR = MR->getBaseRegion()->getAs<SymbolicRegion>()) {
+      State = State->BindExpr(CE, LCtx, SrcV);
----------------
NoQ wrote:
> Why does it matter whether the region is symbolic or not?
Hm, I think you are right, we could omit that if-statement. I wanted to specify to reuse conjured symbols.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2110
+    if (const auto *SR = MR->getBaseRegion()->getAs<SymbolicRegion>()) {
+      State = State->BindExpr(CE, LCtx, SrcV);
+      C.addTransition(State);
----------------
NoQ wrote:
> Mmm, that's not a correct return value for these functions. These functions don't simply pass through their first argument.
Yes, but we need some index here. It requires a `NonLoc`, so I just randomly picked the first index, but I like the idea of an unknown index. Would we like to introduce `UnknownVal` for indices?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2122
+
+  SVal ConjuredV = SVB.getConjuredHeapSymbolVal(CE, LCtx, C.blockCount());
+  SVal ResultV = loc::MemRegionVal(SVB.getRegionManager().getElementRegion(
----------------
NoQ wrote:
> Why "heap"?
Well, a string which length is at least 16 characters long is going to be allocated on the heap. I have to conjure the string here to create its element.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71155/new/

https://reviews.llvm.org/D71155