================ @@ -1418,31 +1423,42 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call, return ""; }); Pred = C.addTransition(NewState, Pred, Tag); - } else { - const NoteTag *Tag = - C.getNoteTag([Note, RV](PathSensitiveBugReport &BR) -> std::string { - if (BR.isInteresting(RV)) - return Note; + } + } else { + if (!Note.empty() || !ErrnoNote.empty()) { + const NoteTag *Tag = C.getNoteTag( + [Note, ErrnoNote, RV](PathSensitiveBugReport &BR) -> std::string { + // If 'errno' is interesting, show the user a note about the case + // (what happened at the function call) and about how 'errno' + // causes the problem. ErrnoChecker sets the errno (but not RV) to + // interesting. + // If only the return value is interesting, show only the case + // note. + std::optional<Loc> ErrnoLoc = + errno_modeling::getErrnoLoc(BR.getErrorNode()->getState()); + bool ErrnoImportant = !ErrnoNote.empty() && ErrnoLoc && + BR.isInteresting(ErrnoLoc->getAsRegion()); + if (ErrnoImportant) { + BR.markNotInteresting(ErrnoLoc->getAsRegion()); + if (Note.empty()) + return ErrnoNote; + return llvm::formatv("{0}; {1}", Note, ErrnoNote); + } else { + if (BR.isInteresting(RV)) + return Note; + } return ""; }); Pred = C.addTransition(NewState, Pred, Tag); } - - // Pred may be: - // - a nullpointer, if we reach an already existing node (theoretically); - // - a sink, when NewState is posteriorly overconstrained. - // In these situations we cannot add the errno note tag. - if (!Pred || Pred->isSink()) - continue; } - // If we can get a note tag for the errno change, add this additionally to - // the previous. This note is only about value of 'errno' and is displayed - // if 'errno' is interesting. - if (const auto *D = dyn_cast<FunctionDecl>(Call.getDecl())) - if (const NoteTag *NT = - Case.getErrnoConstraint().describe(C, D->getNameAsString())) - Pred = C.addTransition(NewState, Pred, NT); + // Pred may be: + // - a nullpointer, if we reach an already existing node (theoretically); + // - a sink, when NewState is posteriorly overconstrained. + // In these situations we cannot add the errno note tag. + if (!Pred || Pred->isSink()) + continue; ---------------- DonatNagyE wrote:
```suggestion ``` This check can be removed; it was only needed to satisfy the preconditions of the `C.addTransition(NewState, Pred, NT);` call when the code applied the note tag for the errno change as a second separate transition starting in `Pred`. The next if only operates in the trivial case when `Pred` still holds the unchanged initial `Node` (which is clearly non-null and AFAIK non-sink) and it also doesn't try to build an explicit transition onto `Pred`. (Note that `Pred` is declared in the middle of the loop, it doesn't survive into the next iteration.) For the sake of ~~paranoia~~ safety, test the behavior of the code on some open source projects, but I don't see any reasons how this could introduce issues. https://github.com/llvm/llvm-project/pull/71392 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits