NoQ added inline comments.

================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2549-2552
+      FunctionStr = Lexer::getSourceText(
+          CharSourceRange::getTokenRange(
+              {FD->getBeginLoc(), FD->getBody()->getBeginLoc()}),
+          C.getSourceManager(), C.getLangOpts());
----------------
Charusso wrote:
> NoQ wrote:
> > I'm slightly worried that it'll crash when `free()` is being called from 
> > within a body farm.
> > 
> > For now it probably cannot happen because none of the bodyfarmed functions 
> > can call `free()` directly, but i'd anyway rather add a check that the 
> > source locations we're taking are valid.
> Oh, I missed that, thanks! I wanted to check for everything, yes.
I think this is not fixed yet. I'm thinking of something like `if 
(!Body->getBeginLoc().isValid()) { ... }`.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2569
+          if (const RefState *RS = State->get<RegionState>(Sym)) {
+            State = State->remove<RegionState>(Sym);
+            State = State->set<RegionState>(Sym, RefState::getEscaped(RS));
----------------
Charusso wrote:
> NoQ wrote:
> > `remove` is unnecessary, we overwrite it anyway.
> I believe in so as well, just the official code base has this semantic. I 
> have rewritten that, see below in `checkPointerEscapeAux()`.
Yeah, and the official codebase is wrong :)


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2570
+
+  if (IsEscaped)
+    C.addTransition(State);
----------------
The check is unnecessary. `addTransition` automatically takes care of the "no 
changes have happened" case and throws away the transition.

Moreover, in the current patch you aren't really checking if any changes have 
happened: the symbol may already be escaped, so in this case the flag is set to 
true but the state remains the same.


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

https://reviews.llvm.org/D64680



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

Reply via email to