Endre =?utf-8?q?Fülöp?= <endre.fu...@sigmatechnology.se>, Endre =?utf-8?q?Fülöp?= <endre.fu...@sigmatechnology.se>, Endre =?utf-8?q?Fülöp?= <endre.fu...@sigmatechnology.se>, Endre =?utf-8?q?Fülöp?= <endre.fu...@sigmatechnology.se>, Endre =?utf-8?q?Fülöp?= <endre.fu...@sigmatechnology.se>, Endre =?utf-8?q?Fülöp?= <endre.fu...@sigmatechnology.se> Message-ID: In-Reply-To: <llvm/llvm-project/pull/67663/cl...@github.com>
================ @@ -94,23 +119,40 @@ REGISTER_MAP_WITH_PROGRAMSTATE(PreviousCallResultMap, const FunctionDecl *, void InvalidPtrChecker::EnvpInvalidatingCall(const CallEvent &Call, CheckerContext &C) const { StringRef FunctionName = Call.getCalleeIdentifier()->getName(); - ProgramStateRef State = C.getState(); - const MemRegion *SymbolicEnvPtrRegion = State->get<EnvPtrRegion>(); - if (!SymbolicEnvPtrRegion) - return; - - State = State->add<InvalidMemoryRegions>(SymbolicEnvPtrRegion); - const NoteTag *Note = - C.getNoteTag([SymbolicEnvPtrRegion, FunctionName]( - PathSensitiveBugReport &BR, llvm::raw_ostream &Out) { - if (!BR.isInteresting(SymbolicEnvPtrRegion)) - return; - Out << '\'' << FunctionName - << "' call may invalidate the environment parameter of 'main'"; - }); + auto PlaceInvalidationNote = [&C, FunctionName](ProgramStateRef State, + const MemRegion *Region, + StringRef Message, + ExplodedNode *Pred) { + State = State->add<InvalidMemoryRegions>(Region); + + // Make copy of string data for the time when notes are *actually* created. + const NoteTag *Note = + C.getNoteTag([Region, FunctionName = std::string{FunctionName}, + Message = std::string{Message}]( + PathSensitiveBugReport &BR, llvm::raw_ostream &Out) { + if (!BR.isInteresting(Region) || + &BR.getBugType() != InvalidPtrBugType) + return; + Out << '\'' << FunctionName << "' " << Message; + BR.markNotInteresting(Region); + }); + return C.addTransition(State, Pred, Note); + }; - C.addTransition(State, Note); + ProgramStateRef State = C.getState(); + ExplodedNode *CurrentChainEnd = C.getPredecessor(); + + if (const MemRegion *MainEnvPtr = State->get<MainEnvPtrRegion>()) + CurrentChainEnd = PlaceInvalidationNote( + State, MainEnvPtr, + "call may invalidate the environment parameter of 'main'", + CurrentChainEnd); + + for (const MemRegion *EnvPtr : State->get<GetenvEnvPtrRegions>()) + CurrentChainEnd = PlaceInvalidationNote( + State, EnvPtr, "call may invalidate the environment returned by getenv", + CurrentChainEnd); ---------------- DonatNagyE wrote: On the [old Phabricator review](https://reviews.llvm.org/D154603) of these modifications, @steakhal wrote that > I'd prefer if we wouldn't put N separate NoteTags, but rather iterate over > this set of regions inside the NoteTag. That way here you don't need the loop and play with Pred nodes. I agree with his suggestion: try to refactor this code and create a single note tag constructed from a "smart" lambda that captures both the `MainEnvPtr` and the array `State->get<GetenvEnvPtrRegions>()`, checks the bug type and if it corresponds to this checker, then checks each of the captured `MemRegion`s for interestingness and produces the corresponding message if it finds an interesting one. In the common case there will be just one interesting `MemRegion`, but think a bit about corner cases where multiple invalidated regions are referenced at the same time and handle them somehow if you think that they can appear. https://github.com/llvm/llvm-project/pull/67663 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits