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 cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits