[clang] [clang][analyzer] Improve 'errno' handling in StdLibraryFunctionsChecker. (PR #71392)
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 9 07:59:25 PST 2023
https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/71392
>From e92bf72fc80bb6823996cb71cb811d238b304aaa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Thu, 2 Nov 2023 18:12:32 +0100
Subject: [PATCH 1/3] [clang][analyzer] Improve 'errno' handling in
StdLibraryFunctionsChecker.
The checker now displays one combined note tag for errno-related and
"case"-related notes. Previous functions in the errno-modeling part
that were used for construction of note tags are removed. The note tag
added by StdLibraryFunctionsChecker contains the code to display the
note tag for 'errno' (this was done previously by these removed
functions).
---
.../StaticAnalyzer/Checkers/ErrnoModeling.cpp | 12 ---
.../StaticAnalyzer/Checkers/ErrnoModeling.h | 16 +--
.../Checkers/StdLibraryFunctionsChecker.cpp | 101 ++++++++++--------
.../errno-stdlibraryfunctions-notes.c | 4 +-
.../std-c-library-functions-path-notes.c | 9 ++
clang/test/Analysis/stream-errno-note.c | 41 +++++--
6 files changed, 101 insertions(+), 82 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
index be2fa91b994a26d..1b34ea0e056e563 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
@@ -312,18 +312,6 @@ ProgramStateRef setErrnoStdMustBeChecked(ProgramStateRef State,
return setErrnoState(State, MustBeChecked);
}
-const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn) {
- return getErrnoNoteTag(
- C, llvm::formatv(
- "'errno' may be undefined after successful call to '{0}'", Fn));
-}
-
-const NoteTag *getNoteTagForStdMustBeChecked(CheckerContext &C,
- llvm::StringRef Fn) {
- return getErrnoNoteTag(
- C, llvm::formatv("'{0}' indicates failure only by setting 'errno'", Fn));
-}
-
} // namespace errno_modeling
} // namespace ento
} // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
index 0707fd16d6e60a2..6b53572fe5e2d93 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
@@ -84,8 +84,7 @@ const NoteTag *getErrnoNoteTag(CheckerContext &C, const std::string &Message);
/// Set errno state for the common case when a standard function is successful.
/// Set \c ErrnoCheckState to \c MustNotBeChecked (the \c errno value is not
-/// affected). At the state transition a note tag created by
-/// \c getNoteTagForStdSuccess can be used.
+/// affected).
ProgramStateRef setErrnoForStdSuccess(ProgramStateRef State, CheckerContext &C);
/// Set errno state for the common case when a standard function fails.
@@ -100,23 +99,10 @@ ProgramStateRef setErrnoForStdFailure(ProgramStateRef State, CheckerContext &C,
/// Set errno state for the common case when a standard function indicates
/// failure only by \c errno. Sets \c ErrnoCheckState to \c MustBeChecked, and
/// invalidates the errno region (clear of previous value).
-/// At the state transition a note tag created by
-/// \c getNoteTagForStdMustBeChecked can be used.
/// \arg \c InvalE Expression that causes invalidation of \c errno.
ProgramStateRef setErrnoStdMustBeChecked(ProgramStateRef State,
CheckerContext &C, const Expr *InvalE);
-/// Generate the note tag that can be applied at the state generated by
-/// \c setErrnoForStdSuccess .
-/// \arg \c Fn Name of the (standard) function that is modeled.
-const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn);
-
-/// Generate the note tag that can be applied at the state generated by
-/// \c setErrnoStdMustBeChecked .
-/// \arg \c Fn Name of the (standard) function that is modeled.
-const NoteTag *getNoteTagForStdMustBeChecked(CheckerContext &C,
- llvm::StringRef Fn);
-
} // namespace errno_modeling
} // namespace ento
} // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 13bb9cef5e490ed..035314c6f7751d5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -533,13 +533,11 @@ class StdLibraryFunctionsChecker
virtual ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
const Summary &Summary,
CheckerContext &C) const = 0;
- /// Get a NoteTag about the changes made to 'errno' and the possible bug.
- /// It may return \c nullptr (if no bug report from \c ErrnoChecker is
- /// expected).
- virtual const NoteTag *describe(CheckerContext &C,
- StringRef FunctionName) const {
- return nullptr;
- }
+ /// Get a description about what happens with 'errno' here and how it causes
+ /// a later bug report created by ErrnoChecker.
+ /// Empty return value means that 'errno' related bug may not happen from
+ /// the current analyzed function.
+ virtual const std::string describe(CheckerContext &C) const { return ""; }
virtual ~ErrnoConstraintBase() {}
@@ -596,7 +594,8 @@ class StdLibraryFunctionsChecker
};
/// Set errno constraint at success cases of standard functions.
- /// Success case: 'errno' is not allowed to be used.
+ /// Success case: 'errno' is not allowed to be used because the value is
+ /// undefined after successful call.
/// \c ErrnoChecker can emit bug report after such a function call if errno
/// is used.
class SuccessErrnoConstraint : public ErrnoConstraintBase {
@@ -607,12 +606,15 @@ class StdLibraryFunctionsChecker
return errno_modeling::setErrnoForStdSuccess(State, C);
}
- const NoteTag *describe(CheckerContext &C,
- StringRef FunctionName) const override {
- return errno_modeling::getNoteTagForStdSuccess(C, FunctionName);
+ const std::string describe(CheckerContext &C) const {
+ return "'errno' becomes undefined after the call";
}
};
+ /// Set errno constraint at functions that indicate failure only with 'errno'.
+ /// In this case 'errno' is required to be observed.
+ /// \c ErrnoChecker can emit bug report after such a function call if errno
+ /// is overwritten without a read before.
class ErrnoMustBeCheckedConstraint : public ErrnoConstraintBase {
public:
ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
@@ -622,9 +624,8 @@ class StdLibraryFunctionsChecker
Call.getOriginExpr());
}
- const NoteTag *describe(CheckerContext &C,
- StringRef FunctionName) const override {
- return errno_modeling::getNoteTagForStdMustBeChecked(C, FunctionName);
+ const std::string describe(CheckerContext &C) const {
+ return "This function indicates failure only by setting 'errno'";
}
};
@@ -1392,15 +1393,19 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
// Still add these note tags, the other checker should add only its
// specialized note tags. These general note tags are handled always by
// StdLibraryFunctionsChecker.
+
ExplodedNode *Pred = Node;
- if (!Case.getNote().empty()) {
- const SVal RV = Call.getReturnValue();
- // If there is a description for this execution branch (summary case),
- // use it as a note tag.
- std::string Note =
- llvm::formatv(Case.getNote().str().c_str(),
- cast<NamedDecl>(Call.getDecl())->getDeclName());
- if (Summary.getInvalidationKd() == EvalCallAsPure) {
+
+ std::string ErrnoNote = Case.getErrnoConstraint().describe(C);
+ std::string Note =
+ llvm::formatv(Case.getNote().str().c_str(),
+ cast<NamedDecl>(Call.getDecl())->getDeclName());
+ const SVal RV = Call.getReturnValue();
+
+ if (Summary.getInvalidationKd() == EvalCallAsPure) {
+ // Do not expect that errno is interesting (the "pure" functions do not
+ // affect it).
+ if (!Note.empty()) {
const NoteTag *Tag = C.getNoteTag(
[Node, Note, RV](PathSensitiveBugReport &BR) -> std::string {
// Try to omit the note if we know in advance which branch is
@@ -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;
// Add the transition if no note tag could be added.
if (Pred == Node && NewState != State)
@@ -2038,7 +2054,8 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
ErrnoMustNotBeChecked, GenericSuccessMsg)
.Case({ArgumentCondition(1U, WithinRange, SingleValue(0)),
ReturnValueCondition(WithinRange, SingleValue(0))},
- ErrnoMustNotBeChecked, GenericSuccessMsg)
+ ErrnoMustNotBeChecked,
+ "Assuming that argument 'size' to '{0}' is 0")
.ArgConstraint(NotNullBuffer(ArgNo(0), ArgNo(1), ArgNo(2)))
.ArgConstraint(NotNull(ArgNo(3)))
.ArgConstraint(BufferSize(/*Buffer=*/ArgNo(0), /*BufSize=*/ArgNo(1),
diff --git a/clang/test/Analysis/errno-stdlibraryfunctions-notes.c b/clang/test/Analysis/errno-stdlibraryfunctions-notes.c
index c3fac58c46b3763..41f54664cfb4f3d 100644
--- a/clang/test/Analysis/errno-stdlibraryfunctions-notes.c
+++ b/clang/test/Analysis/errno-stdlibraryfunctions-notes.c
@@ -15,7 +15,7 @@ void clang_analyzer_warnIfReached();
void test1() {
access("path", 0);
access("path", 0);
- // expected-note at -1{{'errno' may be undefined after successful call to 'access'}}
+ // expected-note at -1{{Assuming that 'access' is successful; 'errno' becomes undefined after the call}}
if (errno != 0) {
// expected-warning at -1{{An undefined value may be read from 'errno'}}
// expected-note at -2{{An undefined value may be read from 'errno'}}
@@ -39,7 +39,7 @@ void test2() {
void test3() {
if (access("path", 0) != -1) {
// Success path.
- // expected-note at -2{{'errno' may be undefined after successful call to 'access'}}
+ // expected-note at -2{{Assuming that 'access' is successful; 'errno' becomes undefined after the call}}
// expected-note at -3{{Taking true branch}}
if (errno != 0) {
// expected-warning at -1{{An undefined value may be read from 'errno'}}
diff --git a/clang/test/Analysis/std-c-library-functions-path-notes.c b/clang/test/Analysis/std-c-library-functions-path-notes.c
index d0957483c1391ad..4df00fe1e60646f 100644
--- a/clang/test/Analysis/std-c-library-functions-path-notes.c
+++ b/clang/test/Analysis/std-c-library-functions-path-notes.c
@@ -80,3 +80,12 @@ int test_fileno_arg_note(FILE *f1) {
// expected-note{{The 1st argument to 'dup' is < 0 but should be >= 0}} \
// expected-note{{Assuming that 'fileno' fails}}
}
+
+int test_readlink_bufsize_zero(char *Buf, size_t Bufsize) {
+ ssize_t Ret = readlink("path", Buf, Bufsize); // \
+ // expected-note{{Assuming that argument 'bufsize' is 0}} \
+ // expected-note{{'Ret' initialized here}}
+ return 1 / Ret; // \
+ // expected-warning{{Division by zero}} \
+ // expected-note{{Division by zero}}
+}
diff --git a/clang/test/Analysis/stream-errno-note.c b/clang/test/Analysis/stream-errno-note.c
index 32d9d4fd9689dfc..a9f3d68f4f6703d 100644
--- a/clang/test/Analysis/stream-errno-note.c
+++ b/clang/test/Analysis/stream-errno-note.c
@@ -10,7 +10,7 @@
void check_fopen(void) {
FILE *F = fopen("xxx", "r");
- // expected-note at -1{{'errno' may be undefined after successful call to 'fopen'}}
+ // expected-note at -1{{Assuming that 'fopen' is successful; 'errno' becomes undefined after the call}}
// expected-note at +2{{'F' is non-null}}
// expected-note at +1{{Taking false branch}}
if (!F)
@@ -22,7 +22,7 @@ void check_fopen(void) {
void check_tmpfile(void) {
FILE *F = tmpfile();
- // expected-note at -1{{'errno' may be undefined after successful call to 'tmpfile'}}
+ // expected-note at -1{{Assuming that 'tmpfile' is successful; 'errno' becomes undefined after the call}}
// expected-note at +2{{'F' is non-null}}
// expected-note at +1{{Taking false branch}}
if (!F)
@@ -39,7 +39,7 @@ void check_freopen(void) {
if (!F)
return;
F = freopen("xxx", "w", F);
- // expected-note at -1{{'errno' may be undefined after successful call to 'freopen'}}
+ // expected-note at -1{{Assuming that 'freopen' is successful; 'errno' becomes undefined after the call}}
// expected-note at +2{{'F' is non-null}}
// expected-note at +1{{Taking false branch}}
if (!F)
@@ -56,7 +56,7 @@ void check_fclose(void) {
if (!F)
return;
(void)fclose(F);
- // expected-note at -1{{'errno' may be undefined after successful call to 'fclose'}}
+ // expected-note at -1{{Assuming that 'fclose' is successful; 'errno' becomes undefined after the call}}
if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
// expected-note at -1{{An undefined value may be read from 'errno'}}
}
@@ -69,7 +69,7 @@ void check_fread(void) {
if (!F)
return;
(void)fread(Buf, 1, 10, F);
- // expected-note at -1{{'errno' may be undefined after successful call to 'fread'}}
+ // expected-note at -1{{Assuming that 'fread' is successful; 'errno' becomes undefined after the call}}
if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
// expected-note at -1{{An undefined value may be read from 'errno'}}
(void)fclose(F);
@@ -83,7 +83,7 @@ void check_fread_size0(void) {
if (!F)
return;
fread(Buf, 0, 1, F);
- // expected-note at -1{{'errno' may be undefined after successful call to 'fread'}}
+ // expected-note at -1{{Assuming that argument 'size' to 'fread' is 0; 'errno' becomes undefined after the call}}
if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
// expected-note at -1{{An undefined value may be read from 'errno'}}
}
@@ -96,7 +96,7 @@ void check_fread_nmemb0(void) {
if (!F)
return;
fread(Buf, 1, 0, F);
- // expected-note at -1{{'errno' may be undefined after successful call to 'fread'}}
+ // expected-note at -1{{Assuming that 'fread' is successful; 'errno' becomes undefined after the call}}
if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
// expected-note at -1{{An undefined value may be read from 'errno'}}
}
@@ -109,7 +109,7 @@ void check_fwrite(void) {
if (!F)
return;
int R = fwrite(Buf, 1, 10, F);
- // expected-note at -1{{'errno' may be undefined after successful call to 'fwrite'}}
+ // expected-note at -1{{Assuming that 'fwrite' is successful; 'errno' becomes undefined after the call}}
if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
// expected-note at -1{{An undefined value may be read from 'errno'}}
(void)fclose(F);
@@ -122,7 +122,7 @@ void check_fseek(void) {
if (!F)
return;
(void)fseek(F, 11, SEEK_SET);
- // expected-note at -1{{'errno' may be undefined after successful call to 'fseek'}}
+ // expected-note at -1{{Assuming that 'fseek' is successful; 'errno' becomes undefined after the call}}
if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
// expected-note at -1{{An undefined value may be read from 'errno'}}
(void)fclose(F);
@@ -135,7 +135,7 @@ void check_rewind_errnocheck(void) {
if (!F)
return;
errno = 0;
- rewind(F); // expected-note{{'rewind' indicates failure only by setting 'errno'}}
+ rewind(F); // expected-note{{This function indicates failure only by setting 'errno'}}
fclose(F); // expected-warning{{Value of 'errno' was not checked and may be overwritten by function 'fclose' [alpha.unix.Errno]}}
// expected-note at -1{{Value of 'errno' was not checked and may be overwritten by function 'fclose'}}
}
@@ -147,8 +147,27 @@ void check_fileno(void) {
if (!F)
return;
fileno(F);
- // expected-note at -1{{'errno' may be undefined after successful call to 'fileno'}}
+ // expected-note at -1{{Assuming that 'fileno' is successful; 'errno' becomes undefined after the call}}
if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
// expected-note at -1{{An undefined value may be read from 'errno'}}
(void)fclose(F);
}
+
+void check_fwrite_zeroarg(size_t Siz) {
+ char Buf[] = "0123456789";
+ FILE *F = tmpfile();
+ // expected-note at +2{{'F' is non-null}}
+ // expected-note at +1{{Taking false branch}}
+ if (!F)
+ return;
+ errno = 0;
+ int R = fwrite(Buf, Siz, 1, F);
+ // expected-note at -1{{Assuming that argument 'size' to 'fwrite' is 0; 'errno' becomes undefined after the call}}
+ // expected-note at +2{{'R' is <= 0}}
+ // expected-note at +1{{Taking true branch}}
+ if (R <= 0) {
+ if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+ // expected-note at -1{{An undefined value may be read from 'errno'}}
+ }
+ (void)fclose(F);
+}
>From 76a0e1e3d9fc194c26a5d0fafcda237ed07cb19c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Wed, 8 Nov 2023 16:12:48 +0100
Subject: [PATCH 2/3] remove unrelated test
---
clang/test/Analysis/std-c-library-functions-path-notes.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/clang/test/Analysis/std-c-library-functions-path-notes.c b/clang/test/Analysis/std-c-library-functions-path-notes.c
index 4df00fe1e60646f..d0957483c1391ad 100644
--- a/clang/test/Analysis/std-c-library-functions-path-notes.c
+++ b/clang/test/Analysis/std-c-library-functions-path-notes.c
@@ -80,12 +80,3 @@ int test_fileno_arg_note(FILE *f1) {
// expected-note{{The 1st argument to 'dup' is < 0 but should be >= 0}} \
// expected-note{{Assuming that 'fileno' fails}}
}
-
-int test_readlink_bufsize_zero(char *Buf, size_t Bufsize) {
- ssize_t Ret = readlink("path", Buf, Bufsize); // \
- // expected-note{{Assuming that argument 'bufsize' is 0}} \
- // expected-note{{'Ret' initialized here}}
- return 1 / Ret; // \
- // expected-warning{{Division by zero}} \
- // expected-note{{Division by zero}}
-}
>From 44bdb32cbebc5655d6ee923b5358c8343df55ed7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Thu, 9 Nov 2023 16:58:38 +0100
Subject: [PATCH 3/3] fixed review issues
---
.../Checkers/StdLibraryFunctionsChecker.cpp | 43 ++++++++++---------
clang/test/Analysis/stream-errno-note.c | 2 +-
2 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 035314c6f7751d5..51180cb16ddcc5e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -625,7 +625,7 @@ class StdLibraryFunctionsChecker
}
const std::string describe(CheckerContext &C) const {
- return "This function indicates failure only by setting 'errno'";
+ return "reading 'errno' is required to find out if the call has failed";
}
};
@@ -1395,19 +1395,26 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
// StdLibraryFunctionsChecker.
ExplodedNode *Pred = Node;
+ DeclarationName FunctionName =
+ cast<NamedDecl>(Call.getDecl())->getDeclName();
std::string ErrnoNote = Case.getErrnoConstraint().describe(C);
- std::string Note =
- llvm::formatv(Case.getNote().str().c_str(),
- cast<NamedDecl>(Call.getDecl())->getDeclName());
+ std::string CaseNote;
+ if (Case.getNote().empty()) {
+ if (!ErrnoNote.empty())
+ ErrnoNote =
+ llvm::formatv("After calling '{0}' {1}", FunctionName, ErrnoNote);
+ } else {
+ CaseNote = llvm::formatv(Case.getNote().str().c_str(), FunctionName);
+ }
const SVal RV = Call.getReturnValue();
if (Summary.getInvalidationKd() == EvalCallAsPure) {
// Do not expect that errno is interesting (the "pure" functions do not
// affect it).
- if (!Note.empty()) {
+ if (!CaseNote.empty()) {
const NoteTag *Tag = C.getNoteTag(
- [Node, Note, RV](PathSensitiveBugReport &BR) -> std::string {
+ [Node, CaseNote, RV](PathSensitiveBugReport &BR) -> std::string {
// Try to omit the note if we know in advance which branch is
// taken (this means, only one branch exists).
// This check is performed inside the lambda, after other
@@ -1419,15 +1426,16 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
// the successors). This is why this check is only used in the
// EvalCallAsPure case.
if (BR.isInteresting(RV) && Node->succ_size() > 1)
- return Note;
+ return CaseNote;
return "";
});
Pred = C.addTransition(NewState, Pred, Tag);
}
} else {
- if (!Note.empty() || !ErrnoNote.empty()) {
- const NoteTag *Tag = C.getNoteTag(
- [Note, ErrnoNote, RV](PathSensitiveBugReport &BR) -> std::string {
+ if (!CaseNote.empty() || !ErrnoNote.empty()) {
+ const NoteTag *Tag =
+ C.getNoteTag([CaseNote, 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
@@ -1440,12 +1448,12 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
BR.isInteresting(ErrnoLoc->getAsRegion());
if (ErrnoImportant) {
BR.markNotInteresting(ErrnoLoc->getAsRegion());
- if (Note.empty())
+ if (CaseNote.empty())
return ErrnoNote;
- return llvm::formatv("{0}; {1}", Note, ErrnoNote);
+ return llvm::formatv("{0}; {1}", CaseNote, ErrnoNote);
} else {
if (BR.isInteresting(RV))
- return Note;
+ return CaseNote;
}
return "";
});
@@ -1453,14 +1461,7 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
}
}
- // 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;
-
- // Add the transition if no note tag could be added.
+ // Add the transition if no note tag was added.
if (Pred == Node && NewState != State)
C.addTransition(NewState);
}
diff --git a/clang/test/Analysis/stream-errno-note.c b/clang/test/Analysis/stream-errno-note.c
index a9f3d68f4f6703d..d260eb2252f3965 100644
--- a/clang/test/Analysis/stream-errno-note.c
+++ b/clang/test/Analysis/stream-errno-note.c
@@ -135,7 +135,7 @@ void check_rewind_errnocheck(void) {
if (!F)
return;
errno = 0;
- rewind(F); // expected-note{{This function indicates failure only by setting 'errno'}}
+ rewind(F); // expected-note{{After calling 'rewind' reading 'errno' is required to find out if the call has failed}}
fclose(F); // expected-warning{{Value of 'errno' was not checked and may be overwritten by function 'fclose' [alpha.unix.Errno]}}
// expected-note at -1{{Value of 'errno' was not checked and may be overwritten by function 'fclose'}}
}
More information about the cfe-commits
mailing list