Charusso added a comment.
Charusso added inline comments.

================
Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:124
+  if (const SymbolicRegion *SR = DestMR->getSymbolicBase())
+    if (const Expr *SizeExpr = getDynamicSizeExpr(C.getState(), SR))
+      return exprToStr(SizeExpr, C);
----------------
NoQ wrote:
> Charusso wrote:
> > Charusso wrote:
> > > NoQ wrote:
> > > > Again, you will have to highlight the allocation site with a note. 
> > > > Therefore you will have to write a bug visitor that traverses the size 
> > > > expression at some point (or, equivalently, a note tag when the size 
> > > > expression is evaluated). Therefore you don't need to store the 
> > > > expression in the program state.
> > > Yes, you have pointed out the necessary visitor, but it needs more 
> > > thinking.
> > > 
> > > I have a memory region which could be any kind of "memory block region" 
> > > therefore I have no idea where is the size expression. We are supporting 
> > > ~20 different allocations, which is nothing compared to the wild with the 
> > > not so uncommon 5+ parameter allocators. Therefore I still do not want to 
> > > reverse engineer a small MallocChecker + ExprEngine + 
> > > BuiltinFunctionChecker inside my checker. They provide the necessary 
> > > `DynamicSizeInfo` easily, which could be used in at least 4 checkers at 
> > > the moment (which I have pointed out earlier in D68725).
> > > 
> > > If I have the size expression in the dynamic size map, and I can clearly 
> > > point out the destination buffer, it is a lot more simplified to traverse 
> > > the graph where the buffer and its size comes from.
> > Well, you really do not want to store `SizeExpr` of `malloc(SizeExpr)` and 
> > you are right I will have to traverse from it to see whether the `SizeExpr` 
> > is ambiguous or not, where it comes from.
> > 
> > I want to rely on the `trackExpressionValue()` as the `SizeExpr` is 
> > available by `getDynamicSizeExpr()`, so it is one or two lines of code.
> > 
> > Would you create your own switch-case to see where is the size expression 
> > goes in the allocation and use `trackExpressionValue()` on it? So that you 
> > do not store information in the global state which results in better 
> > run-time / less memory.
> > 
> > At first I really wanted to model `malloc()` and `realloc()` and stuff, 
> > then I realized the `MallocChecker` provides every information I need. 
> > Would it be a better idea to create my own tiny `MallocChecker` inside my 
> > checker which does nothing but marks the size expression being interesting 
> > with `NoteTags`?
> > 
> > Also I am thinking of a switch-case on the `DefinedOrUnknownSVal Size` 
> > which somewhere has an expression inside it which I could 
> > `trackExpressionValue()` on.
> > 
> > Basically we are missing the rules what to use and I have picked the 
> > easiest solution. Could you share please which would be the right direction 
> > for such a simple task?
> > I want to rely on the `trackExpressionValue()` as the `SizeExpr` is 
> > available by `getDynamicSizeExpr()`, so it is one or two lines of code.
> 
> This won't work. `trackExpressionValue()` can only track an active expression 
> (that has, or at least should have, a value in the bug-node's environment). 
> You'll have to make a visitor or a note tag.
> 
> You can either make your own visitor (which will detect the node in which the 
> extent information becomes present), or convert `MallocChecker` to use note 
> tags and then inter-operate with those tags (though the interestingness map - 
> "i mark the symbol as interesting so i'm interested in highlighting the 
> allocation site" - or a similar mechanism). The second approach is more work 
> because no such interoperation has ever been implemented yet, but it should 
> be pretty rewarding for the future.
> This won't work. trackExpressionValue() can only track an active expression 
> (that has, or at least should have, a value in the bug-node's environment). 
> You'll have to make a visitor or a note tag.
So because most likely after the `malloc()` the `size` symbol dies, the 
`trackExpressionValue()` cannot track dead symbols? Because we could make the 
`size` dying base on the `buffer`, we have some dependency logic for that. It 
also represents the truth, the size is part of that memory block's region. 
After that we could track the expression of the `size`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D69813



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to