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;
----------------
baloghadamsoftware wrote:
> 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?
> I don't see any indication of that yet.
But I do. Even if I fix most of the crashes almost all the tests fail for
different reasons.
But I also have crashes: what to do with functions called through pointers?
These calls do not have direct callee, thus determining the type of the
parameter based on the call is difficult. If the function has no prototype,
then it is impossible. Maybe we should store the type explicitely indeed?
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
