NoQ added inline comments.

================
Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:191
+const ParmVarDecl *ParamRegion::getDecl() const {
+  const Decl *D = getStackFrame()->getDecl();
+
----------------
baloghadamsoftware wrote:
> NoQ wrote:
> > baloghadamsoftware wrote:
> > > NoQ wrote:
> > > > baloghadamsoftware wrote:
> > > > > NoQ wrote:
> > > > > > This doesn't work when the callee is unknown.
> > > > > Please give me an example where the callee is unknown. As I wrote, 
> > > > > originally I put a `getExpr()` method here as well and `getType()` 
> > > > > fell back to it if it could not find the `Decl()`. However, it was 
> > > > > never invoked on the whole test suite. (I put an `assert(false)` into 
> > > > > it, and did not get a crash.
> > > > > Please give me an example where the callee is unknown.
> > > > 
> > > > >>! In D79704#2034571, @NoQ wrote:
> > > > >>>! In D79704#2032947, @Szelethus wrote:
> > > > >> Could you give a specific code example? 
> > > > > 
> > > > > ```lang=c++
> > > > > struct S {
> > > > >   S() {
> > > > >     this; // What region does 'this' point to...
> > > > >   }
> > > > > };
> > > > > 
> > > > > void foo(void (*bar)(S)) {
> > > > >   bar(S()); // ...in this invocation?
> > > > > }
> > > > > ```
> > > OK, but it still does not crash the analyzer, even if I enable creation 
> > > of stack frames for all the callees, even for those without definition. 
> > > What else should I do to enforce the crash (null pointer dereference)? 
> > > Try to use `getParameterLocation()` in a unit test?
> > Yes. I demonstrated how you can get the region without a decl but you 
> > should turn this into a test that actually calls one of the problematic 
> > functions. Like, `clang_analyzer_dump()` it or something.
> Of  course I tried `clang_analyzer_explain()`, but neither 
> `clang_analyzer_dump()` helps. I also tried a unit test now where I call 
> `getParameterLocation()` explicitly. It turns out that parameter regions are 
> never created for functions without `Decl` because of the first lines in 
> `CallEvent::getCalleeAnalysisDeclContext()`. This function //needs// the 
> `Decl` to retrieve the `AnalysisDeclContext` of the callee, which is needed 
> to retrieve its stack  frame by `getCalleeStackFrame()`. Without stack frame 
> we do not create `ParamRegion`. The other two functions creating 
> `ParamRegion` (`CallEvent::addParameterValuesToBindings` and the newly 
> created `MemRegionManager::getRegionForParam`) start from `ParmVarDecl` which 
> always belongs to a `Decl`. So we can safely assume that currently all 
> parameter regions have a `Decl`. Of course, this can be changed in the 
> future, but I must not include dead code in a patch that cannot even be 
> tested in the current phase. Even creation of callee stack frame for 
> functions without //definition// is not part of this patch, but of the 
> subsequent one.
> neither `clang_analyzer_dump()` helps

So, does it dump a parameter region? Because it obviously should.


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

https://reviews.llvm.org/D79704



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

Reply via email to