NoQ 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); ---------------- Charusso wrote: > 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`? > So because most likely after the malloc() the size symbol dies...? After the `malloc()` is consumed, the size //expression// dies and gets cleaned up from the //Environment//. The symbol will only die if the value wasn't put into the //Store// in the process of modeling the statement that consumes the `malloc()` expression (such as an assignment). But `trackExpressionValue()` can only track live (active) expressions. 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
