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