baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added inline comments.


================
Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:1044
 
+class ParamWithoutVarRegion : public TypedValueRegion {
+  friend class MemRegionManager;
----------------
NoQ wrote:
> baloghadamsoftware wrote:
> > baloghadamsoftware wrote:
> > > baloghadamsoftware wrote:
> > > > NoQ wrote:
> > > > > There should be only one way to express the parameter region. Let's 
> > > > > call this one simply `ParamRegion` or something like that, and 
> > > > > `assert(!isa<ParmVarDecl>(D))` in the constructor of `VarRegion`.
> > > > Do you mean that we should use `ParamRegion` in every case, thus also 
> > > > when we have the definitioan for the function? I wonder whether it 
> > > > breaks too many things.
> > > This will surely not work. The common handling of `ParamVarDecl` and 
> > > `VarDecl` is soo deeply rooted in the whole analyzer that separating them 
> > > means creation of a totally new analyzer engine from scratch.
> > More specifically: whenever a function is inlined, its parameters are used 
> > as variables via `DeclRefExpr`s. A `DeclRefExpr` refers to a `Decl` which 
> > is a `ParamVarDecl` but that has reference neither for the `CallExpr` 
> > (since it is not related to the call, but to the `FunctionDecl` or 
> > `ObjCMethodDecl`) nore for its `Index` in the call. The former is the real 
> > problem that cannot be solved even on theoretical level: a function which 
> > is inlined cannot depend on the different `CallExpr`s where it is called. 
> > Even worse, if the function is analyzed top-level it has not `CallExpr` at 
> > all so using `ParamRegion` for its parameters is completely impossible.
> > A `DeclRefExpr` refers to a `Decl` which is a `ParamVarDecl` but that has 
> > reference neither for the `CallExpr` (since it is not related to the call, 
> > but to the `FunctionDecl` or `ObjCMethodDecl`)
> 
> The call site is available as part of the current location context.
> 
> > nore for its Index in the call
> 
> It's available as part of `ParmVarDecl`.
> 
> > The former is the real problem that cannot be solved even on theoretical 
> > level: a function which is inlined cannot depend on the different 
> > `CallExpr`s where it is called
> 
> It already depends on the `CallExpr`. `ParmVarDecl`-based `VarRegion`s are 
> different for different call sites even if `ParmVarDecl` is the same; 
> moreover, they reside in non-overlapping memory spaces.
> 
> > Even worse, if the function is analyzed top-level it has not `CallExpr` at 
> > all so using `ParamRegion` for its parameters is completely impossible.
> 
> Ok, let's make an exception for this case and either use the old `VarRegion` 
> (given that there's no redecl confusion in this case) or allow the `CallExpr` 
> to be null (it's still trivially easy to extract all the necessary 
> information).
> 
> > The common handling of `ParamVarDecl` and `VarDecl` is soo deeply rooted in 
> > the whole analyzer that separating them means creation of a totally new 
> > analyzer engine from scratch.
> 
> I don't see any indication of that yet.
> or allow the `CallExpr` to be `null`

How do we calculate the type then? Or should we store it explicitly?


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

https://reviews.llvm.org/D77229



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

Reply via email to