NoQ added a comment.
In D77229#1995079 <https://reviews.llvm.org/D77229#1995079>,
@baloghadamsoftware wrote:
> My first attempt for a new special kind of region for parameters of functions
> without definitions. Currently it causes more failed tests and assertions
> than ignoring the problem of different `Decl`s. I will investigate these
> problems tomorrow, but this path seems difficult.
Thank you!! I recommend a separate patch for this :) Yes it's a bit intrusive
because you'll need to add support for the new region in the `RegionStore` but
there's nothing extraordinary here as most of the time it'll be treated the
same way as the old parameter region.
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:438-442
+ Optional<SVal> getReturnValueUnderConstruction(unsigned BlockCount) const;
+
+ Optional<SVal> getArgObject(unsigned Index, unsigned BlockCount) const;
+
+ Optional<SVal> getReturnObject(unsigned BlockCount) const;
----------------
I believe these functions don't need to be optional. There's always a parameter
location and a location to return to (even if the latter is incorrect due to
unimplemented construction context, it'll still be some dummy temporary region
that you can use).
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:1044
+class ParamWithoutVarRegion : public TypedValueRegion {
+ friend class MemRegionManager;
----------------
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`.
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:1066
+ QualType getValueType() const override {
+ return CallE->getType();
+ }
----------------
This should be the type of the parameter, not the return type of the call.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:235
void IteratorModeling::checkBind(SVal Loc, SVal Val, const Stmt *S,
CheckerContext &C) const {
----------------
Do you still need `checkBind` at all?
================
Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:278-279
+
+const ConstructionContext
+*CallEvent::getConstructionContext(unsigned BlockCount) const {
+ const CFGBlock *Block;
----------------
This function obviously does not depend on `BlockCount`. You might as well
always use `0` as `BlockCount`.
The ideal solution, of course, is for `CallEvent` to keep a `CFGElementRef`
from the start.
================
Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:559
+ } else {
+ os << "ParamWithoutVarRegion";
+ }
----------------
We could still dump `CallE->getID()` and `Index`.
================
Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:1208
+ return getSubRegion<ParamWithoutVarRegion>(CE, Index,
+ getStackLocalsRegion(STC));
+}
----------------
StackArguments.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77229/new/
https://reviews.llvm.org/D77229
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
