[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -1191,6 +1199,84 @@ void StreamChecker::evalSetFeofFerror(const
FnDescription *Desc,
C.addTransition(State);
}
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ std::optional Stream = StreamVal.getAs();
+ SymbolRef StreamSym = StreamVal.getAsSymbol();
+ if (!Stream || !StreamSym)
+return;
+
+ ProgramStateRef StateNotNull, StateNull;
+ std::tie(StateNotNull, StateNull) =
+ C.getConstraintManager().assumeDual(State, *Stream);
+ if (StateNotNull)
+if (State = ensureStreamOpened(StreamVal, C, StateNotNull))
+ C.addTransition(State);
+ if (StateNull) {
+const StreamState *SS = StateNull->get(StreamSym);
+if (!SS || !SS->isOpenFailed()) {
+ StateNull = StateNull->set(StreamSym,
+StreamState::getOpenFailed(Desc));
+ if (StateNull)
+C.addTransition(StateNull);
+}
+ }
+}
+
+void StreamChecker::evalFflush(const FnDescription *Desc, const CallEvent
&Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+
+ // We will check the result even if the input is `NULL`,
+ // but do nothing if the input state is unknown.
+ SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+ const StreamState *SS;
+ if (StreamSym) {
+if (!(SS = State->get(StreamSym)))
+ return;
+assert((SS->isOpened() || SS->isOpenFailed()) &&
+ "Stream is expected to opened or open-failed");
+ }
+
+ const CallExpr *CE = dyn_cast_or_null(Call.getOriginExpr());
+ if (!CE)
+return;
+
+ // `fflush` returns EOF on failure, otherwise returns 0.
+ ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+ C.addTransition(StateFailed);
+
+ // Clear error states if `fflush` returns 0, but retain their EOF flags.
+ ProgramStateRef StateNotFailed = bindInt(0, State, C, CE);
+ auto ClearError = [&StateNotFailed, Desc](SymbolRef Sym,
+const StreamState *SS) {
+if (SS->ErrorState & ErrorFError) {
+ StreamErrorState NewES =
+ (SS->ErrorState & ErrorFEof) ? ErrorFEof : ErrorNone;
+ StreamState NewSS = StreamState::getOpened(Desc, NewES, false);
+ StateNotFailed = StateNotFailed->set(Sym, NewSS);
+}
+ };
+
+ if (SS->isOpened()) {
alejandro-alvarez-sonarsource wrote:
Unless I am mistaken, `SS` can be uninitialized here. If `StreamSym` is `NULL`,
SS does not get initialized.
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -1191,6 +1199,84 @@ void StreamChecker::evalSetFeofFerror(const
FnDescription *Desc,
C.addTransition(State);
}
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ std::optional Stream = StreamVal.getAs();
+ SymbolRef StreamSym = StreamVal.getAsSymbol();
+ if (!Stream || !StreamSym)
+return;
+
+ ProgramStateRef StateNotNull, StateNull;
+ std::tie(StateNotNull, StateNull) =
+ C.getConstraintManager().assumeDual(State, *Stream);
+ if (StateNotNull)
+if (State = ensureStreamOpened(StreamVal, C, StateNotNull))
+ C.addTransition(State);
+ if (StateNull) {
alejandro-alvarez-sonarsource wrote:
Due to this transition, these two test cases will behave differently:
```cpp
void test_fflush_2(FILE *F1) {
fflush(F1);
// Due to fflush, the analyzer follows a path where F1 is NULL, and another
where it isn't.
// Raises a "Stream pointer might be NULL" on the next line
if (fwrite("1", 1, 1, F1) == 0)
return;
fclose(F1);
}
void test_fflush_3(FILE *F1) {
// no fflush, the warning does not raise
if (fwrite("1", 1, 1, F1) == 0)
return;
fclose(F1);
}
```
I feel this could be noisy. What do you think about adding the StateNull
transition if, and only if, FILE* can be `NULL`?
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
alejandro-alvarez-sonarsource wrote: @balazske would you agree with my proposal of keeping this logic in `UnixAPIChecker`? I am also happy with adding more NULL checks to `StreamChecker`, but I can understand your concerns about overreaching its scope. https://github.com/llvm/llvm-project/pull/83027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83027
From a061464b75ac02c21e5d74fc4dff8d8afdbba66b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 20 Mar 2024 10:49:08 +0100
Subject: [PATCH 01/18] [NFC] UnixAPIMisuseChecker inherits from
Checker
---
.../Checkers/UnixAPIChecker.cpp | 60 +--
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index 19f1ca2dc824c9..599e5e6cedc606 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -17,6 +17,7 @@
#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "llvm/ADT/STLExtras.h"
@@ -41,8 +42,7 @@ enum class OpenVariant {
namespace {
class UnixAPIMisuseChecker
-: public Checker,
- check::ASTDecl> {
+: public Checker> {
const BugType BT_open{this, "Improper use of 'open'", categories::UnixAPI};
const BugType BT_pthreadOnce{this, "Improper use of 'pthread_once'",
categories::UnixAPI};
@@ -52,14 +52,14 @@ class UnixAPIMisuseChecker
void checkASTDecl(const TranslationUnitDecl *TU, AnalysisManager &Mgr,
BugReporter &BR) const;
- void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
+ void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
- void CheckOpen(CheckerContext &C, const CallExpr *CE) const;
- void CheckOpenAt(CheckerContext &C, const CallExpr *CE) const;
- void CheckPthreadOnce(CheckerContext &C, const CallExpr *CE) const;
+ void CheckOpen(CheckerContext &C, const CallEvent &Call) const;
+ void CheckOpenAt(CheckerContext &C, const CallEvent &Call) const;
+ void CheckPthreadOnce(CheckerContext &C, const CallEvent &Call) const;
- void CheckOpenVariant(CheckerContext &C,
-const CallExpr *CE, OpenVariant Variant) const;
+ void CheckOpenVariant(CheckerContext &C, const CallEvent &Call,
+OpenVariant Variant) const;
void ReportOpenBug(CheckerContext &C, ProgramStateRef State, const char *Msg,
SourceRange SR) const;
@@ -113,9 +113,9 @@ void UnixAPIMisuseChecker::checkASTDecl(const
TranslationUnitDecl *TU,
// "open" (man 2 open)
//===--===/
-void UnixAPIMisuseChecker::checkPreStmt(const CallExpr *CE,
+void UnixAPIMisuseChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
- const FunctionDecl *FD = C.getCalleeDecl(CE);
+ const FunctionDecl *FD = dyn_cast_if_present(Call.getDecl());
if (!FD || FD->getKind() != Decl::Function)
return;
@@ -130,13 +130,13 @@ void UnixAPIMisuseChecker::checkPreStmt(const CallExpr
*CE,
return;
if (FName == "open")
-CheckOpen(C, CE);
+CheckOpen(C, Call);
else if (FName == "openat")
-CheckOpenAt(C, CE);
+CheckOpenAt(C, Call);
else if (FName == "pthread_once")
-CheckPthreadOnce(C, CE);
+CheckPthreadOnce(C, Call);
}
void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext &C,
ProgramStateRef State,
@@ -152,17 +152,17 @@ void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext
&C,
}
void UnixAPIMisuseChecker::CheckOpen(CheckerContext &C,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::Open);
+ const CallEvent &Call) const {
+ CheckOpenVariant(C, Call, OpenVariant::Open);
}
void UnixAPIMisuseChecker::CheckOpenAt(CheckerContext &C,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::OpenAt);
+ const CallEvent &Call) const {
+ CheckOpenVariant(C, Call, OpenVariant::OpenAt);
}
void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
-const CallExpr *CE,
+const CallEvent &Call,
OpenVariant Variant) const {
// The index of the argument taking the flags open flags (O_RDONLY,
// O_WRONLY, O_CREAT, etc.),
@@ -191,11 +191,11 @@ void
UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
ProgramStateRef state = C.getState();
- if (CE->getNumArgs() < MinArgCount) {
+ if (Call.getNumArgs() < MinArgCount) {
// The frontend
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -1179,6 +1195,113 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent &Call,
C.addTransition(StateFailed);
}
+ProgramStateRef StreamChecker::ensureGetdelimBufferAndSizeCorrect(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+ static constexpr char SizeGreaterThanBufferSize[] =
+ "The buffer from the first argument is smaller than the size "
+ "specified by the second parameter";
+ static constexpr char SizeUndef[] =
+ "The buffer from the first argument is not NULL, but the size specified "
+ "by the second parameter is undefined.";
alejandro-alvarez-sonarsource wrote:
Fixed.
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -1179,6 +1195,113 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent &Call,
C.addTransition(StateFailed);
}
+ProgramStateRef StreamChecker::ensureGetdelimBufferAndSizeCorrect(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+ static constexpr char SizeGreaterThanBufferSize[] =
+ "The buffer from the first argument is smaller than the size "
+ "specified by the second parameter";
+ static constexpr char SizeUndef[] =
+ "The buffer from the first argument is not NULL, but the size specified "
+ "by the second parameter is undefined.";
+
+ auto EmitBugReport = [this, &C, SizePtrExpr,
+LinePtrPtrExpr](const ProgramStateRef &BugState,
alejandro-alvarez-sonarsource wrote:
Changed.
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
alejandro-alvarez-sonarsource wrote: Sorry for the force-push, but I have moved now the precondition checks to `UnixAPIChecker` after uplifting its API (now it uses `PreCall` instead of `PreStmt`). From my understanding, the previous implementation used a legacy way of handling them. The idea would be to rebase, keep the `[NFC]` separate, and squash everything that comes after. Since I had a merge from main in the middle, I found it easier to rebase. https://github.com/llvm/llvm-project/pull/83027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83027
From a061464b75ac02c21e5d74fc4dff8d8afdbba66b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 20 Mar 2024 10:49:08 +0100
Subject: [PATCH 01/19] [NFC] UnixAPIMisuseChecker inherits from
Checker
---
.../Checkers/UnixAPIChecker.cpp | 60 +--
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index 19f1ca2dc824c9..599e5e6cedc606 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -17,6 +17,7 @@
#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "llvm/ADT/STLExtras.h"
@@ -41,8 +42,7 @@ enum class OpenVariant {
namespace {
class UnixAPIMisuseChecker
-: public Checker,
- check::ASTDecl> {
+: public Checker> {
const BugType BT_open{this, "Improper use of 'open'", categories::UnixAPI};
const BugType BT_pthreadOnce{this, "Improper use of 'pthread_once'",
categories::UnixAPI};
@@ -52,14 +52,14 @@ class UnixAPIMisuseChecker
void checkASTDecl(const TranslationUnitDecl *TU, AnalysisManager &Mgr,
BugReporter &BR) const;
- void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
+ void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
- void CheckOpen(CheckerContext &C, const CallExpr *CE) const;
- void CheckOpenAt(CheckerContext &C, const CallExpr *CE) const;
- void CheckPthreadOnce(CheckerContext &C, const CallExpr *CE) const;
+ void CheckOpen(CheckerContext &C, const CallEvent &Call) const;
+ void CheckOpenAt(CheckerContext &C, const CallEvent &Call) const;
+ void CheckPthreadOnce(CheckerContext &C, const CallEvent &Call) const;
- void CheckOpenVariant(CheckerContext &C,
-const CallExpr *CE, OpenVariant Variant) const;
+ void CheckOpenVariant(CheckerContext &C, const CallEvent &Call,
+OpenVariant Variant) const;
void ReportOpenBug(CheckerContext &C, ProgramStateRef State, const char *Msg,
SourceRange SR) const;
@@ -113,9 +113,9 @@ void UnixAPIMisuseChecker::checkASTDecl(const
TranslationUnitDecl *TU,
// "open" (man 2 open)
//===--===/
-void UnixAPIMisuseChecker::checkPreStmt(const CallExpr *CE,
+void UnixAPIMisuseChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
- const FunctionDecl *FD = C.getCalleeDecl(CE);
+ const FunctionDecl *FD = dyn_cast_if_present(Call.getDecl());
if (!FD || FD->getKind() != Decl::Function)
return;
@@ -130,13 +130,13 @@ void UnixAPIMisuseChecker::checkPreStmt(const CallExpr
*CE,
return;
if (FName == "open")
-CheckOpen(C, CE);
+CheckOpen(C, Call);
else if (FName == "openat")
-CheckOpenAt(C, CE);
+CheckOpenAt(C, Call);
else if (FName == "pthread_once")
-CheckPthreadOnce(C, CE);
+CheckPthreadOnce(C, Call);
}
void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext &C,
ProgramStateRef State,
@@ -152,17 +152,17 @@ void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext
&C,
}
void UnixAPIMisuseChecker::CheckOpen(CheckerContext &C,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::Open);
+ const CallEvent &Call) const {
+ CheckOpenVariant(C, Call, OpenVariant::Open);
}
void UnixAPIMisuseChecker::CheckOpenAt(CheckerContext &C,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::OpenAt);
+ const CallEvent &Call) const {
+ CheckOpenVariant(C, Call, OpenVariant::OpenAt);
}
void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
-const CallExpr *CE,
+const CallEvent &Call,
OpenVariant Variant) const {
// The index of the argument taking the flags open flags (O_RDONLY,
// O_WRONLY, O_CREAT, etc.),
@@ -191,11 +191,11 @@ void
UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
ProgramStateRef state = C.getState();
- if (CE->getNumArgs() < MinArgCount) {
+ if (Call.getNumArgs() < MinArgCount) {
// The frontend
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -1204,6 +1204,20 @@ void StreamChecker::evalGetdelim(const FnDescription *Desc, State->BindExpr(E.CE, C.getLocationContext(), RetVal); StateNotFailed = E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal, E.getZeroVal(Call)); +// The buffer size `*n` must be enough to hold the whole line, and +// greater than the return value, since it has to account for '\0'. +auto SizePtrSval = Call.getArgSVal(1); alejandro-alvarez-sonarsource wrote: Changed. https://github.com/llvm/llvm-project/pull/83027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -1179,6 +1195,113 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent &Call,
C.addTransition(StateFailed);
}
+ProgramStateRef StreamChecker::ensureGetdelimBufferAndSizeCorrect(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+ static constexpr char SizeGreaterThanBufferSize[] =
alejandro-alvarez-sonarsource wrote:
Now it uses `llvm:StringLiteral`, but I think it (relatively) is. Most existing
uses of `constexpr llvm::StringLiteral` in llvm are with `static`. And, IIUC,
without `static`, the variable is going to live on the stack and be created
each time (even though its value was already computed at compile time).
Probably a moot point, though, since this is not part of a hotpath. For
consistency with other uses, I'd rather keep it, though.
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -1204,6 +1204,20 @@ void StreamChecker::evalGetdelim(const FnDescription
*Desc,
State->BindExpr(E.CE, C.getLocationContext(), RetVal);
StateNotFailed =
E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal, E.getZeroVal(Call));
+// The buffer size `*n` must be enough to hold the whole line, and
+// greater than the return value, since it has to account for '\0'.
+auto SizePtrSval = Call.getArgSVal(1);
+auto NVal = getPointeeVal(SizePtrSval, State);
+if (NVal) {
+ StateNotFailed = StateNotFailed->assume(
+ E.SVB
+ .evalBinOp(StateNotFailed, BO_GT, *NVal, RetVal,
+ E.SVB.getConditionType())
+ .castAs(),
+ true);
+ StateNotFailed =
+ StateNotFailed->BindExpr(E.CE, C.getLocationContext(), RetVal);
+}
alejandro-alvarez-sonarsource wrote:
In `stream.c`, the test `getline_buffer_size_invariant`
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83027
From a061464b75ac02c21e5d74fc4dff8d8afdbba66b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 20 Mar 2024 10:49:08 +0100
Subject: [PATCH 01/20] [NFC] UnixAPIMisuseChecker inherits from
Checker
---
.../Checkers/UnixAPIChecker.cpp | 60 +--
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index 19f1ca2dc824c9..599e5e6cedc606 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -17,6 +17,7 @@
#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "llvm/ADT/STLExtras.h"
@@ -41,8 +42,7 @@ enum class OpenVariant {
namespace {
class UnixAPIMisuseChecker
-: public Checker,
- check::ASTDecl> {
+: public Checker> {
const BugType BT_open{this, "Improper use of 'open'", categories::UnixAPI};
const BugType BT_pthreadOnce{this, "Improper use of 'pthread_once'",
categories::UnixAPI};
@@ -52,14 +52,14 @@ class UnixAPIMisuseChecker
void checkASTDecl(const TranslationUnitDecl *TU, AnalysisManager &Mgr,
BugReporter &BR) const;
- void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
+ void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
- void CheckOpen(CheckerContext &C, const CallExpr *CE) const;
- void CheckOpenAt(CheckerContext &C, const CallExpr *CE) const;
- void CheckPthreadOnce(CheckerContext &C, const CallExpr *CE) const;
+ void CheckOpen(CheckerContext &C, const CallEvent &Call) const;
+ void CheckOpenAt(CheckerContext &C, const CallEvent &Call) const;
+ void CheckPthreadOnce(CheckerContext &C, const CallEvent &Call) const;
- void CheckOpenVariant(CheckerContext &C,
-const CallExpr *CE, OpenVariant Variant) const;
+ void CheckOpenVariant(CheckerContext &C, const CallEvent &Call,
+OpenVariant Variant) const;
void ReportOpenBug(CheckerContext &C, ProgramStateRef State, const char *Msg,
SourceRange SR) const;
@@ -113,9 +113,9 @@ void UnixAPIMisuseChecker::checkASTDecl(const
TranslationUnitDecl *TU,
// "open" (man 2 open)
//===--===/
-void UnixAPIMisuseChecker::checkPreStmt(const CallExpr *CE,
+void UnixAPIMisuseChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
- const FunctionDecl *FD = C.getCalleeDecl(CE);
+ const FunctionDecl *FD = dyn_cast_if_present(Call.getDecl());
if (!FD || FD->getKind() != Decl::Function)
return;
@@ -130,13 +130,13 @@ void UnixAPIMisuseChecker::checkPreStmt(const CallExpr
*CE,
return;
if (FName == "open")
-CheckOpen(C, CE);
+CheckOpen(C, Call);
else if (FName == "openat")
-CheckOpenAt(C, CE);
+CheckOpenAt(C, Call);
else if (FName == "pthread_once")
-CheckPthreadOnce(C, CE);
+CheckPthreadOnce(C, Call);
}
void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext &C,
ProgramStateRef State,
@@ -152,17 +152,17 @@ void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext
&C,
}
void UnixAPIMisuseChecker::CheckOpen(CheckerContext &C,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::Open);
+ const CallEvent &Call) const {
+ CheckOpenVariant(C, Call, OpenVariant::Open);
}
void UnixAPIMisuseChecker::CheckOpenAt(CheckerContext &C,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::OpenAt);
+ const CallEvent &Call) const {
+ CheckOpenVariant(C, Call, OpenVariant::OpenAt);
}
void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
-const CallExpr *CE,
+const CallEvent &Call,
OpenVariant Variant) const {
// The index of the argument taking the flags open flags (O_RDONLY,
// O_WRONLY, O_CREAT, etc.),
@@ -191,11 +191,11 @@ void
UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
ProgramStateRef state = C.getState();
- if (CE->getNumArgs() < MinArgCount) {
+ if (Call.getNumArgs() < MinArgCount) {
// The frontend
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource commented: Applied the feedback. By the way, when `StdLibraryFunctionsChecker` is refactored, I'd be happy to give a hand if you need updating these checkers (UnixAPI and Stream). https://github.com/llvm/llvm-project/pull/83027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -1204,6 +1204,20 @@ void StreamChecker::evalGetdelim(const FnDescription
*Desc,
State->BindExpr(E.CE, C.getLocationContext(), RetVal);
StateNotFailed =
E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal, E.getZeroVal(Call));
+// The buffer size `*n` must be enough to hold the whole line, and
+// greater than the return value, since it has to account for '\0'.
+auto SizePtrSval = Call.getArgSVal(1);
+auto NVal = getPointeeVal(SizePtrSval, State);
+if (NVal) {
+ StateNotFailed = StateNotFailed->assume(
+ E.SVB
+ .evalBinOp(StateNotFailed, BO_GT, *NVal, RetVal,
+ E.SVB.getConditionType())
+ .castAs(),
+ true);
+ StateNotFailed =
+ StateNotFailed->BindExpr(E.CE, C.getLocationContext(), RetVal);
alejandro-alvarez-sonarsource wrote:
Replaced, thanks!
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -1217,6 +1231,11 @@ void StreamChecker::evalGetdelim(const FnDescription
*Desc,
E.isStreamEof() ? ErrorFEof : ErrorFEof | ErrorFError;
StateFailed = E.setStreamState(
StateFailed, StreamState::getOpened(Desc, NewES, !NewES.isFEof()));
+ // On failure, the content of the buffer is undefined.
+ if (auto NewLinePtr = getPointeeVal(Call.getArgSVal(0), State)) {
+StateFailed = StateFailed->bindLoc(*NewLinePtr, UndefinedVal(),
+ C.getLocationContext());
+ }
alejandro-alvarez-sonarsource wrote:
Removed.
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83027
From a061464b75ac02c21e5d74fc4dff8d8afdbba66b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 20 Mar 2024 10:49:08 +0100
Subject: [PATCH 01/22] [NFC] UnixAPIMisuseChecker inherits from
Checker
---
.../Checkers/UnixAPIChecker.cpp | 60 +--
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index 19f1ca2dc824c9..599e5e6cedc606 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -17,6 +17,7 @@
#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "llvm/ADT/STLExtras.h"
@@ -41,8 +42,7 @@ enum class OpenVariant {
namespace {
class UnixAPIMisuseChecker
-: public Checker,
- check::ASTDecl> {
+: public Checker> {
const BugType BT_open{this, "Improper use of 'open'", categories::UnixAPI};
const BugType BT_pthreadOnce{this, "Improper use of 'pthread_once'",
categories::UnixAPI};
@@ -52,14 +52,14 @@ class UnixAPIMisuseChecker
void checkASTDecl(const TranslationUnitDecl *TU, AnalysisManager &Mgr,
BugReporter &BR) const;
- void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
+ void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
- void CheckOpen(CheckerContext &C, const CallExpr *CE) const;
- void CheckOpenAt(CheckerContext &C, const CallExpr *CE) const;
- void CheckPthreadOnce(CheckerContext &C, const CallExpr *CE) const;
+ void CheckOpen(CheckerContext &C, const CallEvent &Call) const;
+ void CheckOpenAt(CheckerContext &C, const CallEvent &Call) const;
+ void CheckPthreadOnce(CheckerContext &C, const CallEvent &Call) const;
- void CheckOpenVariant(CheckerContext &C,
-const CallExpr *CE, OpenVariant Variant) const;
+ void CheckOpenVariant(CheckerContext &C, const CallEvent &Call,
+OpenVariant Variant) const;
void ReportOpenBug(CheckerContext &C, ProgramStateRef State, const char *Msg,
SourceRange SR) const;
@@ -113,9 +113,9 @@ void UnixAPIMisuseChecker::checkASTDecl(const
TranslationUnitDecl *TU,
// "open" (man 2 open)
//===--===/
-void UnixAPIMisuseChecker::checkPreStmt(const CallExpr *CE,
+void UnixAPIMisuseChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
- const FunctionDecl *FD = C.getCalleeDecl(CE);
+ const FunctionDecl *FD = dyn_cast_if_present(Call.getDecl());
if (!FD || FD->getKind() != Decl::Function)
return;
@@ -130,13 +130,13 @@ void UnixAPIMisuseChecker::checkPreStmt(const CallExpr
*CE,
return;
if (FName == "open")
-CheckOpen(C, CE);
+CheckOpen(C, Call);
else if (FName == "openat")
-CheckOpenAt(C, CE);
+CheckOpenAt(C, Call);
else if (FName == "pthread_once")
-CheckPthreadOnce(C, CE);
+CheckPthreadOnce(C, Call);
}
void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext &C,
ProgramStateRef State,
@@ -152,17 +152,17 @@ void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext
&C,
}
void UnixAPIMisuseChecker::CheckOpen(CheckerContext &C,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::Open);
+ const CallEvent &Call) const {
+ CheckOpenVariant(C, Call, OpenVariant::Open);
}
void UnixAPIMisuseChecker::CheckOpenAt(CheckerContext &C,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::OpenAt);
+ const CallEvent &Call) const {
+ CheckOpenVariant(C, Call, OpenVariant::OpenAt);
}
void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
-const CallExpr *CE,
+const CallEvent &Call,
OpenVariant Variant) const {
// The index of the argument taking the flags open flags (O_RDONLY,
// O_WRONLY, O_CREAT, etc.),
@@ -191,11 +191,11 @@ void
UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
ProgramStateRef state = C.getState();
- if (CE->getNumArgs() < MinArgCount) {
+ if (Call.getNumArgs() < MinArgCount) {
// The frontend
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -376,3 +377,122 @@ void fflush_on_open_failed_stream(void) {
}
fclose(F);
}
+
+void getline_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getline(&buffer, &n, NULL); // expected-warning {{Stream pointer might be
NULL}}
+}
+
+void getdelim_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getdelim(&buffer, &n, '\n', NULL); // expected-warning {{Stream pointer
might be NULL}}
+}
+
+void getline_no_return_check() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ char *line = NULL;
+ size_t len = 0;
+ getline(&line, &len, file);
+
+ if (line[0] == '\0') {} // expected-warning {{The left operand of '==' is a
garbage value}}
alejandro-alvarez-sonarsource wrote:
Changed.
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -376,3 +377,122 @@ void fflush_on_open_failed_stream(void) {
}
fclose(F);
}
+
+void getline_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getline(&buffer, &n, NULL); // expected-warning {{Stream pointer might be
NULL}}
+}
+
+void getdelim_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getdelim(&buffer, &n, '\n', NULL); // expected-warning {{Stream pointer
might be NULL}}
+}
+
+void getline_no_return_check() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ char *line = NULL;
+ size_t len = 0;
+ getline(&line, &len, file);
+
+ if (line[0] == '\0') {} // expected-warning {{The left operand of '==' is a
garbage value}}
+
+ free(line);
+ fclose(file);
+}
+
+void getline_after_eof() {
alejandro-alvarez-sonarsource wrote:
Fair. I have removed them.
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -376,3 +377,122 @@ void fflush_on_open_failed_stream(void) {
}
fclose(F);
}
+
+void getline_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getline(&buffer, &n, NULL); // expected-warning {{Stream pointer might be
NULL}}
+}
+
+void getdelim_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getdelim(&buffer, &n, '\n', NULL); // expected-warning {{Stream pointer
might be NULL}}
+}
+
+void getline_no_return_check() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ char *line = NULL;
+ size_t len = 0;
+ getline(&line, &len, file);
+
+ if (line[0] == '\0') {} // expected-warning {{The left operand of '==' is a
garbage value}}
+
+ free(line);
+ fclose(file);
+}
+
+void getline_after_eof() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ size_t n = 10;
+ char *buffer = malloc(n);
+ ssize_t read = fread(buffer, n, 1, file);
+ if (!feof(file)) {
+getline(&buffer, &n, file); // expected-warning {{File position of the
stream might be 'indeterminate' after a failed operation. Can cause undefined
behavior}}
+ }
+ fclose(file);
+ free(buffer);
+}
+
+void getline_feof() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ size_t n = 10;
+ char *buffer = malloc(n);
+ ssize_t read = fread(buffer, n, 1, file);
+ getline(&buffer, &n, file); // expected-warning {{File position of the
stream might be 'indeterminate' after a failed operation. Can cause undefined
behavior}} \\
+ expected-warning {{Read function called when stream is in EOF state.
Function has no effect}}
+ fclose(file);
+ free(buffer);
+}
+
+void getline_feof_check() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ char *line = NULL;
+ size_t len = 0;
+ ssize_t r = getline(&line, &len, file);
+
+ if (r != -1) {
+// success, end-of-file is not possible
+int f = feof(file);
+clang_analyzer_eval(f == 0); // expected-warning {{TRUE}}
+ } else {
+// failure, end-of-file is possible, but not the only reason to fail
+int f = feof(file);
+clang_analyzer_eval(f == 0); // expected-warning {{TRUE}} \\
+expected-warning {{FALSE}}
+ }
+ free(line);
+ fclose(file);
+}
+
+void getline_ret_value() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ size_t n = 0;
+ char *buffer = NULL;
+ ssize_t r = getline(&buffer, &n, file);
+
+ if (r > -1) {
+// The return value does *not* include the terminating null byte.
+// The buffer must be large enough to include it.
+clang_analyzer_eval(n > r); // expected-warning{{TRUE}}
+ }
+
+ fclose(file);
+ free(buffer);
+}
+
+
+void getline_buffer_size_invariant(char *buffer) {
alejandro-alvarez-sonarsource wrote:
I added it because @steakhal [was concerned about the handling of this
combination of
parameters](https://github.com/llvm/llvm-project/pull/83027#discussion_r1516087705).
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -376,3 +377,122 @@ void fflush_on_open_failed_stream(void) {
}
fclose(F);
}
+
+void getline_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getline(&buffer, &n, NULL); // expected-warning {{Stream pointer might be
NULL}}
+}
+
+void getdelim_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getdelim(&buffer, &n, '\n', NULL); // expected-warning {{Stream pointer
might be NULL}}
+}
+
+void getline_no_return_check() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ char *line = NULL;
+ size_t len = 0;
+ getline(&line, &len, file);
+
+ if (line[0] == '\0') {} // expected-warning {{The left operand of '==' is a
garbage value}}
+
+ free(line);
+ fclose(file);
+}
+
+void getline_after_eof() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ size_t n = 10;
+ char *buffer = malloc(n);
+ ssize_t read = fread(buffer, n, 1, file);
+ if (!feof(file)) {
+getline(&buffer, &n, file); // expected-warning {{File position of the
stream might be 'indeterminate' after a failed operation. Can cause undefined
behavior}}
+ }
+ fclose(file);
+ free(buffer);
+}
+
+void getline_feof() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ size_t n = 10;
+ char *buffer = malloc(n);
+ ssize_t read = fread(buffer, n, 1, file);
+ getline(&buffer, &n, file); // expected-warning {{File position of the
stream might be 'indeterminate' after a failed operation. Can cause undefined
behavior}} \\
+ expected-warning {{Read function called when stream is in EOF state.
Function has no effect}}
+ fclose(file);
+ free(buffer);
+}
+
+void getline_feof_check() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ char *line = NULL;
+ size_t len = 0;
+ ssize_t r = getline(&line, &len, file);
+
+ if (r != -1) {
+// success, end-of-file is not possible
+int f = feof(file);
+clang_analyzer_eval(f == 0); // expected-warning {{TRUE}}
+ } else {
+// failure, end-of-file is possible, but not the only reason to fail
+int f = feof(file);
+clang_analyzer_eval(f == 0); // expected-warning {{TRUE}} \\
+expected-warning {{FALSE}}
+ }
+ free(line);
+ fclose(file);
+}
+
+void getline_ret_value() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ size_t n = 0;
+ char *buffer = NULL;
+ ssize_t r = getline(&buffer, &n, file);
+
+ if (r > -1) {
+// The return value does *not* include the terminating null byte.
+// The buffer must be large enough to include it.
+clang_analyzer_eval(n > r); // expected-warning{{TRUE}}
alejandro-alvarez-sonarsource wrote:
Added, and fixed the bug. Good catch, thanks.
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83027
From a061464b75ac02c21e5d74fc4dff8d8afdbba66b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 20 Mar 2024 10:49:08 +0100
Subject: [PATCH 01/23] [NFC] UnixAPIMisuseChecker inherits from
Checker
---
.../Checkers/UnixAPIChecker.cpp | 60 +--
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index 19f1ca2dc824c9..599e5e6cedc606 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -17,6 +17,7 @@
#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "llvm/ADT/STLExtras.h"
@@ -41,8 +42,7 @@ enum class OpenVariant {
namespace {
class UnixAPIMisuseChecker
-: public Checker,
- check::ASTDecl> {
+: public Checker> {
const BugType BT_open{this, "Improper use of 'open'", categories::UnixAPI};
const BugType BT_pthreadOnce{this, "Improper use of 'pthread_once'",
categories::UnixAPI};
@@ -52,14 +52,14 @@ class UnixAPIMisuseChecker
void checkASTDecl(const TranslationUnitDecl *TU, AnalysisManager &Mgr,
BugReporter &BR) const;
- void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
+ void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
- void CheckOpen(CheckerContext &C, const CallExpr *CE) const;
- void CheckOpenAt(CheckerContext &C, const CallExpr *CE) const;
- void CheckPthreadOnce(CheckerContext &C, const CallExpr *CE) const;
+ void CheckOpen(CheckerContext &C, const CallEvent &Call) const;
+ void CheckOpenAt(CheckerContext &C, const CallEvent &Call) const;
+ void CheckPthreadOnce(CheckerContext &C, const CallEvent &Call) const;
- void CheckOpenVariant(CheckerContext &C,
-const CallExpr *CE, OpenVariant Variant) const;
+ void CheckOpenVariant(CheckerContext &C, const CallEvent &Call,
+OpenVariant Variant) const;
void ReportOpenBug(CheckerContext &C, ProgramStateRef State, const char *Msg,
SourceRange SR) const;
@@ -113,9 +113,9 @@ void UnixAPIMisuseChecker::checkASTDecl(const
TranslationUnitDecl *TU,
// "open" (man 2 open)
//===--===/
-void UnixAPIMisuseChecker::checkPreStmt(const CallExpr *CE,
+void UnixAPIMisuseChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
- const FunctionDecl *FD = C.getCalleeDecl(CE);
+ const FunctionDecl *FD = dyn_cast_if_present(Call.getDecl());
if (!FD || FD->getKind() != Decl::Function)
return;
@@ -130,13 +130,13 @@ void UnixAPIMisuseChecker::checkPreStmt(const CallExpr
*CE,
return;
if (FName == "open")
-CheckOpen(C, CE);
+CheckOpen(C, Call);
else if (FName == "openat")
-CheckOpenAt(C, CE);
+CheckOpenAt(C, Call);
else if (FName == "pthread_once")
-CheckPthreadOnce(C, CE);
+CheckPthreadOnce(C, Call);
}
void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext &C,
ProgramStateRef State,
@@ -152,17 +152,17 @@ void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext
&C,
}
void UnixAPIMisuseChecker::CheckOpen(CheckerContext &C,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::Open);
+ const CallEvent &Call) const {
+ CheckOpenVariant(C, Call, OpenVariant::Open);
}
void UnixAPIMisuseChecker::CheckOpenAt(CheckerContext &C,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::OpenAt);
+ const CallEvent &Call) const {
+ CheckOpenVariant(C, Call, OpenVariant::OpenAt);
}
void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
-const CallExpr *CE,
+const CallEvent &Call,
OpenVariant Variant) const {
// The index of the argument taking the flags open flags (O_RDONLY,
// O_WRONLY, O_CREAT, etc.),
@@ -191,11 +191,11 @@ void
UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
ProgramStateRef state = C.getState();
- if (CE->getNumArgs() < MinArgCount) {
+ if (Call.getNumArgs() < MinArgCount) {
// The frontend
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -376,3 +377,122 @@ void fflush_on_open_failed_stream(void) {
}
fclose(F);
}
+
+void getline_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getline(&buffer, &n, NULL); // expected-warning {{Stream pointer might be
NULL}}
+}
+
+void getdelim_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getdelim(&buffer, &n, '\n', NULL); // expected-warning {{Stream pointer
might be NULL}}
+}
+
+void getline_no_return_check() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ char *line = NULL;
+ size_t len = 0;
+ getline(&line, &len, file);
+
+ if (line[0] == '\0') {} // expected-warning {{The left operand of '==' is a
garbage value}}
+
+ free(line);
+ fclose(file);
+}
+
+void getline_after_eof() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ size_t n = 10;
+ char *buffer = malloc(n);
+ ssize_t read = fread(buffer, n, 1, file);
+ if (!feof(file)) {
+getline(&buffer, &n, file); // expected-warning {{File position of the
stream might be 'indeterminate' after a failed operation. Can cause undefined
behavior}}
+ }
+ fclose(file);
+ free(buffer);
+}
+
+void getline_feof() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ size_t n = 10;
+ char *buffer = malloc(n);
+ ssize_t read = fread(buffer, n, 1, file);
+ getline(&buffer, &n, file); // expected-warning {{File position of the
stream might be 'indeterminate' after a failed operation. Can cause undefined
behavior}} \\
+ expected-warning {{Read function called when stream is in EOF state.
Function has no effect}}
+ fclose(file);
+ free(buffer);
+}
+
+void getline_feof_check() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ char *line = NULL;
+ size_t len = 0;
+ ssize_t r = getline(&line, &len, file);
+
+ if (r != -1) {
+// success, end-of-file is not possible
+int f = feof(file);
+clang_analyzer_eval(f == 0); // expected-warning {{TRUE}}
+ } else {
+// failure, end-of-file is possible, but not the only reason to fail
+int f = feof(file);
+clang_analyzer_eval(f == 0); // expected-warning {{TRUE}} \\
+expected-warning {{FALSE}}
+ }
+ free(line);
+ fclose(file);
+}
+
+void getline_ret_value() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ size_t n = 0;
+ char *buffer = NULL;
+ ssize_t r = getline(&buffer, &n, file);
+
+ if (r > -1) {
+// The return value does *not* include the terminating null byte.
+// The buffer must be large enough to include it.
+clang_analyzer_eval(n > r); // expected-warning{{TRUE}}
+ }
+
+ fclose(file);
+ free(buffer);
+}
+
+
+void getline_buffer_size_invariant(char *buffer) {
alejandro-alvarez-sonarsource wrote:
I have reworked the test, including a couple of calls to `clang_analyze_eval `
to make sure the -1 is changed after the call.
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83027
From a061464b75ac02c21e5d74fc4dff8d8afdbba66b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 20 Mar 2024 10:49:08 +0100
Subject: [PATCH 01/24] [NFC] UnixAPIMisuseChecker inherits from
Checker
---
.../Checkers/UnixAPIChecker.cpp | 60 +--
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index 19f1ca2dc824c9..599e5e6cedc606 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -17,6 +17,7 @@
#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "llvm/ADT/STLExtras.h"
@@ -41,8 +42,7 @@ enum class OpenVariant {
namespace {
class UnixAPIMisuseChecker
-: public Checker,
- check::ASTDecl> {
+: public Checker> {
const BugType BT_open{this, "Improper use of 'open'", categories::UnixAPI};
const BugType BT_pthreadOnce{this, "Improper use of 'pthread_once'",
categories::UnixAPI};
@@ -52,14 +52,14 @@ class UnixAPIMisuseChecker
void checkASTDecl(const TranslationUnitDecl *TU, AnalysisManager &Mgr,
BugReporter &BR) const;
- void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
+ void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
- void CheckOpen(CheckerContext &C, const CallExpr *CE) const;
- void CheckOpenAt(CheckerContext &C, const CallExpr *CE) const;
- void CheckPthreadOnce(CheckerContext &C, const CallExpr *CE) const;
+ void CheckOpen(CheckerContext &C, const CallEvent &Call) const;
+ void CheckOpenAt(CheckerContext &C, const CallEvent &Call) const;
+ void CheckPthreadOnce(CheckerContext &C, const CallEvent &Call) const;
- void CheckOpenVariant(CheckerContext &C,
-const CallExpr *CE, OpenVariant Variant) const;
+ void CheckOpenVariant(CheckerContext &C, const CallEvent &Call,
+OpenVariant Variant) const;
void ReportOpenBug(CheckerContext &C, ProgramStateRef State, const char *Msg,
SourceRange SR) const;
@@ -113,9 +113,9 @@ void UnixAPIMisuseChecker::checkASTDecl(const
TranslationUnitDecl *TU,
// "open" (man 2 open)
//===--===/
-void UnixAPIMisuseChecker::checkPreStmt(const CallExpr *CE,
+void UnixAPIMisuseChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
- const FunctionDecl *FD = C.getCalleeDecl(CE);
+ const FunctionDecl *FD = dyn_cast_if_present(Call.getDecl());
if (!FD || FD->getKind() != Decl::Function)
return;
@@ -130,13 +130,13 @@ void UnixAPIMisuseChecker::checkPreStmt(const CallExpr
*CE,
return;
if (FName == "open")
-CheckOpen(C, CE);
+CheckOpen(C, Call);
else if (FName == "openat")
-CheckOpenAt(C, CE);
+CheckOpenAt(C, Call);
else if (FName == "pthread_once")
-CheckPthreadOnce(C, CE);
+CheckPthreadOnce(C, Call);
}
void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext &C,
ProgramStateRef State,
@@ -152,17 +152,17 @@ void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext
&C,
}
void UnixAPIMisuseChecker::CheckOpen(CheckerContext &C,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::Open);
+ const CallEvent &Call) const {
+ CheckOpenVariant(C, Call, OpenVariant::Open);
}
void UnixAPIMisuseChecker::CheckOpenAt(CheckerContext &C,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::OpenAt);
+ const CallEvent &Call) const {
+ CheckOpenVariant(C, Call, OpenVariant::OpenAt);
}
void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
-const CallExpr *CE,
+const CallEvent &Call,
OpenVariant Variant) const {
// The index of the argument taking the flags open flags (O_RDONLY,
// O_WRONLY, O_CREAT, etc.),
@@ -191,11 +191,11 @@ void
UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
ProgramStateRef state = C.getState();
- if (CE->getNumArgs() < MinArgCount) {
+ if (Call.getNumArgs() < MinArgCount) {
// The frontend
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -376,3 +377,75 @@ void fflush_on_open_failed_stream(void) {
}
fclose(F);
}
+
+void getline_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getline(&buffer, &n, NULL); // expected-warning {{Stream pointer might be
NULL}}
+}
+
+void getdelim_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getdelim(&buffer, &n, '\n', NULL); // expected-warning {{Stream pointer
might be NULL}}
+}
+
+void getline_buffer_on_error() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ char *line = NULL;
+ size_t len = 0;
+ if (getline(&line, &len, file) == -1) {
+if (line[0] == '\0') {} // expected-warning {{The left operand of '==' is
a garbage value}}
+ } else {
+if (line[0] == '\0') {} // no warning
+ }
+
+ free(line);
+ fclose(file);
+}
+
+void getline_ret_value() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ size_t n = 0;
+ char *buffer = NULL;
+ ssize_t r = getline(&buffer, &n, file);
+
+ if (r > -1) {
+// The return value does *not* include the terminating null byte.
+// The buffer must be large enough to include it.
+clang_analyzer_eval(n > r); // expected-warning{{TRUE}}
+clang_analyzer_eval(buffer != NULL); // expected-warning{{TRUE}}
+ }
+
+ fclose(file);
+ free(buffer);
+}
+
+
+void getline_buffer_size_negative() {
+ FILE *file = fopen("file.txt", "r");
+ if (file == NULL) {
+return;
+ }
+
+ size_t n = -1;
+ clang_analyzer_eval((ssize_t)n >= 0); // expected-warning{{FALSE}}
+ char *buffer = NULL;
+ ssize_t r = getline(&buffer, &n, file);
+
+ if (r > -1) {
+clang_analyzer_eval((ssize_t)n > r); // expected-warning{{TRUE}}
+ clang_analyzer_eval(buffer != NULL); // expected-warning{{TRUE}}
alejandro-alvarez-sonarsource wrote:
Replaced with spaces.
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83027
From 18569fc14e2d9050acbc63c2367f9a1ec6f8577b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 20 Mar 2024 10:49:08 +0100
Subject: [PATCH 1/2] [clang][analyzer][NFC] UnixAPIMisuseChecker inherits from
Checker
---
.../Checkers/UnixAPIChecker.cpp | 60 +--
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index 19f1ca2dc824c9..599e5e6cedc606 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -17,6 +17,7 @@
#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "llvm/ADT/STLExtras.h"
@@ -41,8 +42,7 @@ enum class OpenVariant {
namespace {
class UnixAPIMisuseChecker
-: public Checker,
- check::ASTDecl> {
+: public Checker> {
const BugType BT_open{this, "Improper use of 'open'", categories::UnixAPI};
const BugType BT_pthreadOnce{this, "Improper use of 'pthread_once'",
categories::UnixAPI};
@@ -52,14 +52,14 @@ class UnixAPIMisuseChecker
void checkASTDecl(const TranslationUnitDecl *TU, AnalysisManager &Mgr,
BugReporter &BR) const;
- void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
+ void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
- void CheckOpen(CheckerContext &C, const CallExpr *CE) const;
- void CheckOpenAt(CheckerContext &C, const CallExpr *CE) const;
- void CheckPthreadOnce(CheckerContext &C, const CallExpr *CE) const;
+ void CheckOpen(CheckerContext &C, const CallEvent &Call) const;
+ void CheckOpenAt(CheckerContext &C, const CallEvent &Call) const;
+ void CheckPthreadOnce(CheckerContext &C, const CallEvent &Call) const;
- void CheckOpenVariant(CheckerContext &C,
-const CallExpr *CE, OpenVariant Variant) const;
+ void CheckOpenVariant(CheckerContext &C, const CallEvent &Call,
+OpenVariant Variant) const;
void ReportOpenBug(CheckerContext &C, ProgramStateRef State, const char *Msg,
SourceRange SR) const;
@@ -113,9 +113,9 @@ void UnixAPIMisuseChecker::checkASTDecl(const
TranslationUnitDecl *TU,
// "open" (man 2 open)
//===--===/
-void UnixAPIMisuseChecker::checkPreStmt(const CallExpr *CE,
+void UnixAPIMisuseChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
- const FunctionDecl *FD = C.getCalleeDecl(CE);
+ const FunctionDecl *FD = dyn_cast_if_present(Call.getDecl());
if (!FD || FD->getKind() != Decl::Function)
return;
@@ -130,13 +130,13 @@ void UnixAPIMisuseChecker::checkPreStmt(const CallExpr
*CE,
return;
if (FName == "open")
-CheckOpen(C, CE);
+CheckOpen(C, Call);
else if (FName == "openat")
-CheckOpenAt(C, CE);
+CheckOpenAt(C, Call);
else if (FName == "pthread_once")
-CheckPthreadOnce(C, CE);
+CheckPthreadOnce(C, Call);
}
void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext &C,
ProgramStateRef State,
@@ -152,17 +152,17 @@ void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext
&C,
}
void UnixAPIMisuseChecker::CheckOpen(CheckerContext &C,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::Open);
+ const CallEvent &Call) const {
+ CheckOpenVariant(C, Call, OpenVariant::Open);
}
void UnixAPIMisuseChecker::CheckOpenAt(CheckerContext &C,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::OpenAt);
+ const CallEvent &Call) const {
+ CheckOpenVariant(C, Call, OpenVariant::OpenAt);
}
void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
-const CallExpr *CE,
+const CallEvent &Call,
OpenVariant Variant) const {
// The index of the argument taking the flags open flags (O_RDONLY,
// O_WRONLY, O_CREAT, etc.),
@@ -191,11 +191,11 @@ void
UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
ProgramStateRef state = C.getState();
- if (CE->getNumArgs() < MinArgCount) {
+ if (Call.getNumArgs() < MinArgCount) {
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
alejandro-alvarez-sonarsource wrote: Rebased and squashed into two commits that should be kept separate. https://github.com/llvm/llvm-project/pull/83027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reapply "[clang][analyzer] StreamChecker: Model getc, vfscanf, putc, … (PR #83281)
alejandro-alvarez-sonarsource wrote: Well, the test now runs for 4 different platforms and the PR checks are all green, so I guess we can merge. https://github.com/llvm/llvm-project/pull/83281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83027
From 997501888aacdbae59ace767e085922c9aa96a22 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 21 Feb 2024 14:46:01 +0100
Subject: [PATCH] [clang][analyzer] Model getline/getdelim preconditions
1. lineptr, n and stream can not be NULL
2. if *lineptr is NULL, *n must be 0
---
.../Core/PathSensitive/CheckerHelpers.h | 6 +
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 155 -
.../StaticAnalyzer/Core/CheckerHelpers.cpp| 9 +
clang/test/Analysis/getline-stream.c | 327 ++
4 files changed, 495 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Analysis/getline-stream.c
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
index 65982457ad8393..60421e5437d82f 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
@@ -13,6 +13,9 @@
#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
+#include "ProgramState_Fwd.h"
+#include "SVals.h"
+
#include "clang/AST/OperationKinds.h"
#include "clang/AST/Stmt.h"
#include "clang/Basic/OperatorKinds.h"
@@ -110,6 +113,9 @@ class OperatorKind {
OperatorKind operationKindFromOverloadedOperator(OverloadedOperatorKind OOK,
bool IsBinary);
+std::optional getPointeeDefVal(SVal PtrSVal,
+ProgramStateRef State);
+
} // namespace ento
} // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 2ec47bf55df76b..bacac7613f880c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/Sequence.h"
#include
#include
@@ -234,6 +235,9 @@ class StreamChecker : public Checker();
+ if (!Ptr)
+return nullptr;
+
+ assert(PtrExpr && "Expected an argument");
+
+ const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
+ if (!PtrNotNull && PtrNull) {
+if (ExplodedNode *N = C.generateErrorNode(PtrNull)) {
+ SmallString<256> buf;
+ llvm::raw_svector_ostream os(buf);
+ os << PtrDescr << " pointer might be NULL.";
+
+ auto R = std::make_unique(BT_SizeNull, buf, N);
+ bugreporter::trackExpressionValue(N, PtrExpr, *R);
+ C.emitReport(std::move(R));
+}
+return nullptr;
+ }
+
+ return PtrNotNull;
+}
+
+ProgramStateRef StreamChecker::ensureSizeZeroIfLineNull(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+ static constexpr char SizeNotZeroMsg[] =
+ "Line pointer might be null while n value is not zero";
+
+ // We have a pointer to a pointer to the buffer, and a pointer to the size.
+ // We want what they point at.
+ auto LinePtrSVal = getPointeeDefVal(LinePtrPtrSVal, State);
+ auto NSVal = getPointeeDefVal(SizePtrSVal, State);
+ if (!LinePtrSVal || !NSVal)
+return nullptr;
+
+ assert(LinePtrPtrExpr &&
+ "Expected an argument with a pointer to a pointer to the buffer.");
+ assert(SizePtrExpr &&
+ "Expected an argument with a pointer to the buffer size.");
+
+ // If the line pointer is null, and n is > 0, there is UB.
+ const auto [LinePtrNotNull, LinePtrNull] = State->assume(*LinePtrSVal);
+ if (LinePtrNull && !LinePtrNotNull) {
+const auto [NIsNotZero, NIsZero] = LinePtrNull->assume(*NSVal);
+if (NIsNotZero && !NIsZero) {
+ if (ExplodedNode *N = C.generateErrorNode(NIsNotZero)) {
+auto R = std::make_unique(BT_SizeNotZero,
+ SizeNotZeroMsg, N);
+bugreporter::trackExpressionValue(N, SizePtrExpr, *R);
+bugreporter::trackExpressionValue(N, LinePtrPtrExpr, *R);
+C.emitReport(std::move(R));
+ }
+ return nullptr;
+}
+return NIsZero;
+ }
+ return LinePtrNotNull;
+}
+
+void StreamChecker::preGetdelim(const FnDescription *Desc,
+const CallEvent &Call,
+CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+
+ auto AddTransitionOnReturn = llvm::make_scope_exit([&] {
+if (State != nullptr) {
+ C.addTransition(State);
+}
+ });
+
+ State = ensu
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
alejandro-alvarez-sonarsource wrote: Rebased on top of main and solved conflicts. https://github.com/llvm/llvm-project/pull/83027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -1158,6 +1173,123 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent &Call,
C.addTransition(StateFailed);
}
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+CheckerContext &C, ProgramStateRef State,
+const StringRef PtrDescr) const {
+ const auto Ptr = PtrVal.getAs();
+ if (!Ptr)
+return nullptr;
+
+ assert(PtrExpr && "Expected an argument");
+
+ const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
+ if (!PtrNotNull && PtrNull) {
+if (ExplodedNode *N = C.generateErrorNode(PtrNull)) {
+ SmallString<256> buf;
+ llvm::raw_svector_ostream os(buf);
+ os << PtrDescr << " pointer might be NULL.";
+
+ auto R = std::make_unique(BT_SizeNull, buf, N);
+ bugreporter::trackExpressionValue(N, PtrExpr, *R);
+ C.emitReport(std::move(R));
+}
+return nullptr;
+ }
+
+ return PtrNotNull;
+}
+
+ProgramStateRef StreamChecker::ensureSizeZeroIfLineNull(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+ static constexpr char SizeNotZeroMsg[] =
+ "Line pointer might be null while n value is not zero";
+
+ // We have a pointer to a pointer to the buffer, and a pointer to the size.
+ // We want what they point at.
+ auto LinePtrSVal = getPointeeDefVal(LinePtrPtrSVal, State);
+ auto NSVal = getPointeeDefVal(SizePtrSVal, State);
+ if (!LinePtrSVal || !NSVal)
+return nullptr;
+
+ assert(LinePtrPtrExpr &&
+ "Expected an argument with a pointer to a pointer to the buffer.");
+ assert(SizePtrExpr &&
+ "Expected an argument with a pointer to the buffer size.");
+
+ // If the line pointer is null, and n is > 0, there is UB.
+ const auto [LinePtrNotNull, LinePtrNull] = State->assume(*LinePtrSVal);
+ if (LinePtrNull && !LinePtrNotNull) {
+const auto [NIsNotZero, NIsZero] = LinePtrNull->assume(*NSVal);
+if (NIsNotZero && !NIsZero) {
+ if (ExplodedNode *N = C.generateErrorNode(NIsNotZero)) {
+auto R = std::make_unique(BT_SizeNotZero,
+ SizeNotZeroMsg, N);
+bugreporter::trackExpressionValue(N, SizePtrExpr, *R);
+bugreporter::trackExpressionValue(N, LinePtrPtrExpr, *R);
+C.emitReport(std::move(R));
+ }
+ return nullptr;
+}
+return NIsZero;
+ }
+ return LinePtrNotNull;
+}
+
+void StreamChecker::preGetdelim(const FnDescription *Desc,
+const CallEvent &Call,
+CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+
+ auto AddTransitionOnReturn = llvm::make_scope_exit([&] {
+if (State != nullptr) {
+ C.addTransition(State);
+}
+ });
+
+ State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
+ State);
+ if (!State)
+return;
+ State = ensureStreamOpened(StreamVal, C, State);
+ if (!State)
+return;
+ State = ensureNoFilePositionIndeterminate(StreamVal, C, State);
+ if (!State)
+return;
+
+ // n must not be NULL
alejandro-alvarez-sonarsource wrote:
I went through them, capitalized, quoted, and punctuated. I hope I did not miss
any.
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83027
From 997501888aacdbae59ace767e085922c9aa96a22 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 21 Feb 2024 14:46:01 +0100
Subject: [PATCH 1/2] [clang][analyzer] Model getline/getdelim preconditions
1. lineptr, n and stream can not be NULL
2. if *lineptr is NULL, *n must be 0
---
.../Core/PathSensitive/CheckerHelpers.h | 6 +
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 155 -
.../StaticAnalyzer/Core/CheckerHelpers.cpp| 9 +
clang/test/Analysis/getline-stream.c | 327 ++
4 files changed, 495 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Analysis/getline-stream.c
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
index 65982457ad8393..60421e5437d82f 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
@@ -13,6 +13,9 @@
#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
+#include "ProgramState_Fwd.h"
+#include "SVals.h"
+
#include "clang/AST/OperationKinds.h"
#include "clang/AST/Stmt.h"
#include "clang/Basic/OperatorKinds.h"
@@ -110,6 +113,9 @@ class OperatorKind {
OperatorKind operationKindFromOverloadedOperator(OverloadedOperatorKind OOK,
bool IsBinary);
+std::optional getPointeeDefVal(SVal PtrSVal,
+ProgramStateRef State);
+
} // namespace ento
} // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 2ec47bf55df76b..bacac7613f880c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/Sequence.h"
#include
#include
@@ -234,6 +235,9 @@ class StreamChecker : public Checker();
+ if (!Ptr)
+return nullptr;
+
+ assert(PtrExpr && "Expected an argument");
+
+ const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
+ if (!PtrNotNull && PtrNull) {
+if (ExplodedNode *N = C.generateErrorNode(PtrNull)) {
+ SmallString<256> buf;
+ llvm::raw_svector_ostream os(buf);
+ os << PtrDescr << " pointer might be NULL.";
+
+ auto R = std::make_unique(BT_SizeNull, buf, N);
+ bugreporter::trackExpressionValue(N, PtrExpr, *R);
+ C.emitReport(std::move(R));
+}
+return nullptr;
+ }
+
+ return PtrNotNull;
+}
+
+ProgramStateRef StreamChecker::ensureSizeZeroIfLineNull(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+ static constexpr char SizeNotZeroMsg[] =
+ "Line pointer might be null while n value is not zero";
+
+ // We have a pointer to a pointer to the buffer, and a pointer to the size.
+ // We want what they point at.
+ auto LinePtrSVal = getPointeeDefVal(LinePtrPtrSVal, State);
+ auto NSVal = getPointeeDefVal(SizePtrSVal, State);
+ if (!LinePtrSVal || !NSVal)
+return nullptr;
+
+ assert(LinePtrPtrExpr &&
+ "Expected an argument with a pointer to a pointer to the buffer.");
+ assert(SizePtrExpr &&
+ "Expected an argument with a pointer to the buffer size.");
+
+ // If the line pointer is null, and n is > 0, there is UB.
+ const auto [LinePtrNotNull, LinePtrNull] = State->assume(*LinePtrSVal);
+ if (LinePtrNull && !LinePtrNotNull) {
+const auto [NIsNotZero, NIsZero] = LinePtrNull->assume(*NSVal);
+if (NIsNotZero && !NIsZero) {
+ if (ExplodedNode *N = C.generateErrorNode(NIsNotZero)) {
+auto R = std::make_unique(BT_SizeNotZero,
+ SizeNotZeroMsg, N);
+bugreporter::trackExpressionValue(N, SizePtrExpr, *R);
+bugreporter::trackExpressionValue(N, LinePtrPtrExpr, *R);
+C.emitReport(std::move(R));
+ }
+ return nullptr;
+}
+return NIsZero;
+ }
+ return LinePtrNotNull;
+}
+
+void StreamChecker::preGetdelim(const FnDescription *Desc,
+const CallEvent &Call,
+CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+
+ auto AddTransitionOnReturn = llvm::make_scope_exit([&] {
+if (State != nullptr) {
+ C.addTransition(State);
+}
+ });
+
+ State =
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -13,6 +13,9 @@ #ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H #define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H +#include "ProgramState_Fwd.h" +#include "SVals.h" + alejandro-alvarez-sonarsource wrote: Removed. https://github.com/llvm/llvm-project/pull/83027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -0,0 +1,327 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s + +#include "Inputs/system-header-simulator.h" +#include "Inputs/system-header-simulator-for-malloc.h" +#include "Inputs/system-header-simulator-for-valist.h" + +void clang_analyzer_eval(int); +void clang_analyzer_dump_int(int); +extern void clang_analyzer_dump_ptr(void*); +extern void clang_analyzer_warnIfReached(); alejandro-alvarez-sonarsource wrote: Removed them for consistency. https://github.com/llvm/llvm-project/pull/83027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83027
From 5c919832f9176d4b1af1312a4ee7cf30b788958a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 21 Feb 2024 14:46:01 +0100
Subject: [PATCH 1/2] [clang][analyzer] Model getline/getdelim preconditions
1. lineptr, n and stream can not be NULL
2. if *lineptr is NULL, *n must be 0
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 155 -
clang/test/Analysis/getline-stream.c | 327 ++
2 files changed, 480 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Analysis/getline-stream.c
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 2ec47bf55df76b..bacac7613f880c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/Sequence.h"
#include
#include
@@ -234,6 +235,9 @@ class StreamChecker : public Checker();
+ if (!Ptr)
+return nullptr;
+
+ assert(PtrExpr && "Expected an argument");
+
+ const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
+ if (!PtrNotNull && PtrNull) {
+if (ExplodedNode *N = C.generateErrorNode(PtrNull)) {
+ SmallString<256> buf;
+ llvm::raw_svector_ostream os(buf);
+ os << PtrDescr << " pointer might be NULL.";
+
+ auto R = std::make_unique(BT_SizeNull, buf, N);
+ bugreporter::trackExpressionValue(N, PtrExpr, *R);
+ C.emitReport(std::move(R));
+}
+return nullptr;
+ }
+
+ return PtrNotNull;
+}
+
+ProgramStateRef StreamChecker::ensureSizeZeroIfLineNull(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+ static constexpr char SizeNotZeroMsg[] =
+ "Line pointer might be null while n value is not zero";
+
+ // We have a pointer to a pointer to the buffer, and a pointer to the size.
+ // We want what they point at.
+ auto LinePtrSVal = getPointeeDefVal(LinePtrPtrSVal, State);
+ auto NSVal = getPointeeDefVal(SizePtrSVal, State);
+ if (!LinePtrSVal || !NSVal)
+return nullptr;
+
+ assert(LinePtrPtrExpr &&
+ "Expected an argument with a pointer to a pointer to the buffer.");
+ assert(SizePtrExpr &&
+ "Expected an argument with a pointer to the buffer size.");
+
+ // If the line pointer is null, and n is > 0, there is UB.
+ const auto [LinePtrNotNull, LinePtrNull] = State->assume(*LinePtrSVal);
+ if (LinePtrNull && !LinePtrNotNull) {
+const auto [NIsNotZero, NIsZero] = LinePtrNull->assume(*NSVal);
+if (NIsNotZero && !NIsZero) {
+ if (ExplodedNode *N = C.generateErrorNode(NIsNotZero)) {
+auto R = std::make_unique(BT_SizeNotZero,
+ SizeNotZeroMsg, N);
+bugreporter::trackExpressionValue(N, SizePtrExpr, *R);
+bugreporter::trackExpressionValue(N, LinePtrPtrExpr, *R);
+C.emitReport(std::move(R));
+ }
+ return nullptr;
+}
+return NIsZero;
+ }
+ return LinePtrNotNull;
+}
+
+void StreamChecker::preGetdelim(const FnDescription *Desc,
+const CallEvent &Call,
+CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+
+ auto AddTransitionOnReturn = llvm::make_scope_exit([&] {
+if (State != nullptr) {
+ C.addTransition(State);
+}
+ });
+
+ State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
+ State);
+ if (!State)
+return;
+ State = ensureStreamOpened(StreamVal, C, State);
+ if (!State)
+return;
+ State = ensureNoFilePositionIndeterminate(StreamVal, C, State);
+ if (!State)
+return;
+
+ // n must not be NULL
+ SVal SizePtrSval = Call.getArgSVal(1);
+ State = ensurePtrNotNull(SizePtrSval, Call.getArgExpr(1), C, State, "Size");
+ if (!State)
+return;
+
+ // lineptr must not be NULL
+ SVal LinePtrPtrSVal = Call.getArgSVal(0);
+ State =
+ ensurePtrNotNull(LinePtrPtrSVal, Call.getArgExpr(0), C, State, "Line");
+ if (!State)
+return;
+
+ // If lineptr points to a NULL pointer, *n must be 0
+ State =
+ ensureSizeZeroIfLineNull(LinePtrPtrSVal, SizePtrSval, Call.getArgExpr(0),
+ Call.getArgExpr(1), C, State);
+ if (!State)
+return;
+
+ SymbolRef Sym = StreamVal.getAsSymbol();
+ if (Sym && State->get(Sym)) {
+const StreamState *SS = State->get(Sym);
+if (SS->ErrorState & ErrorFEof)
+ reportFEofWarning(Sym, C, State);
+ } else
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
alejandro-alvarez-sonarsource wrote: Rebased on top of main to solve conflicts. https://github.com/llvm/llvm-project/pull/83027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83027
From 5c919832f9176d4b1af1312a4ee7cf30b788958a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 21 Feb 2024 14:46:01 +0100
Subject: [PATCH 1/3] [clang][analyzer] Model getline/getdelim preconditions
1. lineptr, n and stream can not be NULL
2. if *lineptr is NULL, *n must be 0
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 155 -
clang/test/Analysis/getline-stream.c | 327 ++
2 files changed, 480 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Analysis/getline-stream.c
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 2ec47bf55df76b..bacac7613f880c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/Sequence.h"
#include
#include
@@ -234,6 +235,9 @@ class StreamChecker : public Checker();
+ if (!Ptr)
+return nullptr;
+
+ assert(PtrExpr && "Expected an argument");
+
+ const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
+ if (!PtrNotNull && PtrNull) {
+if (ExplodedNode *N = C.generateErrorNode(PtrNull)) {
+ SmallString<256> buf;
+ llvm::raw_svector_ostream os(buf);
+ os << PtrDescr << " pointer might be NULL.";
+
+ auto R = std::make_unique(BT_SizeNull, buf, N);
+ bugreporter::trackExpressionValue(N, PtrExpr, *R);
+ C.emitReport(std::move(R));
+}
+return nullptr;
+ }
+
+ return PtrNotNull;
+}
+
+ProgramStateRef StreamChecker::ensureSizeZeroIfLineNull(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+ static constexpr char SizeNotZeroMsg[] =
+ "Line pointer might be null while n value is not zero";
+
+ // We have a pointer to a pointer to the buffer, and a pointer to the size.
+ // We want what they point at.
+ auto LinePtrSVal = getPointeeDefVal(LinePtrPtrSVal, State);
+ auto NSVal = getPointeeDefVal(SizePtrSVal, State);
+ if (!LinePtrSVal || !NSVal)
+return nullptr;
+
+ assert(LinePtrPtrExpr &&
+ "Expected an argument with a pointer to a pointer to the buffer.");
+ assert(SizePtrExpr &&
+ "Expected an argument with a pointer to the buffer size.");
+
+ // If the line pointer is null, and n is > 0, there is UB.
+ const auto [LinePtrNotNull, LinePtrNull] = State->assume(*LinePtrSVal);
+ if (LinePtrNull && !LinePtrNotNull) {
+const auto [NIsNotZero, NIsZero] = LinePtrNull->assume(*NSVal);
+if (NIsNotZero && !NIsZero) {
+ if (ExplodedNode *N = C.generateErrorNode(NIsNotZero)) {
+auto R = std::make_unique(BT_SizeNotZero,
+ SizeNotZeroMsg, N);
+bugreporter::trackExpressionValue(N, SizePtrExpr, *R);
+bugreporter::trackExpressionValue(N, LinePtrPtrExpr, *R);
+C.emitReport(std::move(R));
+ }
+ return nullptr;
+}
+return NIsZero;
+ }
+ return LinePtrNotNull;
+}
+
+void StreamChecker::preGetdelim(const FnDescription *Desc,
+const CallEvent &Call,
+CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+
+ auto AddTransitionOnReturn = llvm::make_scope_exit([&] {
+if (State != nullptr) {
+ C.addTransition(State);
+}
+ });
+
+ State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
+ State);
+ if (!State)
+return;
+ State = ensureStreamOpened(StreamVal, C, State);
+ if (!State)
+return;
+ State = ensureNoFilePositionIndeterminate(StreamVal, C, State);
+ if (!State)
+return;
+
+ // n must not be NULL
+ SVal SizePtrSval = Call.getArgSVal(1);
+ State = ensurePtrNotNull(SizePtrSval, Call.getArgExpr(1), C, State, "Size");
+ if (!State)
+return;
+
+ // lineptr must not be NULL
+ SVal LinePtrPtrSVal = Call.getArgSVal(0);
+ State =
+ ensurePtrNotNull(LinePtrPtrSVal, Call.getArgExpr(0), C, State, "Line");
+ if (!State)
+return;
+
+ // If lineptr points to a NULL pointer, *n must be 0
+ State =
+ ensureSizeZeroIfLineNull(LinePtrPtrSVal, SizePtrSval, Call.getArgExpr(0),
+ Call.getArgExpr(1), C, State);
+ if (!State)
+return;
+
+ SymbolRef Sym = StreamVal.getAsSymbol();
+ if (Sym && State->get(Sym)) {
+const StreamState *SS = State->get(Sym);
+if (SS->ErrorState & ErrorFEof)
+ reportFEofWarning(Sym, C, State);
+ } else
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83027
From 5c919832f9176d4b1af1312a4ee7cf30b788958a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 21 Feb 2024 14:46:01 +0100
Subject: [PATCH 1/4] [clang][analyzer] Model getline/getdelim preconditions
1. lineptr, n and stream can not be NULL
2. if *lineptr is NULL, *n must be 0
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 155 -
clang/test/Analysis/getline-stream.c | 327 ++
2 files changed, 480 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Analysis/getline-stream.c
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 2ec47bf55df76b..bacac7613f880c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/Sequence.h"
#include
#include
@@ -234,6 +235,9 @@ class StreamChecker : public Checker();
+ if (!Ptr)
+return nullptr;
+
+ assert(PtrExpr && "Expected an argument");
+
+ const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
+ if (!PtrNotNull && PtrNull) {
+if (ExplodedNode *N = C.generateErrorNode(PtrNull)) {
+ SmallString<256> buf;
+ llvm::raw_svector_ostream os(buf);
+ os << PtrDescr << " pointer might be NULL.";
+
+ auto R = std::make_unique(BT_SizeNull, buf, N);
+ bugreporter::trackExpressionValue(N, PtrExpr, *R);
+ C.emitReport(std::move(R));
+}
+return nullptr;
+ }
+
+ return PtrNotNull;
+}
+
+ProgramStateRef StreamChecker::ensureSizeZeroIfLineNull(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+ static constexpr char SizeNotZeroMsg[] =
+ "Line pointer might be null while n value is not zero";
+
+ // We have a pointer to a pointer to the buffer, and a pointer to the size.
+ // We want what they point at.
+ auto LinePtrSVal = getPointeeDefVal(LinePtrPtrSVal, State);
+ auto NSVal = getPointeeDefVal(SizePtrSVal, State);
+ if (!LinePtrSVal || !NSVal)
+return nullptr;
+
+ assert(LinePtrPtrExpr &&
+ "Expected an argument with a pointer to a pointer to the buffer.");
+ assert(SizePtrExpr &&
+ "Expected an argument with a pointer to the buffer size.");
+
+ // If the line pointer is null, and n is > 0, there is UB.
+ const auto [LinePtrNotNull, LinePtrNull] = State->assume(*LinePtrSVal);
+ if (LinePtrNull && !LinePtrNotNull) {
+const auto [NIsNotZero, NIsZero] = LinePtrNull->assume(*NSVal);
+if (NIsNotZero && !NIsZero) {
+ if (ExplodedNode *N = C.generateErrorNode(NIsNotZero)) {
+auto R = std::make_unique(BT_SizeNotZero,
+ SizeNotZeroMsg, N);
+bugreporter::trackExpressionValue(N, SizePtrExpr, *R);
+bugreporter::trackExpressionValue(N, LinePtrPtrExpr, *R);
+C.emitReport(std::move(R));
+ }
+ return nullptr;
+}
+return NIsZero;
+ }
+ return LinePtrNotNull;
+}
+
+void StreamChecker::preGetdelim(const FnDescription *Desc,
+const CallEvent &Call,
+CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+
+ auto AddTransitionOnReturn = llvm::make_scope_exit([&] {
+if (State != nullptr) {
+ C.addTransition(State);
+}
+ });
+
+ State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
+ State);
+ if (!State)
+return;
+ State = ensureStreamOpened(StreamVal, C, State);
+ if (!State)
+return;
+ State = ensureNoFilePositionIndeterminate(StreamVal, C, State);
+ if (!State)
+return;
+
+ // n must not be NULL
+ SVal SizePtrSval = Call.getArgSVal(1);
+ State = ensurePtrNotNull(SizePtrSval, Call.getArgExpr(1), C, State, "Size");
+ if (!State)
+return;
+
+ // lineptr must not be NULL
+ SVal LinePtrPtrSVal = Call.getArgSVal(0);
+ State =
+ ensurePtrNotNull(LinePtrPtrSVal, Call.getArgExpr(0), C, State, "Line");
+ if (!State)
+return;
+
+ // If lineptr points to a NULL pointer, *n must be 0
+ State =
+ ensureSizeZeroIfLineNull(LinePtrPtrSVal, SizePtrSval, Call.getArgExpr(0),
+ Call.getArgExpr(1), C, State);
+ if (!State)
+return;
+
+ SymbolRef Sym = StreamVal.getAsSymbol();
+ if (Sym && State->get(Sym)) {
+const StreamState *SS = State->get(Sym);
+if (SS->ErrorState & ErrorFEof)
+ reportFEofWarning(Sym, C, State);
+ } else
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -1196,6 +1342,11 @@ void StreamChecker::evalGetdelim(const FnDescription
*Desc,
E.isStreamEof() ? ErrorFEof : ErrorFEof | ErrorFError;
StateFailed = E.setStreamState(
StateFailed, StreamState::getOpened(Desc, NewES, !NewES.isFEof()));
+ // On failure, the content of the buffer is undefined.
+ if (auto NewLinePtr = getPointeeDefVal(Call.getArgSVal(0), State)) {
+StateFailed = StateFailed->bindLoc(*NewLinePtr, UndefinedVal(),
+ C.getLocationContext());
+ }
alejandro-alvarez-sonarsource wrote:
Yes, in
[`test_getline_no_return_check()`](https://github.com/llvm/llvm-project/pull/83027/files#diff-243f1fa5dbb855f53ee0b05c1da2383352ec55ee9375a214c477df00592f1737R183)
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -1183,6 +1315,20 @@ void StreamChecker::evalGetdelim(const FnDescription
*Desc,
State->BindExpr(E.CE, C.getLocationContext(), RetVal);
StateNotFailed =
E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal, E.getZeroVal(Call));
+// The buffer size `*n` must be enough to hold the whole line, and
+// greater than the return value, since it has to account for '\0'.
+auto SizePtrSval = Call.getArgSVal(1);
+auto NVal = getPointeeDefVal(SizePtrSval, State);
+if (NVal) {
+ StateNotFailed = StateNotFailed->assume(
+ E.SVB
+ .evalBinOp(StateNotFailed, BinaryOperator::Opcode::BO_GT, *NVal,
+ RetVal, E.SVB.getConditionType())
+ .castAs(),
+ true);
alejandro-alvarez-sonarsource wrote:
It doesn't, since the `n` got invalidated first. Anyway, I have added two more
tests.
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83027
From 5c919832f9176d4b1af1312a4ee7cf30b788958a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 21 Feb 2024 14:46:01 +0100
Subject: [PATCH 1/6] [clang][analyzer] Model getline/getdelim preconditions
1. lineptr, n and stream can not be NULL
2. if *lineptr is NULL, *n must be 0
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 155 -
clang/test/Analysis/getline-stream.c | 327 ++
2 files changed, 480 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Analysis/getline-stream.c
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 2ec47bf55df76b..bacac7613f880c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/Sequence.h"
#include
#include
@@ -234,6 +235,9 @@ class StreamChecker : public Checker();
+ if (!Ptr)
+return nullptr;
+
+ assert(PtrExpr && "Expected an argument");
+
+ const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
+ if (!PtrNotNull && PtrNull) {
+if (ExplodedNode *N = C.generateErrorNode(PtrNull)) {
+ SmallString<256> buf;
+ llvm::raw_svector_ostream os(buf);
+ os << PtrDescr << " pointer might be NULL.";
+
+ auto R = std::make_unique(BT_SizeNull, buf, N);
+ bugreporter::trackExpressionValue(N, PtrExpr, *R);
+ C.emitReport(std::move(R));
+}
+return nullptr;
+ }
+
+ return PtrNotNull;
+}
+
+ProgramStateRef StreamChecker::ensureSizeZeroIfLineNull(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+ static constexpr char SizeNotZeroMsg[] =
+ "Line pointer might be null while n value is not zero";
+
+ // We have a pointer to a pointer to the buffer, and a pointer to the size.
+ // We want what they point at.
+ auto LinePtrSVal = getPointeeDefVal(LinePtrPtrSVal, State);
+ auto NSVal = getPointeeDefVal(SizePtrSVal, State);
+ if (!LinePtrSVal || !NSVal)
+return nullptr;
+
+ assert(LinePtrPtrExpr &&
+ "Expected an argument with a pointer to a pointer to the buffer.");
+ assert(SizePtrExpr &&
+ "Expected an argument with a pointer to the buffer size.");
+
+ // If the line pointer is null, and n is > 0, there is UB.
+ const auto [LinePtrNotNull, LinePtrNull] = State->assume(*LinePtrSVal);
+ if (LinePtrNull && !LinePtrNotNull) {
+const auto [NIsNotZero, NIsZero] = LinePtrNull->assume(*NSVal);
+if (NIsNotZero && !NIsZero) {
+ if (ExplodedNode *N = C.generateErrorNode(NIsNotZero)) {
+auto R = std::make_unique(BT_SizeNotZero,
+ SizeNotZeroMsg, N);
+bugreporter::trackExpressionValue(N, SizePtrExpr, *R);
+bugreporter::trackExpressionValue(N, LinePtrPtrExpr, *R);
+C.emitReport(std::move(R));
+ }
+ return nullptr;
+}
+return NIsZero;
+ }
+ return LinePtrNotNull;
+}
+
+void StreamChecker::preGetdelim(const FnDescription *Desc,
+const CallEvent &Call,
+CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+
+ auto AddTransitionOnReturn = llvm::make_scope_exit([&] {
+if (State != nullptr) {
+ C.addTransition(State);
+}
+ });
+
+ State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
+ State);
+ if (!State)
+return;
+ State = ensureStreamOpened(StreamVal, C, State);
+ if (!State)
+return;
+ State = ensureNoFilePositionIndeterminate(StreamVal, C, State);
+ if (!State)
+return;
+
+ // n must not be NULL
+ SVal SizePtrSval = Call.getArgSVal(1);
+ State = ensurePtrNotNull(SizePtrSval, Call.getArgExpr(1), C, State, "Size");
+ if (!State)
+return;
+
+ // lineptr must not be NULL
+ SVal LinePtrPtrSVal = Call.getArgSVal(0);
+ State =
+ ensurePtrNotNull(LinePtrPtrSVal, Call.getArgExpr(0), C, State, "Line");
+ if (!State)
+return;
+
+ // If lineptr points to a NULL pointer, *n must be 0
+ State =
+ ensureSizeZeroIfLineNull(LinePtrPtrSVal, SizePtrSval, Call.getArgExpr(0),
+ Call.getArgExpr(1), C, State);
+ if (!State)
+return;
+
+ SymbolRef Sym = StreamVal.getAsSymbol();
+ if (Sym && State->get(Sym)) {
+const StreamState *SS = State->get(Sym);
+if (SS->ErrorState & ErrorFEof)
+ reportFEofWarning(Sym, C, State);
+ } else
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83027
From 5c919832f9176d4b1af1312a4ee7cf30b788958a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 21 Feb 2024 14:46:01 +0100
Subject: [PATCH 1/7] [clang][analyzer] Model getline/getdelim preconditions
1. lineptr, n and stream can not be NULL
2. if *lineptr is NULL, *n must be 0
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 155 -
clang/test/Analysis/getline-stream.c | 327 ++
2 files changed, 480 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Analysis/getline-stream.c
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 2ec47bf55df76b..bacac7613f880c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/Sequence.h"
#include
#include
@@ -234,6 +235,9 @@ class StreamChecker : public Checker();
+ if (!Ptr)
+return nullptr;
+
+ assert(PtrExpr && "Expected an argument");
+
+ const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
+ if (!PtrNotNull && PtrNull) {
+if (ExplodedNode *N = C.generateErrorNode(PtrNull)) {
+ SmallString<256> buf;
+ llvm::raw_svector_ostream os(buf);
+ os << PtrDescr << " pointer might be NULL.";
+
+ auto R = std::make_unique(BT_SizeNull, buf, N);
+ bugreporter::trackExpressionValue(N, PtrExpr, *R);
+ C.emitReport(std::move(R));
+}
+return nullptr;
+ }
+
+ return PtrNotNull;
+}
+
+ProgramStateRef StreamChecker::ensureSizeZeroIfLineNull(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+ static constexpr char SizeNotZeroMsg[] =
+ "Line pointer might be null while n value is not zero";
+
+ // We have a pointer to a pointer to the buffer, and a pointer to the size.
+ // We want what they point at.
+ auto LinePtrSVal = getPointeeDefVal(LinePtrPtrSVal, State);
+ auto NSVal = getPointeeDefVal(SizePtrSVal, State);
+ if (!LinePtrSVal || !NSVal)
+return nullptr;
+
+ assert(LinePtrPtrExpr &&
+ "Expected an argument with a pointer to a pointer to the buffer.");
+ assert(SizePtrExpr &&
+ "Expected an argument with a pointer to the buffer size.");
+
+ // If the line pointer is null, and n is > 0, there is UB.
+ const auto [LinePtrNotNull, LinePtrNull] = State->assume(*LinePtrSVal);
+ if (LinePtrNull && !LinePtrNotNull) {
+const auto [NIsNotZero, NIsZero] = LinePtrNull->assume(*NSVal);
+if (NIsNotZero && !NIsZero) {
+ if (ExplodedNode *N = C.generateErrorNode(NIsNotZero)) {
+auto R = std::make_unique(BT_SizeNotZero,
+ SizeNotZeroMsg, N);
+bugreporter::trackExpressionValue(N, SizePtrExpr, *R);
+bugreporter::trackExpressionValue(N, LinePtrPtrExpr, *R);
+C.emitReport(std::move(R));
+ }
+ return nullptr;
+}
+return NIsZero;
+ }
+ return LinePtrNotNull;
+}
+
+void StreamChecker::preGetdelim(const FnDescription *Desc,
+const CallEvent &Call,
+CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+
+ auto AddTransitionOnReturn = llvm::make_scope_exit([&] {
+if (State != nullptr) {
+ C.addTransition(State);
+}
+ });
+
+ State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
+ State);
+ if (!State)
+return;
+ State = ensureStreamOpened(StreamVal, C, State);
+ if (!State)
+return;
+ State = ensureNoFilePositionIndeterminate(StreamVal, C, State);
+ if (!State)
+return;
+
+ // n must not be NULL
+ SVal SizePtrSval = Call.getArgSVal(1);
+ State = ensurePtrNotNull(SizePtrSval, Call.getArgExpr(1), C, State, "Size");
+ if (!State)
+return;
+
+ // lineptr must not be NULL
+ SVal LinePtrPtrSVal = Call.getArgSVal(0);
+ State =
+ ensurePtrNotNull(LinePtrPtrSVal, Call.getArgExpr(0), C, State, "Line");
+ if (!State)
+return;
+
+ // If lineptr points to a NULL pointer, *n must be 0
+ State =
+ ensureSizeZeroIfLineNull(LinePtrPtrSVal, SizePtrSval, Call.getArgExpr(0),
+ Call.getArgExpr(1), C, State);
+ if (!State)
+return;
+
+ SymbolRef Sym = StreamVal.getAsSymbol();
+ if (Sym && State->get(Sym)) {
+const StreamState *SS = State->get(Sym);
+if (SS->ErrorState & ErrorFEof)
+ reportFEofWarning(Sym, C, State);
+ } else
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
alejandro-alvarez-sonarsource wrote: > : "If *n is non-zero, the application shall ensure that *lineptr either > points to an object of size at least *n bytes, or is a null pointer." Ah! I was following at the 2008 version, and there "[...], or is a null pointer." is missing. I will update accordingly. I wonder, though, if the pointer is NULL, would an undefined (as non-initialized) *n be acceptable? https://github.com/llvm/llvm-project/pull/83027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -1158,6 +1173,118 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent &Call,
C.addTransition(StateFailed);
}
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+CheckerContext &C, ProgramStateRef State,
+const StringRef PtrDescr) const {
+ const auto Ptr = PtrVal.getAs();
+ if (!Ptr)
+return nullptr;
+
+ assert(PtrExpr && "Expected an argument");
+
+ const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
+ if (!PtrNotNull && PtrNull) {
+if (ExplodedNode *N = C.generateErrorNode(PtrNull)) {
+ auto R = std::make_unique(
+ BT_SizeNull, (PtrDescr + " pointer might be NULL.").str(), N);
+ bugreporter::trackExpressionValue(N, PtrExpr, *R);
+ C.emitReport(std::move(R));
+}
+return nullptr;
+ }
+
+ return PtrNotNull;
+}
+
+ProgramStateRef StreamChecker::ensureSizeZeroIfLineNull(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+ static constexpr char SizeNotZeroMsg[] =
+ "Line pointer might be null while n value is not zero";
alejandro-alvarez-sonarsource wrote:
I have followed this advise after changing the function to follow POSIX 2018.
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -234,6 +235,9 @@ class StreamChecker : public Checkerhttps://github.com/llvm/llvm-project/pull/83027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83027
From 5c919832f9176d4b1af1312a4ee7cf30b788958a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 21 Feb 2024 14:46:01 +0100
Subject: [PATCH 01/14] [clang][analyzer] Model getline/getdelim preconditions
1. lineptr, n and stream can not be NULL
2. if *lineptr is NULL, *n must be 0
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 155 -
clang/test/Analysis/getline-stream.c | 327 ++
2 files changed, 480 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Analysis/getline-stream.c
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 2ec47bf55df76b..bacac7613f880c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/Sequence.h"
#include
#include
@@ -234,6 +235,9 @@ class StreamChecker : public Checker();
+ if (!Ptr)
+return nullptr;
+
+ assert(PtrExpr && "Expected an argument");
+
+ const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
+ if (!PtrNotNull && PtrNull) {
+if (ExplodedNode *N = C.generateErrorNode(PtrNull)) {
+ SmallString<256> buf;
+ llvm::raw_svector_ostream os(buf);
+ os << PtrDescr << " pointer might be NULL.";
+
+ auto R = std::make_unique(BT_SizeNull, buf, N);
+ bugreporter::trackExpressionValue(N, PtrExpr, *R);
+ C.emitReport(std::move(R));
+}
+return nullptr;
+ }
+
+ return PtrNotNull;
+}
+
+ProgramStateRef StreamChecker::ensureSizeZeroIfLineNull(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+ static constexpr char SizeNotZeroMsg[] =
+ "Line pointer might be null while n value is not zero";
+
+ // We have a pointer to a pointer to the buffer, and a pointer to the size.
+ // We want what they point at.
+ auto LinePtrSVal = getPointeeDefVal(LinePtrPtrSVal, State);
+ auto NSVal = getPointeeDefVal(SizePtrSVal, State);
+ if (!LinePtrSVal || !NSVal)
+return nullptr;
+
+ assert(LinePtrPtrExpr &&
+ "Expected an argument with a pointer to a pointer to the buffer.");
+ assert(SizePtrExpr &&
+ "Expected an argument with a pointer to the buffer size.");
+
+ // If the line pointer is null, and n is > 0, there is UB.
+ const auto [LinePtrNotNull, LinePtrNull] = State->assume(*LinePtrSVal);
+ if (LinePtrNull && !LinePtrNotNull) {
+const auto [NIsNotZero, NIsZero] = LinePtrNull->assume(*NSVal);
+if (NIsNotZero && !NIsZero) {
+ if (ExplodedNode *N = C.generateErrorNode(NIsNotZero)) {
+auto R = std::make_unique(BT_SizeNotZero,
+ SizeNotZeroMsg, N);
+bugreporter::trackExpressionValue(N, SizePtrExpr, *R);
+bugreporter::trackExpressionValue(N, LinePtrPtrExpr, *R);
+C.emitReport(std::move(R));
+ }
+ return nullptr;
+}
+return NIsZero;
+ }
+ return LinePtrNotNull;
+}
+
+void StreamChecker::preGetdelim(const FnDescription *Desc,
+const CallEvent &Call,
+CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+
+ auto AddTransitionOnReturn = llvm::make_scope_exit([&] {
+if (State != nullptr) {
+ C.addTransition(State);
+}
+ });
+
+ State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
+ State);
+ if (!State)
+return;
+ State = ensureStreamOpened(StreamVal, C, State);
+ if (!State)
+return;
+ State = ensureNoFilePositionIndeterminate(StreamVal, C, State);
+ if (!State)
+return;
+
+ // n must not be NULL
+ SVal SizePtrSval = Call.getArgSVal(1);
+ State = ensurePtrNotNull(SizePtrSval, Call.getArgExpr(1), C, State, "Size");
+ if (!State)
+return;
+
+ // lineptr must not be NULL
+ SVal LinePtrPtrSVal = Call.getArgSVal(0);
+ State =
+ ensurePtrNotNull(LinePtrPtrSVal, Call.getArgExpr(0), C, State, "Line");
+ if (!State)
+return;
+
+ // If lineptr points to a NULL pointer, *n must be 0
+ State =
+ ensureSizeZeroIfLineNull(LinePtrPtrSVal, SizePtrSval, Call.getArgExpr(0),
+ Call.getArgExpr(1), C, State);
+ if (!State)
+return;
+
+ SymbolRef Sym = StreamVal.getAsSymbol();
+ if (Sym && State->get(Sym)) {
+const StreamState *SS = State->get(Sym);
+if (SS->ErrorState & ErrorFEof)
+ reportFEofWarning(Sym, C, State);
+ } els
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -1158,6 +1173,118 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent &Call,
C.addTransition(StateFailed);
}
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+CheckerContext &C, ProgramStateRef State,
+const StringRef PtrDescr) const {
+ const auto Ptr = PtrVal.getAs();
+ if (!Ptr)
+return nullptr;
+
+ assert(PtrExpr && "Expected an argument");
+
+ const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
+ if (!PtrNotNull && PtrNull) {
+if (ExplodedNode *N = C.generateErrorNode(PtrNull)) {
+ auto R = std::make_unique(
+ BT_SizeNull, (PtrDescr + " pointer might be NULL.").str(), N);
+ bugreporter::trackExpressionValue(N, PtrExpr, *R);
+ C.emitReport(std::move(R));
+}
+return nullptr;
+ }
+
+ return PtrNotNull;
+}
+
+ProgramStateRef StreamChecker::ensureSizeZeroIfLineNull(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+ static constexpr char SizeNotZeroMsg[] =
+ "Line pointer might be null while n value is not zero";
+
+ // We have a pointer to a pointer to the buffer, and a pointer to the size.
+ // We want what they point at.
+ auto LinePtrSVal = getPointeeDefVal(LinePtrPtrSVal, State);
+ auto NSVal = getPointeeDefVal(SizePtrSVal, State);
+ if (!LinePtrSVal || !NSVal)
+return nullptr;
+
+ assert(LinePtrPtrExpr &&
+ "Expected an argument with a pointer to a pointer to the buffer.");
+ assert(SizePtrExpr &&
+ "Expected an argument with a pointer to the buffer size.");
alejandro-alvarez-sonarsource wrote:
Simplified to a single assert.
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -1158,6 +1173,118 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent &Call,
C.addTransition(StateFailed);
}
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+CheckerContext &C, ProgramStateRef State,
+const StringRef PtrDescr) const {
+ const auto Ptr = PtrVal.getAs();
+ if (!Ptr)
+return nullptr;
+
+ assert(PtrExpr && "Expected an argument");
+
+ const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
+ if (!PtrNotNull && PtrNull) {
+if (ExplodedNode *N = C.generateErrorNode(PtrNull)) {
+ auto R = std::make_unique(
+ BT_SizeNull, (PtrDescr + " pointer might be NULL.").str(), N);
+ bugreporter::trackExpressionValue(N, PtrExpr, *R);
+ C.emitReport(std::move(R));
+}
+return nullptr;
+ }
+
+ return PtrNotNull;
+}
+
+ProgramStateRef StreamChecker::ensureSizeZeroIfLineNull(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+ static constexpr char SizeNotZeroMsg[] =
+ "Line pointer might be null while n value is not zero";
+
+ // We have a pointer to a pointer to the buffer, and a pointer to the size.
+ // We want what they point at.
+ auto LinePtrSVal = getPointeeDefVal(LinePtrPtrSVal, State);
+ auto NSVal = getPointeeDefVal(SizePtrSVal, State);
+ if (!LinePtrSVal || !NSVal)
+return nullptr;
+
+ assert(LinePtrPtrExpr &&
+ "Expected an argument with a pointer to a pointer to the buffer.");
+ assert(SizePtrExpr &&
+ "Expected an argument with a pointer to the buffer size.");
+
+ // If the line pointer is null, and n is > 0, there is UB.
+ const auto [LinePtrNotNull, LinePtrNull] = State->assume(*LinePtrSVal);
+ if (LinePtrNull && !LinePtrNotNull) {
+const auto [NIsNotZero, NIsZero] = LinePtrNull->assume(*NSVal);
+if (NIsNotZero && !NIsZero) {
+ if (ExplodedNode *N = C.generateErrorNode(NIsNotZero)) {
+auto R = std::make_unique(BT_SizeNotZero,
+ SizeNotZeroMsg, N);
+bugreporter::trackExpressionValue(N, SizePtrExpr, *R);
+bugreporter::trackExpressionValue(N, LinePtrPtrExpr, *R);
+C.emitReport(std::move(R));
+ }
+ return nullptr;
+}
+return NIsZero;
+ }
+ return LinePtrNotNull;
+}
+
+void StreamChecker::preGetdelim(const FnDescription *Desc,
+const CallEvent &Call,
+CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+
+ auto AddTransitionOnReturn = llvm::make_scope_exit([&] {
alejandro-alvarez-sonarsource wrote:
Indeed. This is a leftover from unrelated downstream changes. I have removed it.
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -1158,6 +1173,118 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent &Call,
C.addTransition(StateFailed);
}
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+CheckerContext &C, ProgramStateRef State,
+const StringRef PtrDescr) const {
alejandro-alvarez-sonarsource wrote:
`ensureStreamNonNull` now calls `ensurePtrNotNull`, so the logic is shared.
https://github.com/llvm/llvm-project/pull/83027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
@@ -510,6 +517,14 @@ class StreamChecker : public Checkerhttps://github.com/llvm/llvm-project/pull/83027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
alejandro-alvarez-sonarsource wrote: > Additionally, the checked preconditions look not exact. For example the POSIX > documentation for `getdelim` says: "If *n is non-zero, the application shall > ensure that *lineptr either points to an object of size at least *n bytes, or > is a null pointer." This means `*lineptr` can be NULL when `*n` is a nonzero > value. The buffer size of `*lineptr` could be checked that is at least `*n` > (if `*lineptr` is not NULL). With 9db5a4a261655c6825cf83c3ace545129060b7df now this behavior is modeled. As for where to model the preconditions. `StdLibraryFunctionsChecker` actually has a comment about these functions: ``` // FIXME these are actually defined by POSIX and not by the C standard, we // should handle them together with the rest of the POSIX functions. ``` So, it seems removing them from `StdLibraryFunctionsChecker` is not out of the question. We can leave them together with other stream functions, or we could move them to `UnixAPIChecker`, which we have enabled downstream. I think the latter is a reasonable compromise so `StreamChecker` scope is the stream itself, and not everything surrounding the `FILE*` APIs. https://github.com/llvm/llvm-project/pull/83027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] StreamChecker: Add more APIs, invalidate fscanf args (PR #82476)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/82476
From a21881d82fe3674b344d4a3807e9d2590c98ce93 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Tue, 14 Nov 2023 09:28:45 +0100
Subject: [PATCH 1/4] [clang][analyzer] StreamChecker: add more APIs,
invalidate fscanf args
1. Model getc, vfscanf, putc, vfprintf.
2. fscanf invalidates all arguments after the format string.
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 39 +-
...ystem-header-simulator-for-simple-stream.h | 2 +-
.../system-header-simulator-for-valist.h | 4 +
.../Analysis/Inputs/system-header-simulator.h | 3 +
clang/test/Analysis/stream.c | 128 ++
5 files changed, 174 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index a070f451694a3b..7938a0d30a91a3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,8 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/SmallVector.h"
#include
#include
@@ -171,7 +173,7 @@ using FnCheck = std::function;
using ArgNoTy = unsigned int;
-static const ArgNoTy ArgNone = std::numeric_limits::max();
+const ArgNoTy ArgNone = std::numeric_limits::max();
struct FnDescription {
FnCheck PreFn;
@@ -179,6 +181,26 @@ struct FnDescription {
ArgNoTy StreamArgNo;
};
+[[nodiscard]] ProgramStateRef
+escapeArgsAfterIndex(ProgramStateRef State, CheckerContext &C,
+ const CallEvent &Call, unsigned FirstEscapingArgIndex) {
+ const auto *CE = Call.getOriginExpr();
+ assert(CE);
+
+ if (Call.getNumArgs() <= FirstEscapingArgIndex)
+return State;
+
+ SmallVector EscapingArgs;
+ EscapingArgs.reserve(Call.getNumArgs() - FirstEscapingArgIndex);
+ for (auto EscArgIdx :
+ llvm::seq(FirstEscapingArgIndex, Call.getNumArgs()))
+EscapingArgs.push_back(Call.getArgSVal(EscArgIdx));
+ State = State->invalidateRegions(EscapingArgs, CE, C.blockCount(),
+ C.getLocationContext(),
+ /*CausesPointerEscape=*/false);
+ return State;
+}
+
/// Get the value of the stream argument out of the passed call event.
/// The call should contain a function that is described by Desc.
SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
@@ -396,6 +418,18 @@ class StreamChecker : public Checker FnTestDescriptions = {
@@ -997,6 +1031,9 @@ void StreamChecker::evalFscanf(const FnDescription *Desc,
const CallEvent &Call,
if (!E.Init(Desc, Call, C, State))
return;
+ // The pointers passed to fscanf escape and get invalidated.
+ State = escapeArgsAfterIndex(State, C, Call, /*FirstEscapingArgIndex=*/2);
+
// Add the success state.
// In this context "success" means there is not an EOF or other read error
// before any item is matched in 'fscanf'. But there may be match failure,
diff --git
a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
index 098a2208fecbe9..c26d3582149120 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
@@ -5,7 +5,7 @@
// suppressed.
#pragma clang system_header
-typedef struct __sFILE {
+typedef struct _FILE {
unsigned char *_p;
} FILE;
FILE *fopen(const char *restrict, const char *restrict) __asm("_" "fopen" );
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
index 7299b61353d460..87688bd8b312f4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
@@ -10,6 +10,8 @@
#define restrict /*restrict*/
#endif
+typedef struct _FILE FILE;
+
typedef __builtin_va_list va_list;
#define va_start(ap, param) __builtin_va_start(ap, param)
@@ -21,6 +23,8 @@ int vprintf (const char *restrict format, va_list arg);
int vsprintf (char *restrict s, const char *restrict format, va_list arg);
+int vfscanf(FILE *stream, const char *format, va_list ap);
+
int some_library_function(int n, va_list arg);
// No warning from system header.
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h
b/clang/test/Analysis/Inputs/system-header-simulator.h
index 15986984802c0e..8fd51449ecc0a4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -73,6 +73,9 @@ int ferror(FILE *stream);
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
https://github.com/alejandro-alvarez-sonarsource edited https://github.com/llvm/llvm-project/pull/82476 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/82476
From a21881d82fe3674b344d4a3807e9d2590c98ce93 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Tue, 14 Nov 2023 09:28:45 +0100
Subject: [PATCH 1/5] [clang][analyzer] StreamChecker: add more APIs,
invalidate fscanf args
1. Model getc, vfscanf, putc, vfprintf.
2. fscanf invalidates all arguments after the format string.
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 39 +-
...ystem-header-simulator-for-simple-stream.h | 2 +-
.../system-header-simulator-for-valist.h | 4 +
.../Analysis/Inputs/system-header-simulator.h | 3 +
clang/test/Analysis/stream.c | 128 ++
5 files changed, 174 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index a070f451694a3b..7938a0d30a91a3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,8 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/SmallVector.h"
#include
#include
@@ -171,7 +173,7 @@ using FnCheck = std::function;
using ArgNoTy = unsigned int;
-static const ArgNoTy ArgNone = std::numeric_limits::max();
+const ArgNoTy ArgNone = std::numeric_limits::max();
struct FnDescription {
FnCheck PreFn;
@@ -179,6 +181,26 @@ struct FnDescription {
ArgNoTy StreamArgNo;
};
+[[nodiscard]] ProgramStateRef
+escapeArgsAfterIndex(ProgramStateRef State, CheckerContext &C,
+ const CallEvent &Call, unsigned FirstEscapingArgIndex) {
+ const auto *CE = Call.getOriginExpr();
+ assert(CE);
+
+ if (Call.getNumArgs() <= FirstEscapingArgIndex)
+return State;
+
+ SmallVector EscapingArgs;
+ EscapingArgs.reserve(Call.getNumArgs() - FirstEscapingArgIndex);
+ for (auto EscArgIdx :
+ llvm::seq(FirstEscapingArgIndex, Call.getNumArgs()))
+EscapingArgs.push_back(Call.getArgSVal(EscArgIdx));
+ State = State->invalidateRegions(EscapingArgs, CE, C.blockCount(),
+ C.getLocationContext(),
+ /*CausesPointerEscape=*/false);
+ return State;
+}
+
/// Get the value of the stream argument out of the passed call event.
/// The call should contain a function that is described by Desc.
SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
@@ -396,6 +418,18 @@ class StreamChecker : public Checker FnTestDescriptions = {
@@ -997,6 +1031,9 @@ void StreamChecker::evalFscanf(const FnDescription *Desc,
const CallEvent &Call,
if (!E.Init(Desc, Call, C, State))
return;
+ // The pointers passed to fscanf escape and get invalidated.
+ State = escapeArgsAfterIndex(State, C, Call, /*FirstEscapingArgIndex=*/2);
+
// Add the success state.
// In this context "success" means there is not an EOF or other read error
// before any item is matched in 'fscanf'. But there may be match failure,
diff --git
a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
index 098a2208fecbe9..c26d3582149120 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
@@ -5,7 +5,7 @@
// suppressed.
#pragma clang system_header
-typedef struct __sFILE {
+typedef struct _FILE {
unsigned char *_p;
} FILE;
FILE *fopen(const char *restrict, const char *restrict) __asm("_" "fopen" );
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
index 7299b61353d460..87688bd8b312f4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
@@ -10,6 +10,8 @@
#define restrict /*restrict*/
#endif
+typedef struct _FILE FILE;
+
typedef __builtin_va_list va_list;
#define va_start(ap, param) __builtin_va_start(ap, param)
@@ -21,6 +23,8 @@ int vprintf (const char *restrict format, va_list arg);
int vsprintf (char *restrict s, const char *restrict format, va_list arg);
+int vfscanf(FILE *stream, const char *format, va_list ap);
+
int some_library_function(int n, va_list arg);
// No warning from system header.
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h
b/clang/test/Analysis/Inputs/system-header-simulator.h
index 15986984802c0e..8fd51449ecc0a4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -73,6 +73,9 @@ int ferror(FILE *stream);
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/82476
From a21881d82fe3674b344d4a3807e9d2590c98ce93 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Tue, 14 Nov 2023 09:28:45 +0100
Subject: [PATCH 1/5] [clang][analyzer] StreamChecker: add more APIs,
invalidate fscanf args
1. Model getc, vfscanf, putc, vfprintf.
2. fscanf invalidates all arguments after the format string.
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 39 +-
...ystem-header-simulator-for-simple-stream.h | 2 +-
.../system-header-simulator-for-valist.h | 4 +
.../Analysis/Inputs/system-header-simulator.h | 3 +
clang/test/Analysis/stream.c | 128 ++
5 files changed, 174 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index a070f451694a3b..7938a0d30a91a3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,8 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/SmallVector.h"
#include
#include
@@ -171,7 +173,7 @@ using FnCheck = std::function;
using ArgNoTy = unsigned int;
-static const ArgNoTy ArgNone = std::numeric_limits::max();
+const ArgNoTy ArgNone = std::numeric_limits::max();
struct FnDescription {
FnCheck PreFn;
@@ -179,6 +181,26 @@ struct FnDescription {
ArgNoTy StreamArgNo;
};
+[[nodiscard]] ProgramStateRef
+escapeArgsAfterIndex(ProgramStateRef State, CheckerContext &C,
+ const CallEvent &Call, unsigned FirstEscapingArgIndex) {
+ const auto *CE = Call.getOriginExpr();
+ assert(CE);
+
+ if (Call.getNumArgs() <= FirstEscapingArgIndex)
+return State;
+
+ SmallVector EscapingArgs;
+ EscapingArgs.reserve(Call.getNumArgs() - FirstEscapingArgIndex);
+ for (auto EscArgIdx :
+ llvm::seq(FirstEscapingArgIndex, Call.getNumArgs()))
+EscapingArgs.push_back(Call.getArgSVal(EscArgIdx));
+ State = State->invalidateRegions(EscapingArgs, CE, C.blockCount(),
+ C.getLocationContext(),
+ /*CausesPointerEscape=*/false);
+ return State;
+}
+
/// Get the value of the stream argument out of the passed call event.
/// The call should contain a function that is described by Desc.
SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
@@ -396,6 +418,18 @@ class StreamChecker : public Checker FnTestDescriptions = {
@@ -997,6 +1031,9 @@ void StreamChecker::evalFscanf(const FnDescription *Desc,
const CallEvent &Call,
if (!E.Init(Desc, Call, C, State))
return;
+ // The pointers passed to fscanf escape and get invalidated.
+ State = escapeArgsAfterIndex(State, C, Call, /*FirstEscapingArgIndex=*/2);
+
// Add the success state.
// In this context "success" means there is not an EOF or other read error
// before any item is matched in 'fscanf'. But there may be match failure,
diff --git
a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
index 098a2208fecbe9..c26d3582149120 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
@@ -5,7 +5,7 @@
// suppressed.
#pragma clang system_header
-typedef struct __sFILE {
+typedef struct _FILE {
unsigned char *_p;
} FILE;
FILE *fopen(const char *restrict, const char *restrict) __asm("_" "fopen" );
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
index 7299b61353d460..87688bd8b312f4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
@@ -10,6 +10,8 @@
#define restrict /*restrict*/
#endif
+typedef struct _FILE FILE;
+
typedef __builtin_va_list va_list;
#define va_start(ap, param) __builtin_va_start(ap, param)
@@ -21,6 +23,8 @@ int vprintf (const char *restrict format, va_list arg);
int vsprintf (char *restrict s, const char *restrict format, va_list arg);
+int vfscanf(FILE *stream, const char *format, va_list ap);
+
int some_library_function(int n, va_list arg);
// No warning from system header.
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h
b/clang/test/Analysis/Inputs/system-header-simulator.h
index 15986984802c0e..8fd51449ecc0a4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -73,6 +73,9 @@ int ferror(FILE *stream);
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
https://github.com/alejandro-alvarez-sonarsource edited https://github.com/llvm/llvm-project/pull/82476 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
alejandro-alvarez-sonarsource wrote: I have reduced the scope of the PR, and changed the title and the description accordingly. https://github.com/llvm/llvm-project/pull/82476 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource created
https://github.com/llvm/llvm-project/pull/83027
1. `lineptr`, `n` and `stream` can not be `NULL`
2. if `*lineptr` is `NULL`, `*n` must be 0
This patch models `getline` specific preconditions, constraints the size to be
greater than the return value on success --- since the former must include the
null terminator ---, and sets the buffer to uninitialized on failure --- since
it may be a freshly allocated memory area. The modeling of the allocating
behavior affects `MallocChecker`, for which I plan to submit a separate PR,
self-contained and independent of this one.
From 5e62483adf6620d481bdb873dd91f81b8a95f6fc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 21 Feb 2024 14:46:01 +0100
Subject: [PATCH] [clang][analyzer] Model getline/getdelim preconditions
1. lineptr, n and stream can not be NULL
2. if *lineptr is NULL, *n must be 0
---
.../Core/PathSensitive/CheckerHelpers.h | 6 +
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 155 -
.../StaticAnalyzer/Core/CheckerHelpers.cpp| 9 +
.../system-header-simulator-for-malloc.h | 1 +
clang/test/Analysis/getline-stream.c | 327 ++
5 files changed, 496 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Analysis/getline-stream.c
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
index 65982457ad8393..60421e5437d82f 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
@@ -13,6 +13,9 @@
#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
+#include "ProgramState_Fwd.h"
+#include "SVals.h"
+
#include "clang/AST/OperationKinds.h"
#include "clang/AST/Stmt.h"
#include "clang/Basic/OperatorKinds.h"
@@ -110,6 +113,9 @@ class OperatorKind {
OperatorKind operationKindFromOverloadedOperator(OverloadedOperatorKind OOK,
bool IsBinary);
+std::optional getPointeeDefVal(SVal PtrSVal,
+ProgramStateRef State);
+
} // namespace ento
} // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 65bdc4cac30940..b4b828784c9725 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/Sequence.h"
#include
#include
@@ -312,6 +313,9 @@ class StreamChecker : public Checker();
+ if (!Ptr)
+return nullptr;
+
+ assert(PtrExpr && "Expected an argument");
+
+ const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
+ if (!PtrNotNull && PtrNull) {
+if (ExplodedNode *N = C.generateErrorNode(PtrNull)) {
+ SmallString<256> buf;
+ llvm::raw_svector_ostream os(buf);
+ os << PtrDescr << " pointer might be NULL.";
+
+ auto R = std::make_unique(BT_SizeNull, buf, N);
+ bugreporter::trackExpressionValue(N, PtrExpr, *R);
+ C.emitReport(std::move(R));
+}
+return nullptr;
+ }
+
+ return PtrNotNull;
+}
+
+ProgramStateRef StreamChecker::ensureSizeZeroIfLineNull(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+ static constexpr char SizeNotZeroMsg[] =
+ "Line pointer might be null while n value is not zero";
+
+ // We have a pointer to a pointer to the buffer, and a pointer to the size.
+ // We want what they point at.
+ auto LinePtrSVal = getPointeeDefVal(LinePtrPtrSVal, State);
+ auto NSVal = getPointeeDefVal(SizePtrSVal, State);
+ if (!LinePtrSVal || !NSVal)
+return nullptr;
+
+ assert(LinePtrPtrExpr &&
+ "Expected an argument with a pointer to a pointer to the buffer.");
+ assert(SizePtrExpr &&
+ "Expected an argument with a pointer to the buffer size.");
+
+ // If the line pointer is null, and n is > 0, there is UB.
+ const auto [LinePtrNotNull, LinePtrNull] = State->assume(*LinePtrSVal);
+ if (LinePtrNull && !LinePtrNotNull) {
+const auto [NIsNotZero, NIsZero] = LinePtrNull->assume(*NSVal);
+if (NIsNotZero && !NIsZero) {
+ if (ExplodedNode *N = C.generateErrorNode(NIsNotZero)) {
+auto R = std::make_unique(BT_SizeNotZero,
+ SizeNotZeroMsg, N);
+bugreporter::trackExpressionValue(N, SizePtrExpr, *R);
+bugreporter::trackExp
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/82476
From a21881d82fe3674b344d4a3807e9d2590c98ce93 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Tue, 14 Nov 2023 09:28:45 +0100
Subject: [PATCH 1/7] [clang][analyzer] StreamChecker: add more APIs,
invalidate fscanf args
1. Model getc, vfscanf, putc, vfprintf.
2. fscanf invalidates all arguments after the format string.
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 39 +-
...ystem-header-simulator-for-simple-stream.h | 2 +-
.../system-header-simulator-for-valist.h | 4 +
.../Analysis/Inputs/system-header-simulator.h | 3 +
clang/test/Analysis/stream.c | 128 ++
5 files changed, 174 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index a070f451694a3b..7938a0d30a91a3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,8 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/SmallVector.h"
#include
#include
@@ -171,7 +173,7 @@ using FnCheck = std::function;
using ArgNoTy = unsigned int;
-static const ArgNoTy ArgNone = std::numeric_limits::max();
+const ArgNoTy ArgNone = std::numeric_limits::max();
struct FnDescription {
FnCheck PreFn;
@@ -179,6 +181,26 @@ struct FnDescription {
ArgNoTy StreamArgNo;
};
+[[nodiscard]] ProgramStateRef
+escapeArgsAfterIndex(ProgramStateRef State, CheckerContext &C,
+ const CallEvent &Call, unsigned FirstEscapingArgIndex) {
+ const auto *CE = Call.getOriginExpr();
+ assert(CE);
+
+ if (Call.getNumArgs() <= FirstEscapingArgIndex)
+return State;
+
+ SmallVector EscapingArgs;
+ EscapingArgs.reserve(Call.getNumArgs() - FirstEscapingArgIndex);
+ for (auto EscArgIdx :
+ llvm::seq(FirstEscapingArgIndex, Call.getNumArgs()))
+EscapingArgs.push_back(Call.getArgSVal(EscArgIdx));
+ State = State->invalidateRegions(EscapingArgs, CE, C.blockCount(),
+ C.getLocationContext(),
+ /*CausesPointerEscape=*/false);
+ return State;
+}
+
/// Get the value of the stream argument out of the passed call event.
/// The call should contain a function that is described by Desc.
SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
@@ -396,6 +418,18 @@ class StreamChecker : public Checker FnTestDescriptions = {
@@ -997,6 +1031,9 @@ void StreamChecker::evalFscanf(const FnDescription *Desc,
const CallEvent &Call,
if (!E.Init(Desc, Call, C, State))
return;
+ // The pointers passed to fscanf escape and get invalidated.
+ State = escapeArgsAfterIndex(State, C, Call, /*FirstEscapingArgIndex=*/2);
+
// Add the success state.
// In this context "success" means there is not an EOF or other read error
// before any item is matched in 'fscanf'. But there may be match failure,
diff --git
a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
index 098a2208fecbe9..c26d3582149120 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
@@ -5,7 +5,7 @@
// suppressed.
#pragma clang system_header
-typedef struct __sFILE {
+typedef struct _FILE {
unsigned char *_p;
} FILE;
FILE *fopen(const char *restrict, const char *restrict) __asm("_" "fopen" );
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
index 7299b61353d460..87688bd8b312f4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
@@ -10,6 +10,8 @@
#define restrict /*restrict*/
#endif
+typedef struct _FILE FILE;
+
typedef __builtin_va_list va_list;
#define va_start(ap, param) __builtin_va_start(ap, param)
@@ -21,6 +23,8 @@ int vprintf (const char *restrict format, va_list arg);
int vsprintf (char *restrict s, const char *restrict format, va_list arg);
+int vfscanf(FILE *stream, const char *format, va_list ap);
+
int some_library_function(int n, va_list arg);
// No warning from system header.
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h
b/clang/test/Analysis/Inputs/system-header-simulator.h
index 15986984802c0e..8fd51449ecc0a4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -73,6 +73,9 @@ int ferror(FILE *stream);
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
@@ -339,3 +363,138 @@ void fflush_on_open_failed_stream(void) {
}
fclose(F);
}
+
+void test_fscanf_eof() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ int a;
+ unsigned b;
+ int ret = fscanf(F1, "%d %u", &a, &b);
+ char c = fgetc(F1); // expected-warning {{Read function called when stream
is in EOF state. Function has no effect}}
+ // expected-warning@-1 {{File position of the stream might be
'indeterminate' after a failed operation. Can cause undefined behavior}}
+ fclose(F1);
+}
+
+void test_fscanf_escape() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ int a = 48;
+ unsigned b = 127;
+ char buffer[] = "FSCANF"; // 70 83 67 65 78 70
+
+ clang_analyzer_dump_int(a); // expected-warning {{48 S32b}}
+ clang_analyzer_dump_int(b); // expected-warning {{127 S32b}}
+ clang_analyzer_dump_char(buffer[2]); // expected-warning {{67 S8b}}
+
+ int ret = fscanf(F1, "%d %u %s", &a, &b, buffer);
+ if (ret != EOF) {
+clang_analyzer_dump_int(a); // expected-warning {{conj_$}}
+clang_analyzer_dump_int(b); // expected-warning {{conj_$}}
+clang_analyzer_dump_char(buffer[2]); // expected-warning {{derived_$}}
+ } else {
+clang_analyzer_dump_int(a); // expected-warning {{48 S32b}}
+clang_analyzer_dump_int(b); // expected-warning {{127 S32b}}
+clang_analyzer_dump_char(buffer[2]); // expected-warning {{67 S8b}}
+ }
+
+ if (ret != EOF) {
+char c = fgetc(F1); // ok
+ }
+
+ fclose(F1);
+}
+
+void test_fputc() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ char a = 'y'; // 'y' = 121 ASCII
+ char r = fputc(a, F1);
+ if (r != EOF) {
+clang_analyzer_dump_char(r); // expected-warning {{121 S8b}}
+char z = fgetc(F1);
+ } else {
+clang_analyzer_dump_char(r); // expected-warning {{-1 S8b}}
+ }
+
+ fclose(F1);
+}
+
+void test_fputs() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ char buffer[] = "HELLO";
+ int r = fputs(buffer, F1);
+ if (r >= 0) {
+// fputs does not invalidate the input buffer (72 is ascii for 'H')
+clang_analyzer_dump_char(buffer[0]); // expected-warning {{72 S8b}}
+ } else if (r == EOF) {
+// fputs does not invalidate the input buffer, *and* this branch
+// can happen
+clang_analyzer_dump_char(buffer[0]); // expected-warning {{72 S8b}}
+ } else {
+// This branch can not happen
+int *p = NULL;
+*p = 0;
+ }
+
+ fclose(F1);
+}
+
+void test_fprintf() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ unsigned a = 42;
+ char *output = "HELLO";
+ int r = fprintf(F1, "%s\t%u\n", output, a);
+ // fprintf does not invalidate any of its input
+ // 69 is ascii for 'E'
+ clang_analyzer_dump_int(a); // expected-warning {{42 S32b}}
+ clang_analyzer_dump_char(output[1]); // expected-warning {{69 S8b}}
+ if (r < 0) {
+// Failure
+fprintf(F1, "%s\t%u\n", output, a); // expected-warning {{File position of
the stream might be 'indeterminate' after a failed operation. Can cause
undefined behavior}}
+ } else {
+char buffer[10];
+fscanf(F1, "%s", buffer);
+if (fseek(F1, 0, SEEK_SET) == 0) {
+ fprintf(F1, "%s\t%u\n", buffer, a); // ok
+}
+ }
+
+ fclose(F1);
+}
+
+
+int test_vscanf_inner(const char *fmt, ...) {
alejandro-alvarez-sonarsource wrote:
Indeed, sorry for the typo.
https://github.com/llvm/llvm-project/pull/82476
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
@@ -339,3 +363,138 @@ void fflush_on_open_failed_stream(void) {
}
fclose(F);
}
+
+void test_fscanf_eof() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ int a;
+ unsigned b;
+ int ret = fscanf(F1, "%d %u", &a, &b);
+ char c = fgetc(F1); // expected-warning {{Read function called when stream
is in EOF state. Function has no effect}}
+ // expected-warning@-1 {{File position of the stream might be
'indeterminate' after a failed operation. Can cause undefined behavior}}
+ fclose(F1);
+}
+
+void test_fscanf_escape() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ int a = 48;
+ unsigned b = 127;
+ char buffer[] = "FSCANF"; // 70 83 67 65 78 70
+
+ clang_analyzer_dump_int(a); // expected-warning {{48 S32b}}
+ clang_analyzer_dump_int(b); // expected-warning {{127 S32b}}
+ clang_analyzer_dump_char(buffer[2]); // expected-warning {{67 S8b}}
+
+ int ret = fscanf(F1, "%d %u %s", &a, &b, buffer);
+ if (ret != EOF) {
+clang_analyzer_dump_int(a); // expected-warning {{conj_$}}
+clang_analyzer_dump_int(b); // expected-warning {{conj_$}}
+clang_analyzer_dump_char(buffer[2]); // expected-warning {{derived_$}}
+ } else {
+clang_analyzer_dump_int(a); // expected-warning {{48 S32b}}
+clang_analyzer_dump_int(b); // expected-warning {{127 S32b}}
+clang_analyzer_dump_char(buffer[2]); // expected-warning {{67 S8b}}
+ }
+
+ if (ret != EOF) {
+char c = fgetc(F1); // ok
+ }
+
+ fclose(F1);
+}
+
+void test_fputc() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ char a = 'y'; // 'y' = 121 ASCII
+ char r = fputc(a, F1);
+ if (r != EOF) {
+clang_analyzer_dump_char(r); // expected-warning {{121 S8b}}
+char z = fgetc(F1);
+ } else {
+clang_analyzer_dump_char(r); // expected-warning {{-1 S8b}}
+ }
+
+ fclose(F1);
+}
+
+void test_fputs() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ char buffer[] = "HELLO";
+ int r = fputs(buffer, F1);
+ if (r >= 0) {
+// fputs does not invalidate the input buffer (72 is ascii for 'H')
+clang_analyzer_dump_char(buffer[0]); // expected-warning {{72 S8b}}
+ } else if (r == EOF) {
+// fputs does not invalidate the input buffer, *and* this branch
+// can happen
+clang_analyzer_dump_char(buffer[0]); // expected-warning {{72 S8b}}
+ } else {
+// This branch can not happen
+int *p = NULL;
+*p = 0;
+ }
+
+ fclose(F1);
+}
+
+void test_fprintf() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ unsigned a = 42;
+ char *output = "HELLO";
+ int r = fprintf(F1, "%s\t%u\n", output, a);
+ // fprintf does not invalidate any of its input
+ // 69 is ascii for 'E'
+ clang_analyzer_dump_int(a); // expected-warning {{42 S32b}}
+ clang_analyzer_dump_char(output[1]); // expected-warning {{69 S8b}}
+ if (r < 0) {
+// Failure
+fprintf(F1, "%s\t%u\n", output, a); // expected-warning {{File position of
the stream might be 'indeterminate' after a failed operation. Can cause
undefined behavior}}
+ } else {
+char buffer[10];
+fscanf(F1, "%s", buffer);
+if (fseek(F1, 0, SEEK_SET) == 0) {
+ fprintf(F1, "%s\t%u\n", buffer, a); // ok
+}
+ }
+
+ fclose(F1);
+}
+
+
+int test_vscanf_inner(const char *fmt, ...) {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return EOF;
+
+ va_list ap;
+ va_start(ap, fmt);
+
+ int r = vfscanf(F1, fmt, ap);
+
+ fclose(F1);
+ va_end(ap);
+ return r;
+}
+
+void test_vscanf() {
alejandro-alvarez-sonarsource wrote:
Same typo, fixed.
https://github.com/llvm/llvm-project/pull/82476
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
@@ -339,3 +363,138 @@ void fflush_on_open_failed_stream(void) {
}
fclose(F);
}
+
+void test_fscanf_eof() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ int a;
+ unsigned b;
+ int ret = fscanf(F1, "%d %u", &a, &b);
+ char c = fgetc(F1); // expected-warning {{Read function called when stream
is in EOF state. Function has no effect}}
+ // expected-warning@-1 {{File position of the stream might be
'indeterminate' after a failed operation. Can cause undefined behavior}}
+ fclose(F1);
+}
+
+void test_fscanf_escape() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ int a = 48;
+ unsigned b = 127;
+ char buffer[] = "FSCANF"; // 70 83 67 65 78 70
+
+ clang_analyzer_dump_int(a); // expected-warning {{48 S32b}}
+ clang_analyzer_dump_int(b); // expected-warning {{127 S32b}}
+ clang_analyzer_dump_char(buffer[2]); // expected-warning {{67 S8b}}
+
+ int ret = fscanf(F1, "%d %u %s", &a, &b, buffer);
+ if (ret != EOF) {
+clang_analyzer_dump_int(a); // expected-warning {{conj_$}}
+clang_analyzer_dump_int(b); // expected-warning {{conj_$}}
+clang_analyzer_dump_char(buffer[2]); // expected-warning {{derived_$}}
+ } else {
+clang_analyzer_dump_int(a); // expected-warning {{48 S32b}}
+clang_analyzer_dump_int(b); // expected-warning {{127 S32b}}
+clang_analyzer_dump_char(buffer[2]); // expected-warning {{67 S8b}}
+ }
+
+ if (ret != EOF) {
+char c = fgetc(F1); // ok
+ }
+
+ fclose(F1);
+}
+
+void test_fputc() {
alejandro-alvarez-sonarsource wrote:
Removed since it is redundant.
https://github.com/llvm/llvm-project/pull/82476
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
@@ -339,3 +363,138 @@ void fflush_on_open_failed_stream(void) {
}
fclose(F);
}
+
+void test_fscanf_eof() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ int a;
+ unsigned b;
+ int ret = fscanf(F1, "%d %u", &a, &b);
+ char c = fgetc(F1); // expected-warning {{Read function called when stream
is in EOF state. Function has no effect}}
+ // expected-warning@-1 {{File position of the stream might be
'indeterminate' after a failed operation. Can cause undefined behavior}}
+ fclose(F1);
+}
+
+void test_fscanf_escape() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ int a = 48;
+ unsigned b = 127;
+ char buffer[] = "FSCANF"; // 70 83 67 65 78 70
+
+ clang_analyzer_dump_int(a); // expected-warning {{48 S32b}}
+ clang_analyzer_dump_int(b); // expected-warning {{127 S32b}}
+ clang_analyzer_dump_char(buffer[2]); // expected-warning {{67 S8b}}
+
+ int ret = fscanf(F1, "%d %u %s", &a, &b, buffer);
+ if (ret != EOF) {
+clang_analyzer_dump_int(a); // expected-warning {{conj_$}}
+clang_analyzer_dump_int(b); // expected-warning {{conj_$}}
+clang_analyzer_dump_char(buffer[2]); // expected-warning {{derived_$}}
+ } else {
+clang_analyzer_dump_int(a); // expected-warning {{48 S32b}}
+clang_analyzer_dump_int(b); // expected-warning {{127 S32b}}
+clang_analyzer_dump_char(buffer[2]); // expected-warning {{67 S8b}}
+ }
+
+ if (ret != EOF) {
+char c = fgetc(F1); // ok
+ }
+
+ fclose(F1);
+}
+
+void test_fputc() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ char a = 'y'; // 'y' = 121 ASCII
+ char r = fputc(a, F1);
+ if (r != EOF) {
+clang_analyzer_dump_char(r); // expected-warning {{121 S8b}}
+char z = fgetc(F1);
+ } else {
+clang_analyzer_dump_char(r); // expected-warning {{-1 S8b}}
+ }
+
+ fclose(F1);
+}
+
+void test_fputs() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ char buffer[] = "HELLO";
+ int r = fputs(buffer, F1);
+ if (r >= 0) {
+// fputs does not invalidate the input buffer (72 is ascii for 'H')
+clang_analyzer_dump_char(buffer[0]); // expected-warning {{72 S8b}}
+ } else if (r == EOF) {
+// fputs does not invalidate the input buffer, *and* this branch
+// can happen
+clang_analyzer_dump_char(buffer[0]); // expected-warning {{72 S8b}}
+ } else {
+// This branch can not happen
alejandro-alvarez-sonarsource wrote:
Likewise, removed.
https://github.com/llvm/llvm-project/pull/82476
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83027
From efd1411ff7fa93a84cb3d385cb55aa411af9e9ca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 21 Feb 2024 14:46:01 +0100
Subject: [PATCH] [clang][analyzer] Model getline/getdelim preconditions
1. lineptr, n and stream can not be NULL
2. if *lineptr is NULL, *n must be 0
---
.../Core/PathSensitive/CheckerHelpers.h | 6 +
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 155 -
.../StaticAnalyzer/Core/CheckerHelpers.cpp| 9 +
.../system-header-simulator-for-malloc.h | 1 +
clang/test/Analysis/getline-stream.c | 327 ++
5 files changed, 496 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Analysis/getline-stream.c
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
index 65982457ad8393..60421e5437d82f 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
@@ -13,6 +13,9 @@
#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
+#include "ProgramState_Fwd.h"
+#include "SVals.h"
+
#include "clang/AST/OperationKinds.h"
#include "clang/AST/Stmt.h"
#include "clang/Basic/OperatorKinds.h"
@@ -110,6 +113,9 @@ class OperatorKind {
OperatorKind operationKindFromOverloadedOperator(OverloadedOperatorKind OOK,
bool IsBinary);
+std::optional getPointeeDefVal(SVal PtrSVal,
+ProgramStateRef State);
+
} // namespace ento
} // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 65bdc4cac30940..f83d4e436d0382 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/Sequence.h"
#include
#include
@@ -312,6 +313,9 @@ class StreamChecker : public Checker();
+ if (!Ptr)
+return nullptr;
+
+ assert(PtrExpr && "Expected an argument");
+
+ const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
+ if (!PtrNotNull && PtrNull) {
+if (ExplodedNode *N = C.generateErrorNode(PtrNull)) {
+ SmallString<256> buf;
+ llvm::raw_svector_ostream os(buf);
+ os << PtrDescr << " pointer might be NULL.";
+
+ auto R = std::make_unique(BT_SizeNull, buf, N);
+ bugreporter::trackExpressionValue(N, PtrExpr, *R);
+ C.emitReport(std::move(R));
+}
+return nullptr;
+ }
+
+ return PtrNotNull;
+}
+
+ProgramStateRef StreamChecker::ensureSizeZeroIfLineNull(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+ static constexpr char SizeNotZeroMsg[] =
+ "Line pointer might be null while n value is not zero";
+
+ // We have a pointer to a pointer to the buffer, and a pointer to the size.
+ // We want what they point at.
+ auto LinePtrSVal = getPointeeDefVal(LinePtrPtrSVal, State);
+ auto NSVal = getPointeeDefVal(SizePtrSVal, State);
+ if (!LinePtrSVal || !NSVal)
+return nullptr;
+
+ assert(LinePtrPtrExpr &&
+ "Expected an argument with a pointer to a pointer to the buffer.");
+ assert(SizePtrExpr &&
+ "Expected an argument with a pointer to the buffer size.");
+
+ // If the line pointer is null, and n is > 0, there is UB.
+ const auto [LinePtrNotNull, LinePtrNull] = State->assume(*LinePtrSVal);
+ if (LinePtrNull && !LinePtrNotNull) {
+const auto [NIsNotZero, NIsZero] = LinePtrNull->assume(*NSVal);
+if (NIsNotZero && !NIsZero) {
+ if (ExplodedNode *N = C.generateErrorNode(NIsNotZero)) {
+auto R = std::make_unique(BT_SizeNotZero,
+ SizeNotZeroMsg, N);
+bugreporter::trackExpressionValue(N, SizePtrExpr, *R);
+bugreporter::trackExpressionValue(N, LinePtrPtrExpr, *R);
+C.emitReport(std::move(R));
+ }
+ return nullptr;
+}
+return NIsZero;
+ }
+ return LinePtrNotNull;
+}
+
+void StreamChecker::preGetdelim(const FnDescription *Desc,
+const CallEvent &Call,
+CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+
+ auto AddTransitionOnReturn = llvm::make_scope_exit([&] {
+if (State != nullptr) {
+
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
@@ -339,3 +363,138 @@ void fflush_on_open_failed_stream(void) {
}
fclose(F);
}
+
+void test_fscanf_eof() {
alejandro-alvarez-sonarsource wrote:
Dropped.
https://github.com/llvm/llvm-project/pull/82476
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/82476
From a21881d82fe3674b344d4a3807e9d2590c98ce93 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Tue, 14 Nov 2023 09:28:45 +0100
Subject: [PATCH 1/8] [clang][analyzer] StreamChecker: add more APIs,
invalidate fscanf args
1. Model getc, vfscanf, putc, vfprintf.
2. fscanf invalidates all arguments after the format string.
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 39 +-
...ystem-header-simulator-for-simple-stream.h | 2 +-
.../system-header-simulator-for-valist.h | 4 +
.../Analysis/Inputs/system-header-simulator.h | 3 +
clang/test/Analysis/stream.c | 128 ++
5 files changed, 174 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index a070f451694a3b..7938a0d30a91a3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,8 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/SmallVector.h"
#include
#include
@@ -171,7 +173,7 @@ using FnCheck = std::function;
using ArgNoTy = unsigned int;
-static const ArgNoTy ArgNone = std::numeric_limits::max();
+const ArgNoTy ArgNone = std::numeric_limits::max();
struct FnDescription {
FnCheck PreFn;
@@ -179,6 +181,26 @@ struct FnDescription {
ArgNoTy StreamArgNo;
};
+[[nodiscard]] ProgramStateRef
+escapeArgsAfterIndex(ProgramStateRef State, CheckerContext &C,
+ const CallEvent &Call, unsigned FirstEscapingArgIndex) {
+ const auto *CE = Call.getOriginExpr();
+ assert(CE);
+
+ if (Call.getNumArgs() <= FirstEscapingArgIndex)
+return State;
+
+ SmallVector EscapingArgs;
+ EscapingArgs.reserve(Call.getNumArgs() - FirstEscapingArgIndex);
+ for (auto EscArgIdx :
+ llvm::seq(FirstEscapingArgIndex, Call.getNumArgs()))
+EscapingArgs.push_back(Call.getArgSVal(EscArgIdx));
+ State = State->invalidateRegions(EscapingArgs, CE, C.blockCount(),
+ C.getLocationContext(),
+ /*CausesPointerEscape=*/false);
+ return State;
+}
+
/// Get the value of the stream argument out of the passed call event.
/// The call should contain a function that is described by Desc.
SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
@@ -396,6 +418,18 @@ class StreamChecker : public Checker FnTestDescriptions = {
@@ -997,6 +1031,9 @@ void StreamChecker::evalFscanf(const FnDescription *Desc,
const CallEvent &Call,
if (!E.Init(Desc, Call, C, State))
return;
+ // The pointers passed to fscanf escape and get invalidated.
+ State = escapeArgsAfterIndex(State, C, Call, /*FirstEscapingArgIndex=*/2);
+
// Add the success state.
// In this context "success" means there is not an EOF or other read error
// before any item is matched in 'fscanf'. But there may be match failure,
diff --git
a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
index 098a2208fecbe9..c26d3582149120 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
@@ -5,7 +5,7 @@
// suppressed.
#pragma clang system_header
-typedef struct __sFILE {
+typedef struct _FILE {
unsigned char *_p;
} FILE;
FILE *fopen(const char *restrict, const char *restrict) __asm("_" "fopen" );
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
index 7299b61353d460..87688bd8b312f4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
@@ -10,6 +10,8 @@
#define restrict /*restrict*/
#endif
+typedef struct _FILE FILE;
+
typedef __builtin_va_list va_list;
#define va_start(ap, param) __builtin_va_start(ap, param)
@@ -21,6 +23,8 @@ int vprintf (const char *restrict format, va_list arg);
int vsprintf (char *restrict s, const char *restrict format, va_list arg);
+int vfscanf(FILE *stream, const char *format, va_list ap);
+
int some_library_function(int n, va_list arg);
// No warning from system header.
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h
b/clang/test/Analysis/Inputs/system-header-simulator.h
index 15986984802c0e..8fd51449ecc0a4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -73,6 +73,9 @@ int ferror(FILE *stream);
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/82476
From a21881d82fe3674b344d4a3807e9d2590c98ce93 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Tue, 14 Nov 2023 09:28:45 +0100
Subject: [PATCH 1/9] [clang][analyzer] StreamChecker: add more APIs,
invalidate fscanf args
1. Model getc, vfscanf, putc, vfprintf.
2. fscanf invalidates all arguments after the format string.
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 39 +-
...ystem-header-simulator-for-simple-stream.h | 2 +-
.../system-header-simulator-for-valist.h | 4 +
.../Analysis/Inputs/system-header-simulator.h | 3 +
clang/test/Analysis/stream.c | 128 ++
5 files changed, 174 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index a070f451694a3b..7938a0d30a91a3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,8 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/SmallVector.h"
#include
#include
@@ -171,7 +173,7 @@ using FnCheck = std::function;
using ArgNoTy = unsigned int;
-static const ArgNoTy ArgNone = std::numeric_limits::max();
+const ArgNoTy ArgNone = std::numeric_limits::max();
struct FnDescription {
FnCheck PreFn;
@@ -179,6 +181,26 @@ struct FnDescription {
ArgNoTy StreamArgNo;
};
+[[nodiscard]] ProgramStateRef
+escapeArgsAfterIndex(ProgramStateRef State, CheckerContext &C,
+ const CallEvent &Call, unsigned FirstEscapingArgIndex) {
+ const auto *CE = Call.getOriginExpr();
+ assert(CE);
+
+ if (Call.getNumArgs() <= FirstEscapingArgIndex)
+return State;
+
+ SmallVector EscapingArgs;
+ EscapingArgs.reserve(Call.getNumArgs() - FirstEscapingArgIndex);
+ for (auto EscArgIdx :
+ llvm::seq(FirstEscapingArgIndex, Call.getNumArgs()))
+EscapingArgs.push_back(Call.getArgSVal(EscArgIdx));
+ State = State->invalidateRegions(EscapingArgs, CE, C.blockCount(),
+ C.getLocationContext(),
+ /*CausesPointerEscape=*/false);
+ return State;
+}
+
/// Get the value of the stream argument out of the passed call event.
/// The call should contain a function that is described by Desc.
SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
@@ -396,6 +418,18 @@ class StreamChecker : public Checker FnTestDescriptions = {
@@ -997,6 +1031,9 @@ void StreamChecker::evalFscanf(const FnDescription *Desc,
const CallEvent &Call,
if (!E.Init(Desc, Call, C, State))
return;
+ // The pointers passed to fscanf escape and get invalidated.
+ State = escapeArgsAfterIndex(State, C, Call, /*FirstEscapingArgIndex=*/2);
+
// Add the success state.
// In this context "success" means there is not an EOF or other read error
// before any item is matched in 'fscanf'. But there may be match failure,
diff --git
a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
index 098a2208fecbe9..c26d3582149120 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
@@ -5,7 +5,7 @@
// suppressed.
#pragma clang system_header
-typedef struct __sFILE {
+typedef struct _FILE {
unsigned char *_p;
} FILE;
FILE *fopen(const char *restrict, const char *restrict) __asm("_" "fopen" );
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
index 7299b61353d460..87688bd8b312f4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
@@ -10,6 +10,8 @@
#define restrict /*restrict*/
#endif
+typedef struct _FILE FILE;
+
typedef __builtin_va_list va_list;
#define va_start(ap, param) __builtin_va_start(ap, param)
@@ -21,6 +23,8 @@ int vprintf (const char *restrict format, va_list arg);
int vsprintf (char *restrict s, const char *restrict format, va_list arg);
+int vfscanf(FILE *stream, const char *format, va_list ap);
+
int some_library_function(int n, va_list arg);
// No warning from system header.
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h
b/clang/test/Analysis/Inputs/system-header-simulator.h
index 15986984802c0e..8fd51449ecc0a4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -73,6 +73,9 @@ int ferror(FILE *stream);
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/82476
From a21881d82fe3674b344d4a3807e9d2590c98ce93 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Tue, 14 Nov 2023 09:28:45 +0100
Subject: [PATCH 01/10] [clang][analyzer] StreamChecker: add more APIs,
invalidate fscanf args
1. Model getc, vfscanf, putc, vfprintf.
2. fscanf invalidates all arguments after the format string.
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 39 +-
...ystem-header-simulator-for-simple-stream.h | 2 +-
.../system-header-simulator-for-valist.h | 4 +
.../Analysis/Inputs/system-header-simulator.h | 3 +
clang/test/Analysis/stream.c | 128 ++
5 files changed, 174 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index a070f451694a3b..7938a0d30a91a3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,8 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/SmallVector.h"
#include
#include
@@ -171,7 +173,7 @@ using FnCheck = std::function;
using ArgNoTy = unsigned int;
-static const ArgNoTy ArgNone = std::numeric_limits::max();
+const ArgNoTy ArgNone = std::numeric_limits::max();
struct FnDescription {
FnCheck PreFn;
@@ -179,6 +181,26 @@ struct FnDescription {
ArgNoTy StreamArgNo;
};
+[[nodiscard]] ProgramStateRef
+escapeArgsAfterIndex(ProgramStateRef State, CheckerContext &C,
+ const CallEvent &Call, unsigned FirstEscapingArgIndex) {
+ const auto *CE = Call.getOriginExpr();
+ assert(CE);
+
+ if (Call.getNumArgs() <= FirstEscapingArgIndex)
+return State;
+
+ SmallVector EscapingArgs;
+ EscapingArgs.reserve(Call.getNumArgs() - FirstEscapingArgIndex);
+ for (auto EscArgIdx :
+ llvm::seq(FirstEscapingArgIndex, Call.getNumArgs()))
+EscapingArgs.push_back(Call.getArgSVal(EscArgIdx));
+ State = State->invalidateRegions(EscapingArgs, CE, C.blockCount(),
+ C.getLocationContext(),
+ /*CausesPointerEscape=*/false);
+ return State;
+}
+
/// Get the value of the stream argument out of the passed call event.
/// The call should contain a function that is described by Desc.
SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
@@ -396,6 +418,18 @@ class StreamChecker : public Checker FnTestDescriptions = {
@@ -997,6 +1031,9 @@ void StreamChecker::evalFscanf(const FnDescription *Desc,
const CallEvent &Call,
if (!E.Init(Desc, Call, C, State))
return;
+ // The pointers passed to fscanf escape and get invalidated.
+ State = escapeArgsAfterIndex(State, C, Call, /*FirstEscapingArgIndex=*/2);
+
// Add the success state.
// In this context "success" means there is not an EOF or other read error
// before any item is matched in 'fscanf'. But there may be match failure,
diff --git
a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
index 098a2208fecbe9..c26d3582149120 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
@@ -5,7 +5,7 @@
// suppressed.
#pragma clang system_header
-typedef struct __sFILE {
+typedef struct _FILE {
unsigned char *_p;
} FILE;
FILE *fopen(const char *restrict, const char *restrict) __asm("_" "fopen" );
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
index 7299b61353d460..87688bd8b312f4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
@@ -10,6 +10,8 @@
#define restrict /*restrict*/
#endif
+typedef struct _FILE FILE;
+
typedef __builtin_va_list va_list;
#define va_start(ap, param) __builtin_va_start(ap, param)
@@ -21,6 +23,8 @@ int vprintf (const char *restrict format, va_list arg);
int vsprintf (char *restrict s, const char *restrict format, va_list arg);
+int vfscanf(FILE *stream, const char *format, va_list ap);
+
int some_library_function(int n, va_list arg);
// No warning from system header.
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h
b/clang/test/Analysis/Inputs/system-header-simulator.h
index 15986984802c0e..8fd51449ecc0a4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -73,6 +73,9 @@ int ferror(FILE *stream
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/82476
From a21881d82fe3674b344d4a3807e9d2590c98ce93 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Tue, 14 Nov 2023 09:28:45 +0100
Subject: [PATCH 01/11] [clang][analyzer] StreamChecker: add more APIs,
invalidate fscanf args
1. Model getc, vfscanf, putc, vfprintf.
2. fscanf invalidates all arguments after the format string.
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 39 +-
...ystem-header-simulator-for-simple-stream.h | 2 +-
.../system-header-simulator-for-valist.h | 4 +
.../Analysis/Inputs/system-header-simulator.h | 3 +
clang/test/Analysis/stream.c | 128 ++
5 files changed, 174 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index a070f451694a3b..7938a0d30a91a3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,8 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/SmallVector.h"
#include
#include
@@ -171,7 +173,7 @@ using FnCheck = std::function;
using ArgNoTy = unsigned int;
-static const ArgNoTy ArgNone = std::numeric_limits::max();
+const ArgNoTy ArgNone = std::numeric_limits::max();
struct FnDescription {
FnCheck PreFn;
@@ -179,6 +181,26 @@ struct FnDescription {
ArgNoTy StreamArgNo;
};
+[[nodiscard]] ProgramStateRef
+escapeArgsAfterIndex(ProgramStateRef State, CheckerContext &C,
+ const CallEvent &Call, unsigned FirstEscapingArgIndex) {
+ const auto *CE = Call.getOriginExpr();
+ assert(CE);
+
+ if (Call.getNumArgs() <= FirstEscapingArgIndex)
+return State;
+
+ SmallVector EscapingArgs;
+ EscapingArgs.reserve(Call.getNumArgs() - FirstEscapingArgIndex);
+ for (auto EscArgIdx :
+ llvm::seq(FirstEscapingArgIndex, Call.getNumArgs()))
+EscapingArgs.push_back(Call.getArgSVal(EscArgIdx));
+ State = State->invalidateRegions(EscapingArgs, CE, C.blockCount(),
+ C.getLocationContext(),
+ /*CausesPointerEscape=*/false);
+ return State;
+}
+
/// Get the value of the stream argument out of the passed call event.
/// The call should contain a function that is described by Desc.
SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
@@ -396,6 +418,18 @@ class StreamChecker : public Checker FnTestDescriptions = {
@@ -997,6 +1031,9 @@ void StreamChecker::evalFscanf(const FnDescription *Desc,
const CallEvent &Call,
if (!E.Init(Desc, Call, C, State))
return;
+ // The pointers passed to fscanf escape and get invalidated.
+ State = escapeArgsAfterIndex(State, C, Call, /*FirstEscapingArgIndex=*/2);
+
// Add the success state.
// In this context "success" means there is not an EOF or other read error
// before any item is matched in 'fscanf'. But there may be match failure,
diff --git
a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
index 098a2208fecbe9..c26d3582149120 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
@@ -5,7 +5,7 @@
// suppressed.
#pragma clang system_header
-typedef struct __sFILE {
+typedef struct _FILE {
unsigned char *_p;
} FILE;
FILE *fopen(const char *restrict, const char *restrict) __asm("_" "fopen" );
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
index 7299b61353d460..87688bd8b312f4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
@@ -10,6 +10,8 @@
#define restrict /*restrict*/
#endif
+typedef struct _FILE FILE;
+
typedef __builtin_va_list va_list;
#define va_start(ap, param) __builtin_va_start(ap, param)
@@ -21,6 +23,8 @@ int vprintf (const char *restrict format, va_list arg);
int vsprintf (char *restrict s, const char *restrict format, va_list arg);
+int vfscanf(FILE *stream, const char *format, va_list ap);
+
int some_library_function(int n, va_list arg);
// No warning from system header.
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h
b/clang/test/Analysis/Inputs/system-header-simulator.h
index 15986984802c0e..8fd51449ecc0a4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -73,6 +73,9 @@ int ferror(FILE *stream
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
@@ -65,12 +65,24 @@ void check_fseek(void) {
fclose(fp);
}
+void check_fseeko(void) {
alejandro-alvarez-sonarsource wrote:
Added
https://github.com/llvm/llvm-project/pull/82476
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
@@ -5,7 +5,7 @@
// suppressed.
#pragma clang system_header
-typedef struct __sFILE {
alejandro-alvarez-sonarsource wrote:
I had to modify this line after adding `vfscanf` (and now `vfprintf`) to
`system-header-simulator-for-valist.h`
The reason is that they need the `FILE*` typedef and they have to be consistent
with the typedef of `system-header-simulator.h` since they are used together.
But `system-header-simulator-for-valist.h` was already used together with
`system-header-simulator-for-simple-stream.h` in
[`security-syntax-checks.m`](https://github.com/llvm/llvm-project/blob/main/clang/test/Analysis/security-syntax-checks.m).
So I had to change something to keep them consistent, and `typedef struct _FILE
FILE;` seems to be used more commonly over the tests.
https://github.com/llvm/llvm-project/pull/82476
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
https://github.com/alejandro-alvarez-sonarsource edited https://github.com/llvm/llvm-project/pull/82476 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83027
From efd1411ff7fa93a84cb3d385cb55aa411af9e9ca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 21 Feb 2024 14:46:01 +0100
Subject: [PATCH 1/2] [clang][analyzer] Model getline/getdelim preconditions
1. lineptr, n and stream can not be NULL
2. if *lineptr is NULL, *n must be 0
---
.../Core/PathSensitive/CheckerHelpers.h | 6 +
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 155 -
.../StaticAnalyzer/Core/CheckerHelpers.cpp| 9 +
.../system-header-simulator-for-malloc.h | 1 +
clang/test/Analysis/getline-stream.c | 327 ++
5 files changed, 496 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Analysis/getline-stream.c
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
index 65982457ad8393..60421e5437d82f 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
@@ -13,6 +13,9 @@
#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
+#include "ProgramState_Fwd.h"
+#include "SVals.h"
+
#include "clang/AST/OperationKinds.h"
#include "clang/AST/Stmt.h"
#include "clang/Basic/OperatorKinds.h"
@@ -110,6 +113,9 @@ class OperatorKind {
OperatorKind operationKindFromOverloadedOperator(OverloadedOperatorKind OOK,
bool IsBinary);
+std::optional getPointeeDefVal(SVal PtrSVal,
+ProgramStateRef State);
+
} // namespace ento
} // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 65bdc4cac30940..f83d4e436d0382 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/Sequence.h"
#include
#include
@@ -312,6 +313,9 @@ class StreamChecker : public Checker();
+ if (!Ptr)
+return nullptr;
+
+ assert(PtrExpr && "Expected an argument");
+
+ const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
+ if (!PtrNotNull && PtrNull) {
+if (ExplodedNode *N = C.generateErrorNode(PtrNull)) {
+ SmallString<256> buf;
+ llvm::raw_svector_ostream os(buf);
+ os << PtrDescr << " pointer might be NULL.";
+
+ auto R = std::make_unique(BT_SizeNull, buf, N);
+ bugreporter::trackExpressionValue(N, PtrExpr, *R);
+ C.emitReport(std::move(R));
+}
+return nullptr;
+ }
+
+ return PtrNotNull;
+}
+
+ProgramStateRef StreamChecker::ensureSizeZeroIfLineNull(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext &C, ProgramStateRef State) const {
+ static constexpr char SizeNotZeroMsg[] =
+ "Line pointer might be null while n value is not zero";
+
+ // We have a pointer to a pointer to the buffer, and a pointer to the size.
+ // We want what they point at.
+ auto LinePtrSVal = getPointeeDefVal(LinePtrPtrSVal, State);
+ auto NSVal = getPointeeDefVal(SizePtrSVal, State);
+ if (!LinePtrSVal || !NSVal)
+return nullptr;
+
+ assert(LinePtrPtrExpr &&
+ "Expected an argument with a pointer to a pointer to the buffer.");
+ assert(SizePtrExpr &&
+ "Expected an argument with a pointer to the buffer size.");
+
+ // If the line pointer is null, and n is > 0, there is UB.
+ const auto [LinePtrNotNull, LinePtrNull] = State->assume(*LinePtrSVal);
+ if (LinePtrNull && !LinePtrNotNull) {
+const auto [NIsNotZero, NIsZero] = LinePtrNull->assume(*NSVal);
+if (NIsNotZero && !NIsZero) {
+ if (ExplodedNode *N = C.generateErrorNode(NIsNotZero)) {
+auto R = std::make_unique(BT_SizeNotZero,
+ SizeNotZeroMsg, N);
+bugreporter::trackExpressionValue(N, SizePtrExpr, *R);
+bugreporter::trackExpressionValue(N, LinePtrPtrExpr, *R);
+C.emitReport(std::move(R));
+ }
+ return nullptr;
+}
+return NIsZero;
+ }
+ return LinePtrNotNull;
+}
+
+void StreamChecker::preGetdelim(const FnDescription *Desc,
+const CallEvent &Call,
+CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+
+ auto AddTransitionOnReturn = llvm::make_scope_exit([&] {
+if (State != nullptr) {
+
[clang] [clang][analyzer] Model allocation behavior or getdelim/geline (PR #83138)
https://github.com/alejandro-alvarez-sonarsource created
https://github.com/llvm/llvm-project/pull/83138
`getdelim` and `getline` may free, allocate, or re-allocate the input buffer,
ensuring its size is enough to hold the incoming line, the delimiter, and the
null terminator.
`*lineptr` must be a valid argument to `free`, which means it can be either
1. `NULL`, in which case these functions perform an allocation equivalent to a
call to `malloc` even on failure.
2. A pointer returned by the `malloc` family of functions. Other pointers are
UB (`alloca`, a pointer to a static, to a stack variable, etc.)
From ad17cfb588500d095b4e402bd2f2197772f89b12 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 21 Feb 2024 16:08:04 +0100
Subject: [PATCH] [clang][analyzer] Model allocation behavior or
getdelim/geline
---
.../Core/PathSensitive/CheckerHelpers.h | 6 ++
.../StaticAnalyzer/Checkers/MallocChecker.cpp | 73 +--
.../StaticAnalyzer/Core/CheckerHelpers.cpp| 9 +++
.../system-header-simulator-for-malloc.h | 1 +
clang/test/Analysis/getline-alloc.c | 71 ++
5 files changed, 154 insertions(+), 6 deletions(-)
create mode 100644 clang/test/Analysis/getline-alloc.c
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
index 65982457ad8393..60421e5437d82f 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
@@ -13,6 +13,9 @@
#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
+#include "ProgramState_Fwd.h"
+#include "SVals.h"
+
#include "clang/AST/OperationKinds.h"
#include "clang/AST/Stmt.h"
#include "clang/Basic/OperatorKinds.h"
@@ -110,6 +113,9 @@ class OperatorKind {
OperatorKind operationKindFromOverloadedOperator(OverloadedOperatorKind OOK,
bool IsBinary);
+std::optional getPointeeDefVal(SVal PtrSVal,
+ProgramStateRef State);
+
} // namespace ento
} // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 79ab05f2c7866a..c5c382634c981c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -382,6 +382,8 @@ class MallocChecker
CHECK_FN(checkGMemdup)
CHECK_FN(checkGMallocN)
CHECK_FN(checkGMallocN0)
+ CHECK_FN(preGetdelim)
+ CHECK_FN(checkGetdelim)
CHECK_FN(checkReallocN)
CHECK_FN(checkOwnershipAttr)
@@ -391,6 +393,11 @@ class MallocChecker
using CheckFn = std::function;
+ const CallDescriptionMap PreFnMap{
+ {{{"getline"}, 3}, &MallocChecker::preGetdelim},
+ {{{"getdelim"}, 4}, &MallocChecker::preGetdelim},
+ };
+
const CallDescriptionMap FreeingMemFnMap{
{{{"free"}, 1}, &MallocChecker::checkFree},
{{{"if_freenameindex"}, 1}, &MallocChecker::checkIfFreeNameIndex},
@@ -439,6 +446,8 @@ class MallocChecker
std::bind(&MallocChecker::checkRealloc, _1, _2, _3, false)},
{{{"g_realloc_n"}, 3}, &MallocChecker::checkReallocN},
{{{"g_try_realloc_n"}, 3}, &MallocChecker::checkReallocN},
+ {{{"getline"}, 3}, &MallocChecker::checkGetdelim},
+ {{{"getdelim"}, 4}, &MallocChecker::checkGetdelim},
};
bool isMemCall(const CallEvent &Call) const;
@@ -588,11 +597,14 @@ class MallocChecker
/// }
/// \param [in] ReturnsNullOnFailure Whether the memory deallocation function
/// we're modeling returns with Null on failure.
+ /// \param [in] ArgValOpt Optional value to use for the argument instead of
+ /// the one obtained from ArgExpr.
/// \returns The ProgramState right after deallocation.
[[nodiscard]] ProgramStateRef
FreeMemAux(CheckerContext &C, const Expr *ArgExpr, const CallEvent &Call,
ProgramStateRef State, bool Hold, bool &IsKnownToBeAllocated,
- AllocationFamily Family, bool ReturnsNullOnFailure = false) const;
+ AllocationFamily Family, bool ReturnsNullOnFailure = false,
+ std::optional ArgValOpt = {}) const;
// TODO: Needs some refactoring, as all other deallocation modeling
// functions are suffering from out parameters and messy code due to how
@@ -1423,6 +1435,46 @@ void MallocChecker::checkGMallocN0(const CallEvent &Call,
C.addTransition(State);
}
+void MallocChecker::preGetdelim(const CallEvent &Call,
+CheckerContext &C) const {
+ if (!Call.isGlobalCFunction())
+return;
+
+ ProgramStateRef State = C.getState();
+ const auto LinePtr = getPointeeDefVal(Call.getArgSVal(0), State);
+ if (!LinePtr)
+return;
+
+ bool IsKnownToBeA
[clang] [clang][analyzer] Model allocation behavior or getdelim/geline (PR #83138)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83138
From ad17cfb588500d095b4e402bd2f2197772f89b12 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 21 Feb 2024 16:08:04 +0100
Subject: [PATCH 1/2] [clang][analyzer] Model allocation behavior or
getdelim/geline
---
.../Core/PathSensitive/CheckerHelpers.h | 6 ++
.../StaticAnalyzer/Checkers/MallocChecker.cpp | 73 +--
.../StaticAnalyzer/Core/CheckerHelpers.cpp| 9 +++
.../system-header-simulator-for-malloc.h | 1 +
clang/test/Analysis/getline-alloc.c | 71 ++
5 files changed, 154 insertions(+), 6 deletions(-)
create mode 100644 clang/test/Analysis/getline-alloc.c
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
index 65982457ad8393..60421e5437d82f 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
@@ -13,6 +13,9 @@
#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
+#include "ProgramState_Fwd.h"
+#include "SVals.h"
+
#include "clang/AST/OperationKinds.h"
#include "clang/AST/Stmt.h"
#include "clang/Basic/OperatorKinds.h"
@@ -110,6 +113,9 @@ class OperatorKind {
OperatorKind operationKindFromOverloadedOperator(OverloadedOperatorKind OOK,
bool IsBinary);
+std::optional getPointeeDefVal(SVal PtrSVal,
+ProgramStateRef State);
+
} // namespace ento
} // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 79ab05f2c7866a..c5c382634c981c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -382,6 +382,8 @@ class MallocChecker
CHECK_FN(checkGMemdup)
CHECK_FN(checkGMallocN)
CHECK_FN(checkGMallocN0)
+ CHECK_FN(preGetdelim)
+ CHECK_FN(checkGetdelim)
CHECK_FN(checkReallocN)
CHECK_FN(checkOwnershipAttr)
@@ -391,6 +393,11 @@ class MallocChecker
using CheckFn = std::function;
+ const CallDescriptionMap PreFnMap{
+ {{{"getline"}, 3}, &MallocChecker::preGetdelim},
+ {{{"getdelim"}, 4}, &MallocChecker::preGetdelim},
+ };
+
const CallDescriptionMap FreeingMemFnMap{
{{{"free"}, 1}, &MallocChecker::checkFree},
{{{"if_freenameindex"}, 1}, &MallocChecker::checkIfFreeNameIndex},
@@ -439,6 +446,8 @@ class MallocChecker
std::bind(&MallocChecker::checkRealloc, _1, _2, _3, false)},
{{{"g_realloc_n"}, 3}, &MallocChecker::checkReallocN},
{{{"g_try_realloc_n"}, 3}, &MallocChecker::checkReallocN},
+ {{{"getline"}, 3}, &MallocChecker::checkGetdelim},
+ {{{"getdelim"}, 4}, &MallocChecker::checkGetdelim},
};
bool isMemCall(const CallEvent &Call) const;
@@ -588,11 +597,14 @@ class MallocChecker
/// }
/// \param [in] ReturnsNullOnFailure Whether the memory deallocation function
/// we're modeling returns with Null on failure.
+ /// \param [in] ArgValOpt Optional value to use for the argument instead of
+ /// the one obtained from ArgExpr.
/// \returns The ProgramState right after deallocation.
[[nodiscard]] ProgramStateRef
FreeMemAux(CheckerContext &C, const Expr *ArgExpr, const CallEvent &Call,
ProgramStateRef State, bool Hold, bool &IsKnownToBeAllocated,
- AllocationFamily Family, bool ReturnsNullOnFailure = false) const;
+ AllocationFamily Family, bool ReturnsNullOnFailure = false,
+ std::optional ArgValOpt = {}) const;
// TODO: Needs some refactoring, as all other deallocation modeling
// functions are suffering from out parameters and messy code due to how
@@ -1423,6 +1435,46 @@ void MallocChecker::checkGMallocN0(const CallEvent &Call,
C.addTransition(State);
}
+void MallocChecker::preGetdelim(const CallEvent &Call,
+CheckerContext &C) const {
+ if (!Call.isGlobalCFunction())
+return;
+
+ ProgramStateRef State = C.getState();
+ const auto LinePtr = getPointeeDefVal(Call.getArgSVal(0), State);
+ if (!LinePtr)
+return;
+
+ bool IsKnownToBeAllocated = false;
+ State = FreeMemAux(C, Call.getArgExpr(0), Call, State, false,
+ IsKnownToBeAllocated, AF_Malloc, false, LinePtr);
+ if (State)
+C.addTransition(State);
+}
+
+void MallocChecker::checkGetdelim(const CallEvent &Call,
+ CheckerContext &C) const {
+ if (!Call.isGlobalCFunction())
+return;
+
+ ProgramStateRef State = C.getState();
+ // Handle the post-conditions of getline and getdelim:
+ // Register the new conjured value
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/82476
From a886005893585ce060415619e1fa6164ba4e1729 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Tue, 14 Nov 2023 09:28:45 +0100
Subject: [PATCH 01/11] [clang][analyzer] StreamChecker: add more APIs,
invalidate fscanf args
1. Model getc, vfscanf, putc, vfprintf.
2. fscanf invalidates all arguments after the format string.
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 37 -
...ystem-header-simulator-for-simple-stream.h | 2 +-
.../system-header-simulator-for-valist.h | 4 +
.../Analysis/Inputs/system-header-simulator.h | 3 +
clang/test/Analysis/stream.c | 128 ++
5 files changed, 172 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 65bdc4cac30940..247d612989fd87 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -172,7 +172,7 @@ using FnCheck = std::function;
using ArgNoTy = unsigned int;
-static const ArgNoTy ArgNone = std::numeric_limits::max();
+const ArgNoTy ArgNone = std::numeric_limits::max();
struct FnDescription {
FnCheck PreFn;
@@ -180,6 +180,26 @@ struct FnDescription {
ArgNoTy StreamArgNo;
};
+[[nodiscard]] ProgramStateRef
+escapeArgsAfterIndex(ProgramStateRef State, CheckerContext &C,
+ const CallEvent &Call, unsigned FirstEscapingArgIndex) {
+ const auto *CE = Call.getOriginExpr();
+ assert(CE);
+
+ if (Call.getNumArgs() <= FirstEscapingArgIndex)
+return State;
+
+ SmallVector EscapingArgs;
+ EscapingArgs.reserve(Call.getNumArgs() - FirstEscapingArgIndex);
+ for (auto EscArgIdx :
+ llvm::seq(FirstEscapingArgIndex, Call.getNumArgs()))
+EscapingArgs.push_back(Call.getArgSVal(EscArgIdx));
+ State = State->invalidateRegions(EscapingArgs, CE, C.blockCount(),
+ C.getLocationContext(),
+ /*CausesPointerEscape=*/false);
+ return State;
+}
+
/// Get the value of the stream argument out of the passed call event.
/// The call should contain a function that is described by Desc.
SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
@@ -397,6 +417,18 @@ class StreamChecker : public Checker FnTestDescriptions = {
@@ -1021,6 +1053,9 @@ void StreamChecker::evalFscanf(const FnDescription *Desc,
const CallEvent &Call,
if (!E.Init(Desc, Call, C, State))
return;
+ // The pointers passed to fscanf escape and get invalidated.
+ State = escapeArgsAfterIndex(State, C, Call, /*FirstEscapingArgIndex=*/2);
+
// Add the success state.
// In this context "success" means there is not an EOF or other read error
// before any item is matched in 'fscanf'. But there may be match failure,
diff --git
a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
index 098a2208fecbe9..c26d3582149120 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
@@ -5,7 +5,7 @@
// suppressed.
#pragma clang system_header
-typedef struct __sFILE {
+typedef struct _FILE {
unsigned char *_p;
} FILE;
FILE *fopen(const char *restrict, const char *restrict) __asm("_" "fopen" );
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
index 7299b61353d460..87688bd8b312f4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
@@ -10,6 +10,8 @@
#define restrict /*restrict*/
#endif
+typedef struct _FILE FILE;
+
typedef __builtin_va_list va_list;
#define va_start(ap, param) __builtin_va_start(ap, param)
@@ -21,6 +23,8 @@ int vprintf (const char *restrict format, va_list arg);
int vsprintf (char *restrict s, const char *restrict format, va_list arg);
+int vfscanf(FILE *stream, const char *format, va_list ap);
+
int some_library_function(int n, va_list arg);
// No warning from system header.
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h
b/clang/test/Analysis/Inputs/system-header-simulator.h
index 15986984802c0e..8fd51449ecc0a4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -73,6 +73,9 @@ int ferror(FILE *stream);
int fileno(FILE *stream);
int fflush(FILE *stream);
+
+int getc(FILE *stream);
+
size_t strlen(const char *);
char *strcpy(char *restrict, const char *restrict);
diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index 378c9154f8f6a8..d0fee68d482e7f 100644
--- a/clang/test/Analysis/
[clang] [clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (PR #82476)
alejandro-alvarez-sonarsource wrote: Rebased on top of main to solve a conflict. https://github.com/llvm/llvm-project/pull/82476 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reapply "[clang][analyzer] StreamChecker: Model getc, vfscanf, putc, … (PR #83281)
https://github.com/alejandro-alvarez-sonarsource created
https://github.com/llvm/llvm-project/pull/83281
…vfprintf (#82476)"
`va_list` is a platform-specific type. On some, it is a struct instead of a
pointer to a struct, so `lookupFn` was ignoring calls to `vfprintf` and
`vfscanf`.
`stream.c` now runs in four different platforms to make sure the logic works
across targets.
This reverts commit 570bc5d291f92e19f6264262b02ddff1a2f2e09b.
From ddbe895e3d2893060ac54bc6c984eea22e09b460 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 28 Feb 2024 16:43:25 +0100
Subject: [PATCH] Reapply "[clang][analyzer] StreamChecker: Model getc,
vfscanf, putc, vfprintf (#82476)"
This reverts commit 570bc5d291f92e19f6264262b02ddff1a2f2e09b.
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 34 +-
...ystem-header-simulator-for-simple-stream.h | 2 +-
.../system-header-simulator-for-valist.h | 6 +
.../Analysis/Inputs/system-header-simulator.h | 3 +
clang/test/Analysis/stream-invalidate.c | 42 +++
clang/test/Analysis/stream.c | 341 +-
6 files changed, 91 insertions(+), 337 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 65bdc4cac30940..f9928e1325c53d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -348,18 +348,30 @@ class StreamChecker : public CheckergetType();
- if (!T->isIntegralOrEnumerationType() && !T->isPointerType())
+ if (!T->isIntegralOrEnumerationType() && !T->isPointerType() &&
+ T.getCanonicalType() != VaListType)
return nullptr;
}
@@ -600,6 +615,11 @@ class StreamChecker : public CheckerPreFn)
@@ -1038,10 +1059,13 @@ void StreamChecker::evalFscanf(const FnDescription
*Desc, const CallEvent &Call,
if (!StateNotFailed)
return;
-SmallVector EscArgs;
-for (auto EscArg : llvm::seq(2u, Call.getNumArgs()))
- EscArgs.push_back(EscArg);
-StateNotFailed = escapeArgs(StateNotFailed, C, Call, EscArgs);
+if (auto const *Callee = Call.getCalleeIdentifier();
+!Callee || !Callee->getName().equals("vfscanf")) {
+ SmallVector EscArgs;
+ for (auto EscArg : llvm::seq(2u, Call.getNumArgs()))
+EscArgs.push_back(EscArg);
+ StateNotFailed = escapeArgs(StateNotFailed, C, Call, EscArgs);
+}
if (StateNotFailed)
C.addTransition(StateNotFailed);
diff --git
a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
index 098a2208fecbe9..c26d3582149120 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
@@ -5,7 +5,7 @@
// suppressed.
#pragma clang system_header
-typedef struct __sFILE {
+typedef struct _FILE {
unsigned char *_p;
} FILE;
FILE *fopen(const char *restrict, const char *restrict) __asm("_" "fopen" );
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
index 7299b61353d460..720944abb8ad47 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
@@ -10,6 +10,8 @@
#define restrict /*restrict*/
#endif
+typedef struct _FILE FILE;
+
typedef __builtin_va_list va_list;
#define va_start(ap, param) __builtin_va_start(ap, param)
@@ -21,6 +23,10 @@ int vprintf (const char *restrict format, va_list arg);
int vsprintf (char *restrict s, const char *restrict format, va_list arg);
+int vfprintf(FILE *stream, const char *format, va_list ap);
+
+int vfscanf(FILE *stream, const char *format, va_list ap);
+
int some_library_function(int n, va_list arg);
// No warning from system header.
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h
b/clang/test/Analysis/Inputs/system-header-simulator.h
index 15986984802c0e..8fd51449ecc0a4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -73,6 +73,9 @@ int ferror(FILE *stream);
int fileno(FILE *stream);
int fflush(FILE *stream);
+
+int getc(FILE *stream);
+
size_t strlen(const char *);
char *strcpy(char *restrict, const char *restrict);
diff --git a/clang/test/Analysis/stream-invalidate.c
b/clang/test/Analysis/stream-invalidate.c
index 6745d11a2fe701..5046a356d0583d 100644
--- a/clang/test/Analysis/stream-invalidate.c
+++ b/clang/test/Analysis/stream-invalidate.c
@@ -4,6 +4,7 @@
// RUN: -analyzer-checker=debug.ExprInspection
#include "Inputs/system-header-simulator.h"
+#include "Inputs/system-header-simulator-for-valist.h"
void clang_analyzer_eval(int);
void clang_analyzer_dump(int);
@@ -145,3 +146,44 @@ void
[clang] Reapply "[clang][analyzer] StreamChecker: Model getc, vfscanf, putc, … (PR #83281)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83281
From a9419bc3ffa3d383a9373f0a116795162632dd40 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 28 Feb 2024 16:49:35 +0100
Subject: [PATCH] Reapply "[clang][analyzer] StreamChecker: Model getc,
vfscanf, putc, vfprintf (#82476)"
This reverts commit 570bc5d291f92e19f6264262b02ddff1a2f2e09b
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 34 ---
...ystem-header-simulator-for-simple-stream.h | 2 +-
.../system-header-simulator-for-valist.h | 6 +++
.../Analysis/Inputs/system-header-simulator.h | 3 ++
clang/test/Analysis/stream-invalidate.c | 42 +++
clang/test/Analysis/stream.c | 39 -
6 files changed, 119 insertions(+), 7 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 65bdc4cac30940..f9928e1325c53d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -348,18 +348,30 @@ class StreamChecker : public CheckergetType();
- if (!T->isIntegralOrEnumerationType() && !T->isPointerType())
+ if (!T->isIntegralOrEnumerationType() && !T->isPointerType() &&
+ T.getCanonicalType() != VaListType)
return nullptr;
}
@@ -600,6 +615,11 @@ class StreamChecker : public CheckerPreFn)
@@ -1038,10 +1059,13 @@ void StreamChecker::evalFscanf(const FnDescription
*Desc, const CallEvent &Call,
if (!StateNotFailed)
return;
-SmallVector EscArgs;
-for (auto EscArg : llvm::seq(2u, Call.getNumArgs()))
- EscArgs.push_back(EscArg);
-StateNotFailed = escapeArgs(StateNotFailed, C, Call, EscArgs);
+if (auto const *Callee = Call.getCalleeIdentifier();
+!Callee || !Callee->getName().equals("vfscanf")) {
+ SmallVector EscArgs;
+ for (auto EscArg : llvm::seq(2u, Call.getNumArgs()))
+EscArgs.push_back(EscArg);
+ StateNotFailed = escapeArgs(StateNotFailed, C, Call, EscArgs);
+}
if (StateNotFailed)
C.addTransition(StateNotFailed);
diff --git
a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
index 098a2208fecbe9..c26d3582149120 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
@@ -5,7 +5,7 @@
// suppressed.
#pragma clang system_header
-typedef struct __sFILE {
+typedef struct _FILE {
unsigned char *_p;
} FILE;
FILE *fopen(const char *restrict, const char *restrict) __asm("_" "fopen" );
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
index 7299b61353d460..720944abb8ad47 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
@@ -10,6 +10,8 @@
#define restrict /*restrict*/
#endif
+typedef struct _FILE FILE;
+
typedef __builtin_va_list va_list;
#define va_start(ap, param) __builtin_va_start(ap, param)
@@ -21,6 +23,10 @@ int vprintf (const char *restrict format, va_list arg);
int vsprintf (char *restrict s, const char *restrict format, va_list arg);
+int vfprintf(FILE *stream, const char *format, va_list ap);
+
+int vfscanf(FILE *stream, const char *format, va_list ap);
+
int some_library_function(int n, va_list arg);
// No warning from system header.
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h
b/clang/test/Analysis/Inputs/system-header-simulator.h
index 15986984802c0e..8fd51449ecc0a4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -73,6 +73,9 @@ int ferror(FILE *stream);
int fileno(FILE *stream);
int fflush(FILE *stream);
+
+int getc(FILE *stream);
+
size_t strlen(const char *);
char *strcpy(char *restrict, const char *restrict);
diff --git a/clang/test/Analysis/stream-invalidate.c
b/clang/test/Analysis/stream-invalidate.c
index 6745d11a2fe701..5046a356d0583d 100644
--- a/clang/test/Analysis/stream-invalidate.c
+++ b/clang/test/Analysis/stream-invalidate.c
@@ -4,6 +4,7 @@
// RUN: -analyzer-checker=debug.ExprInspection
#include "Inputs/system-header-simulator.h"
+#include "Inputs/system-header-simulator-for-valist.h"
void clang_analyzer_eval(int);
void clang_analyzer_dump(int);
@@ -145,3 +146,44 @@ void test_fgetpos() {
fclose(F);
}
+
+void test_fprintf() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ unsigned a = 42;
+ char *output = "HELLO";
+ int r = fprintf(F1, "%s\t%u\n", output, a);
+ // fprintf does not invalidate any of its input
+ // 69 is ascii for 'E'
+ clang_analyzer_dump(a); // expect
[clang] Reapply "[clang][analyzer] StreamChecker: Model getc, vfscanf, putc, … (PR #83281)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/83281
From a9419bc3ffa3d383a9373f0a116795162632dd40 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Wed, 28 Feb 2024 16:49:35 +0100
Subject: [PATCH 1/2] Reapply "[clang][analyzer] StreamChecker: Model getc,
vfscanf, putc, vfprintf (#82476)"
This reverts commit 570bc5d291f92e19f6264262b02ddff1a2f2e09b
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 34 ---
...ystem-header-simulator-for-simple-stream.h | 2 +-
.../system-header-simulator-for-valist.h | 6 +++
.../Analysis/Inputs/system-header-simulator.h | 3 ++
clang/test/Analysis/stream-invalidate.c | 42 +++
clang/test/Analysis/stream.c | 39 -
6 files changed, 119 insertions(+), 7 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 65bdc4cac30940..f9928e1325c53d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -348,18 +348,30 @@ class StreamChecker : public CheckergetType();
- if (!T->isIntegralOrEnumerationType() && !T->isPointerType())
+ if (!T->isIntegralOrEnumerationType() && !T->isPointerType() &&
+ T.getCanonicalType() != VaListType)
return nullptr;
}
@@ -600,6 +615,11 @@ class StreamChecker : public CheckerPreFn)
@@ -1038,10 +1059,13 @@ void StreamChecker::evalFscanf(const FnDescription
*Desc, const CallEvent &Call,
if (!StateNotFailed)
return;
-SmallVector EscArgs;
-for (auto EscArg : llvm::seq(2u, Call.getNumArgs()))
- EscArgs.push_back(EscArg);
-StateNotFailed = escapeArgs(StateNotFailed, C, Call, EscArgs);
+if (auto const *Callee = Call.getCalleeIdentifier();
+!Callee || !Callee->getName().equals("vfscanf")) {
+ SmallVector EscArgs;
+ for (auto EscArg : llvm::seq(2u, Call.getNumArgs()))
+EscArgs.push_back(EscArg);
+ StateNotFailed = escapeArgs(StateNotFailed, C, Call, EscArgs);
+}
if (StateNotFailed)
C.addTransition(StateNotFailed);
diff --git
a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
index 098a2208fecbe9..c26d3582149120 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
@@ -5,7 +5,7 @@
// suppressed.
#pragma clang system_header
-typedef struct __sFILE {
+typedef struct _FILE {
unsigned char *_p;
} FILE;
FILE *fopen(const char *restrict, const char *restrict) __asm("_" "fopen" );
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
index 7299b61353d460..720944abb8ad47 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
@@ -10,6 +10,8 @@
#define restrict /*restrict*/
#endif
+typedef struct _FILE FILE;
+
typedef __builtin_va_list va_list;
#define va_start(ap, param) __builtin_va_start(ap, param)
@@ -21,6 +23,10 @@ int vprintf (const char *restrict format, va_list arg);
int vsprintf (char *restrict s, const char *restrict format, va_list arg);
+int vfprintf(FILE *stream, const char *format, va_list ap);
+
+int vfscanf(FILE *stream, const char *format, va_list ap);
+
int some_library_function(int n, va_list arg);
// No warning from system header.
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h
b/clang/test/Analysis/Inputs/system-header-simulator.h
index 15986984802c0e..8fd51449ecc0a4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -73,6 +73,9 @@ int ferror(FILE *stream);
int fileno(FILE *stream);
int fflush(FILE *stream);
+
+int getc(FILE *stream);
+
size_t strlen(const char *);
char *strcpy(char *restrict, const char *restrict);
diff --git a/clang/test/Analysis/stream-invalidate.c
b/clang/test/Analysis/stream-invalidate.c
index 6745d11a2fe701..5046a356d0583d 100644
--- a/clang/test/Analysis/stream-invalidate.c
+++ b/clang/test/Analysis/stream-invalidate.c
@@ -4,6 +4,7 @@
// RUN: -analyzer-checker=debug.ExprInspection
#include "Inputs/system-header-simulator.h"
+#include "Inputs/system-header-simulator-for-valist.h"
void clang_analyzer_eval(int);
void clang_analyzer_dump(int);
@@ -145,3 +146,44 @@ void test_fgetpos() {
fclose(F);
}
+
+void test_fprintf() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ unsigned a = 42;
+ char *output = "HELLO";
+ int r = fprintf(F1, "%s\t%u\n", output, a);
+ // fprintf does not invalidate any of its input
+ // 69 is ascii for 'E'
+ clang_analyzer_dump(a); // ex
[clang] Reapply "[clang][analyzer] StreamChecker: Model getc, vfscanf, putc, … (PR #83281)
alejandro-alvarez-sonarsource wrote: @balazske @steakhal I broke some tests on aarch64 since there `__builtin_valist` is a struct and not a pointer to struct, so `lookupFn` was not matching `vfprintf` nor `vfscanf`, and the NULL pointer dereference triggered on the `fclose`. I have fixed that and modified `stream.c` to always run with four different target architectures, to make sure different `va_list` are handled. Not sure if that is the best way, though. https://github.com/llvm/llvm-project/pull/83281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] UnixAPIMisuseChecker Get O_CREAT from preprocessor (PR #81855)
https://github.com/alejandro-alvarez-sonarsource created
https://github.com/llvm/llvm-project/pull/81855
Now calling `open` with the `O_CREAT` flag and no mode parameter will raise an
issue in any system that defines `O_CREAT`.
The value for this flag is obtained after the full source code has been parsed,
leveraging `checkASTDecl`.
Hence, any `#define` or `#undefine` of `O_CREAT` following an `open` may alter
the results. Nevertheless, since redefining reserved identifiers is UB, this is
probably ok.
From 702ab6679c3b030c42e8d18acd10f438b4880eeb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Thu, 15 Feb 2024 13:22:40 +0100
Subject: [PATCH] [analyzer] UnixAPIMisuseChecker Get O_CREAT from preprocessor
Now calling `open` with the `O_CREAT` flag and no mode parameter will
raise an issue in any system that defines `O_CREAT`.
The value for this flag is obtained after the full source code
has been parsed, leveraging `checkASTDecl`.
Hence, any `#define` or `#undefine` of `O_CREAT` following an
`open` may alter the results. Nevertheless, since redefining
reserved identifiers is UB, this is probably ok.
---
.../Checkers/UnixAPIChecker.cpp | 42 +-
.../Inputs/expected-plists/unix-fns.c.plist | 678 +-
clang/test/Analysis/unix-fns.c| 2 +
3 files changed, 365 insertions(+), 357 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index b05ce610067cfa..19f1ca2dc824c9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -18,6 +18,7 @@
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
@@ -39,13 +40,18 @@ enum class OpenVariant {
namespace {
-class UnixAPIMisuseChecker : public Checker< check::PreStmt > {
+class UnixAPIMisuseChecker
+: public Checker,
+ check::ASTDecl> {
const BugType BT_open{this, "Improper use of 'open'", categories::UnixAPI};
const BugType BT_pthreadOnce{this, "Improper use of 'pthread_once'",
categories::UnixAPI};
mutable std::optional Val_O_CREAT;
public:
+ void checkASTDecl(const TranslationUnitDecl *TU, AnalysisManager &Mgr,
+BugReporter &BR) const;
+
void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
void CheckOpen(CheckerContext &C, const CallExpr *CE) const;
@@ -55,11 +61,8 @@ class UnixAPIMisuseChecker : public Checker<
check::PreStmt > {
void CheckOpenVariant(CheckerContext &C,
const CallExpr *CE, OpenVariant Variant) const;
- void ReportOpenBug(CheckerContext &C,
- ProgramStateRef State,
- const char *Msg,
+ void ReportOpenBug(CheckerContext &C, ProgramStateRef State, const char *Msg,
SourceRange SR) const;
-
};
class UnixAPIPortabilityChecker : public Checker< check::PreStmt > {
@@ -90,7 +93,21 @@ class UnixAPIPortabilityChecker : public Checker<
check::PreStmt > {
const char *fn) const;
};
-} //end anonymous namespace
+} // end anonymous namespace
+
+void UnixAPIMisuseChecker::checkASTDecl(const TranslationUnitDecl *TU,
+AnalysisManager &Mgr,
+BugReporter &) const {
+ // The definition of O_CREAT is platform specific.
+ // Try to get the macro value from the preprocessor.
+ Val_O_CREAT = tryExpandAsInteger("O_CREAT", Mgr.getPreprocessor());
+ // If we failed, fall-back to known values.
+ if (!Val_O_CREAT) {
+if (TU->getASTContext().getTargetInfo().getTriple().getVendor() ==
+llvm::Triple::Apple)
+ Val_O_CREAT = 0x0200;
+ }
+}
//===--===//
// "open" (man 2 open)
@@ -204,19 +221,8 @@ void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext
&C,
return;
}
- // The definition of O_CREAT is platform specific. We need a better way
- // of querying this information from the checking environment.
if (!Val_O_CREAT) {
-if (C.getASTContext().getTargetInfo().getTriple().getVendor()
- == llvm::Triple::Apple)
- Val_O_CREAT = 0x0200;
-else {
- // FIXME: We need a more general way of getting the O_CREAT value.
- // We could possibly grovel through the preprocessor state, but
- // that would require passing the Preprocessor object to the ExprEngine.
- // See also: MallocChecker.cpp / M_ZERO.
- return;
-}
+return;
}
// N
[clang] [analyzer] UnixAPIMisuseChecker Get O_CREAT from preprocessor (PR #81855)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/81855
From 702ab6679c3b030c42e8d18acd10f438b4880eeb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Thu, 15 Feb 2024 13:22:40 +0100
Subject: [PATCH] [analyzer] UnixAPIMisuseChecker Get O_CREAT from preprocessor
Now calling `open` with the `O_CREAT` flag and no mode parameter will
raise an issue in any system that defines `O_CREAT`.
The value for this flag is obtained after the full source code
has been parsed, leveraging `checkASTDecl`.
Hence, any `#define` or `#undefine` of `O_CREAT` following an
`open` may alter the results. Nevertheless, since redefining
reserved identifiers is UB, this is probably ok.
---
.../Checkers/UnixAPIChecker.cpp | 42 +-
.../Inputs/expected-plists/unix-fns.c.plist | 678 +-
clang/test/Analysis/unix-fns.c| 2 +
3 files changed, 365 insertions(+), 357 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index b05ce610067cfa..19f1ca2dc824c9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -18,6 +18,7 @@
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
@@ -39,13 +40,18 @@ enum class OpenVariant {
namespace {
-class UnixAPIMisuseChecker : public Checker< check::PreStmt > {
+class UnixAPIMisuseChecker
+: public Checker,
+ check::ASTDecl> {
const BugType BT_open{this, "Improper use of 'open'", categories::UnixAPI};
const BugType BT_pthreadOnce{this, "Improper use of 'pthread_once'",
categories::UnixAPI};
mutable std::optional Val_O_CREAT;
public:
+ void checkASTDecl(const TranslationUnitDecl *TU, AnalysisManager &Mgr,
+BugReporter &BR) const;
+
void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
void CheckOpen(CheckerContext &C, const CallExpr *CE) const;
@@ -55,11 +61,8 @@ class UnixAPIMisuseChecker : public Checker<
check::PreStmt > {
void CheckOpenVariant(CheckerContext &C,
const CallExpr *CE, OpenVariant Variant) const;
- void ReportOpenBug(CheckerContext &C,
- ProgramStateRef State,
- const char *Msg,
+ void ReportOpenBug(CheckerContext &C, ProgramStateRef State, const char *Msg,
SourceRange SR) const;
-
};
class UnixAPIPortabilityChecker : public Checker< check::PreStmt > {
@@ -90,7 +93,21 @@ class UnixAPIPortabilityChecker : public Checker<
check::PreStmt > {
const char *fn) const;
};
-} //end anonymous namespace
+} // end anonymous namespace
+
+void UnixAPIMisuseChecker::checkASTDecl(const TranslationUnitDecl *TU,
+AnalysisManager &Mgr,
+BugReporter &) const {
+ // The definition of O_CREAT is platform specific.
+ // Try to get the macro value from the preprocessor.
+ Val_O_CREAT = tryExpandAsInteger("O_CREAT", Mgr.getPreprocessor());
+ // If we failed, fall-back to known values.
+ if (!Val_O_CREAT) {
+if (TU->getASTContext().getTargetInfo().getTriple().getVendor() ==
+llvm::Triple::Apple)
+ Val_O_CREAT = 0x0200;
+ }
+}
//===--===//
// "open" (man 2 open)
@@ -204,19 +221,8 @@ void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext
&C,
return;
}
- // The definition of O_CREAT is platform specific. We need a better way
- // of querying this information from the checking environment.
if (!Val_O_CREAT) {
-if (C.getASTContext().getTargetInfo().getTriple().getVendor()
- == llvm::Triple::Apple)
- Val_O_CREAT = 0x0200;
-else {
- // FIXME: We need a more general way of getting the O_CREAT value.
- // We could possibly grovel through the preprocessor state, but
- // that would require passing the Preprocessor object to the ExprEngine.
- // See also: MallocChecker.cpp / M_ZERO.
- return;
-}
+return;
}
// Now check if oflags has O_CREAT set.
diff --git a/clang/test/Analysis/Inputs/expected-plists/unix-fns.c.plist
b/clang/test/Analysis/Inputs/expected-plists/unix-fns.c.plist
index 2594f3b6d097d5..d7913cbc338fd0 100644
--- a/clang/test/Analysis/Inputs/expected-plists/unix-fns.c.plist
+++ b/clang/test/Analysis/Inputs/expected-plists/unix-fns.c.plist
@@ -16,12 +16,12 @@
start
-
[clang] [analyzer] UnixAPIMisuseChecker Get O_CREAT from preprocessor (PR #81855)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/81855
From 702ab6679c3b030c42e8d18acd10f438b4880eeb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Thu, 15 Feb 2024 13:22:40 +0100
Subject: [PATCH 1/2] [analyzer] UnixAPIMisuseChecker Get O_CREAT from
preprocessor
Now calling `open` with the `O_CREAT` flag and no mode parameter will
raise an issue in any system that defines `O_CREAT`.
The value for this flag is obtained after the full source code
has been parsed, leveraging `checkASTDecl`.
Hence, any `#define` or `#undefine` of `O_CREAT` following an
`open` may alter the results. Nevertheless, since redefining
reserved identifiers is UB, this is probably ok.
---
.../Checkers/UnixAPIChecker.cpp | 42 +-
.../Inputs/expected-plists/unix-fns.c.plist | 678 +-
clang/test/Analysis/unix-fns.c| 2 +
3 files changed, 365 insertions(+), 357 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index b05ce610067cfa..19f1ca2dc824c9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -18,6 +18,7 @@
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
@@ -39,13 +40,18 @@ enum class OpenVariant {
namespace {
-class UnixAPIMisuseChecker : public Checker< check::PreStmt > {
+class UnixAPIMisuseChecker
+: public Checker,
+ check::ASTDecl> {
const BugType BT_open{this, "Improper use of 'open'", categories::UnixAPI};
const BugType BT_pthreadOnce{this, "Improper use of 'pthread_once'",
categories::UnixAPI};
mutable std::optional Val_O_CREAT;
public:
+ void checkASTDecl(const TranslationUnitDecl *TU, AnalysisManager &Mgr,
+BugReporter &BR) const;
+
void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
void CheckOpen(CheckerContext &C, const CallExpr *CE) const;
@@ -55,11 +61,8 @@ class UnixAPIMisuseChecker : public Checker<
check::PreStmt > {
void CheckOpenVariant(CheckerContext &C,
const CallExpr *CE, OpenVariant Variant) const;
- void ReportOpenBug(CheckerContext &C,
- ProgramStateRef State,
- const char *Msg,
+ void ReportOpenBug(CheckerContext &C, ProgramStateRef State, const char *Msg,
SourceRange SR) const;
-
};
class UnixAPIPortabilityChecker : public Checker< check::PreStmt > {
@@ -90,7 +93,21 @@ class UnixAPIPortabilityChecker : public Checker<
check::PreStmt > {
const char *fn) const;
};
-} //end anonymous namespace
+} // end anonymous namespace
+
+void UnixAPIMisuseChecker::checkASTDecl(const TranslationUnitDecl *TU,
+AnalysisManager &Mgr,
+BugReporter &) const {
+ // The definition of O_CREAT is platform specific.
+ // Try to get the macro value from the preprocessor.
+ Val_O_CREAT = tryExpandAsInteger("O_CREAT", Mgr.getPreprocessor());
+ // If we failed, fall-back to known values.
+ if (!Val_O_CREAT) {
+if (TU->getASTContext().getTargetInfo().getTriple().getVendor() ==
+llvm::Triple::Apple)
+ Val_O_CREAT = 0x0200;
+ }
+}
//===--===//
// "open" (man 2 open)
@@ -204,19 +221,8 @@ void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext
&C,
return;
}
- // The definition of O_CREAT is platform specific. We need a better way
- // of querying this information from the checking environment.
if (!Val_O_CREAT) {
-if (C.getASTContext().getTargetInfo().getTriple().getVendor()
- == llvm::Triple::Apple)
- Val_O_CREAT = 0x0200;
-else {
- // FIXME: We need a more general way of getting the O_CREAT value.
- // We could possibly grovel through the preprocessor state, but
- // that would require passing the Preprocessor object to the ExprEngine.
- // See also: MallocChecker.cpp / M_ZERO.
- return;
-}
+return;
}
// Now check if oflags has O_CREAT set.
diff --git a/clang/test/Analysis/Inputs/expected-plists/unix-fns.c.plist
b/clang/test/Analysis/Inputs/expected-plists/unix-fns.c.plist
index 2594f3b6d097d5..d7913cbc338fd0 100644
--- a/clang/test/Analysis/Inputs/expected-plists/unix-fns.c.plist
+++ b/clang/test/Analysis/Inputs/expected-plists/unix-fns.c.plist
@@ -16,12 +16,12 @@
start
[clang] [analyzer] UnixAPIMisuseChecker Get O_CREAT from preprocessor (PR #81855)
alejandro-alvarez-sonarsource wrote: Thanks! I have added a new test file where the values are those of [FreeRTOS](https://www.freertos.org/Documentation/api-ref/POSIX/fcntl_8h.html) https://github.com/llvm/llvm-project/pull/81855 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] StreamChecker: add more APIs, invalidate fscanf args (PR #82476)
https://github.com/alejandro-alvarez-sonarsource created
https://github.com/llvm/llvm-project/pull/82476
1. Model `getc`, `vfscanf`, `putc`, `vfprintf`.
2. `fscanf` invalidates all arguments after the format string.
Also, add tests for `ftello` and `fseeko`.
From a21881d82fe3674b344d4a3807e9d2590c98ce93 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Tue, 14 Nov 2023 09:28:45 +0100
Subject: [PATCH] [clang][analyzer] StreamChecker: add more APIs, invalidate
fscanf args
1. Model getc, vfscanf, putc, vfprintf.
2. fscanf invalidates all arguments after the format string.
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 39 +-
...ystem-header-simulator-for-simple-stream.h | 2 +-
.../system-header-simulator-for-valist.h | 4 +
.../Analysis/Inputs/system-header-simulator.h | 3 +
clang/test/Analysis/stream.c | 128 ++
5 files changed, 174 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index a070f451694a3b..7938a0d30a91a3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,8 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/SmallVector.h"
#include
#include
@@ -171,7 +173,7 @@ using FnCheck = std::function;
using ArgNoTy = unsigned int;
-static const ArgNoTy ArgNone = std::numeric_limits::max();
+const ArgNoTy ArgNone = std::numeric_limits::max();
struct FnDescription {
FnCheck PreFn;
@@ -179,6 +181,26 @@ struct FnDescription {
ArgNoTy StreamArgNo;
};
+[[nodiscard]] ProgramStateRef
+escapeArgsAfterIndex(ProgramStateRef State, CheckerContext &C,
+ const CallEvent &Call, unsigned FirstEscapingArgIndex) {
+ const auto *CE = Call.getOriginExpr();
+ assert(CE);
+
+ if (Call.getNumArgs() <= FirstEscapingArgIndex)
+return State;
+
+ SmallVector EscapingArgs;
+ EscapingArgs.reserve(Call.getNumArgs() - FirstEscapingArgIndex);
+ for (auto EscArgIdx :
+ llvm::seq(FirstEscapingArgIndex, Call.getNumArgs()))
+EscapingArgs.push_back(Call.getArgSVal(EscArgIdx));
+ State = State->invalidateRegions(EscapingArgs, CE, C.blockCount(),
+ C.getLocationContext(),
+ /*CausesPointerEscape=*/false);
+ return State;
+}
+
/// Get the value of the stream argument out of the passed call event.
/// The call should contain a function that is described by Desc.
SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
@@ -396,6 +418,18 @@ class StreamChecker : public Checker FnTestDescriptions = {
@@ -997,6 +1031,9 @@ void StreamChecker::evalFscanf(const FnDescription *Desc,
const CallEvent &Call,
if (!E.Init(Desc, Call, C, State))
return;
+ // The pointers passed to fscanf escape and get invalidated.
+ State = escapeArgsAfterIndex(State, C, Call, /*FirstEscapingArgIndex=*/2);
+
// Add the success state.
// In this context "success" means there is not an EOF or other read error
// before any item is matched in 'fscanf'. But there may be match failure,
diff --git
a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
index 098a2208fecbe9..c26d3582149120 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
@@ -5,7 +5,7 @@
// suppressed.
#pragma clang system_header
-typedef struct __sFILE {
+typedef struct _FILE {
unsigned char *_p;
} FILE;
FILE *fopen(const char *restrict, const char *restrict) __asm("_" "fopen" );
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
index 7299b61353d460..87688bd8b312f4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
@@ -10,6 +10,8 @@
#define restrict /*restrict*/
#endif
+typedef struct _FILE FILE;
+
typedef __builtin_va_list va_list;
#define va_start(ap, param) __builtin_va_start(ap, param)
@@ -21,6 +23,8 @@ int vprintf (const char *restrict format, va_list arg);
int vsprintf (char *restrict s, const char *restrict format, va_list arg);
+int vfscanf(FILE *stream, const char *format, va_list ap);
+
int some_library_function(int n, va_list arg);
// No warning from system header.
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h
b/clang/test/Analysis/Inputs/system-header-simulator.h
index 15986984802c0e..8fd51449ecc0a4 100644
--- a/cla
[clang] [clang][analyzer] StreamChecker: Add more APIs, invalidate fscanf args (PR #82476)
https://github.com/alejandro-alvarez-sonarsource edited https://github.com/llvm/llvm-project/pull/82476 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] StreamChecker: Add more APIs, invalidate fscanf args (PR #82476)
alejandro-alvarez-sonarsource wrote: > How does this relate to the other PR #79470 (why comes this new patch when > there is another for the same problem)? I think it is better to first commit > #79470, then add remaining functions `getc`, `putc`, `vfscanf`, `vfprintf`. > These should be added anyway in a separate change because it is less related > to invalidation. I started the work on this patch before #79470 was submitted, and didn't double-check if someone had tackled the same in the meantime. My bad. I agree on waiting for that one to land and strip this one down to the new set of functions. https://github.com/llvm/llvm-project/pull/82476 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] StreamChecker: Add more APIs, invalidate fscanf args (PR #82476)
@@ -339,3 +363,107 @@ void fflush_on_open_failed_stream(void) {
}
fclose(F);
}
+
+void test_fscanf_eof() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+return;
+
+ int a;
+ unsigned b;
+ int ret = fscanf(F1, "%d %u", &a, &b);
+ char c = fgetc(F1); // expected-warning {{Read function called when stream
is in EOF state. Function has no effect}}
+ // expected-warning@-1 {{File position of the stream might be
'indeterminate' after a failed operation. Can cause undefined behavior}}
alejandro-alvarez-sonarsource wrote:
Yes, there is a note such as
```
clang/test/Analysis/stream.c Line 374: Assuming stream reaches end-of-file here
```
https://github.com/llvm/llvm-project/pull/82476
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] StreamChecker: Add more APIs, invalidate fscanf args (PR #82476)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/82476
From a21881d82fe3674b344d4a3807e9d2590c98ce93 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Tue, 14 Nov 2023 09:28:45 +0100
Subject: [PATCH 1/3] [clang][analyzer] StreamChecker: add more APIs,
invalidate fscanf args
1. Model getc, vfscanf, putc, vfprintf.
2. fscanf invalidates all arguments after the format string.
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 39 +-
...ystem-header-simulator-for-simple-stream.h | 2 +-
.../system-header-simulator-for-valist.h | 4 +
.../Analysis/Inputs/system-header-simulator.h | 3 +
clang/test/Analysis/stream.c | 128 ++
5 files changed, 174 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index a070f451694a3b..7938a0d30a91a3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -21,6 +21,8 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/SmallVector.h"
#include
#include
@@ -171,7 +173,7 @@ using FnCheck = std::function;
using ArgNoTy = unsigned int;
-static const ArgNoTy ArgNone = std::numeric_limits::max();
+const ArgNoTy ArgNone = std::numeric_limits::max();
struct FnDescription {
FnCheck PreFn;
@@ -179,6 +181,26 @@ struct FnDescription {
ArgNoTy StreamArgNo;
};
+[[nodiscard]] ProgramStateRef
+escapeArgsAfterIndex(ProgramStateRef State, CheckerContext &C,
+ const CallEvent &Call, unsigned FirstEscapingArgIndex) {
+ const auto *CE = Call.getOriginExpr();
+ assert(CE);
+
+ if (Call.getNumArgs() <= FirstEscapingArgIndex)
+return State;
+
+ SmallVector EscapingArgs;
+ EscapingArgs.reserve(Call.getNumArgs() - FirstEscapingArgIndex);
+ for (auto EscArgIdx :
+ llvm::seq(FirstEscapingArgIndex, Call.getNumArgs()))
+EscapingArgs.push_back(Call.getArgSVal(EscArgIdx));
+ State = State->invalidateRegions(EscapingArgs, CE, C.blockCount(),
+ C.getLocationContext(),
+ /*CausesPointerEscape=*/false);
+ return State;
+}
+
/// Get the value of the stream argument out of the passed call event.
/// The call should contain a function that is described by Desc.
SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
@@ -396,6 +418,18 @@ class StreamChecker : public Checker FnTestDescriptions = {
@@ -997,6 +1031,9 @@ void StreamChecker::evalFscanf(const FnDescription *Desc,
const CallEvent &Call,
if (!E.Init(Desc, Call, C, State))
return;
+ // The pointers passed to fscanf escape and get invalidated.
+ State = escapeArgsAfterIndex(State, C, Call, /*FirstEscapingArgIndex=*/2);
+
// Add the success state.
// In this context "success" means there is not an EOF or other read error
// before any item is matched in 'fscanf'. But there may be match failure,
diff --git
a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
index 098a2208fecbe9..c26d3582149120 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
@@ -5,7 +5,7 @@
// suppressed.
#pragma clang system_header
-typedef struct __sFILE {
+typedef struct _FILE {
unsigned char *_p;
} FILE;
FILE *fopen(const char *restrict, const char *restrict) __asm("_" "fopen" );
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
index 7299b61353d460..87688bd8b312f4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h
@@ -10,6 +10,8 @@
#define restrict /*restrict*/
#endif
+typedef struct _FILE FILE;
+
typedef __builtin_va_list va_list;
#define va_start(ap, param) __builtin_va_start(ap, param)
@@ -21,6 +23,8 @@ int vprintf (const char *restrict format, va_list arg);
int vsprintf (char *restrict s, const char *restrict format, va_list arg);
+int vfscanf(FILE *stream, const char *format, va_list ap);
+
int some_library_function(int n, va_list arg);
// No warning from system header.
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h
b/clang/test/Analysis/Inputs/system-header-simulator.h
index 15986984802c0e..8fd51449ecc0a4 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -73,6 +73,9 @@ int ferror(FILE *stream);
[clang] [clang][analyzer] Fix argument invalidations in StreamChecker. (PR #79470)
@@ -763,6 +779,11 @@ void StreamChecker::evalFreadFwrite(const FnDescription
*Desc,
return;
}
+ // At read, invalidate the buffer in any case of error or success,
+ // except if EOF was already present.
+ if (IsFread && (OldSS->ErrorState != ErrorFEof))
+State = escapeArgs(State, C, Call, {0});
alejandro-alvarez-sonarsource wrote:
From what I can tell, the lambda `UpdateBufferRegionForFread` is already used
to take care of this in a more fine-grained manner. For instance:
```cpp
int buffer[10];
buffer[5] = 42;
if (1 == fread(buffer, sizeof(int), 5, fd)) {
assert(buffer[5] == 42);
}
```
Before this change, the assertion would pass, since lambda took `nmemb` into
account. With this change, the whole buffer is invalidated.
https://github.com/llvm/llvm-project/pull/79470
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Fix argument invalidations in StreamChecker. (PR #79470)
https://github.com/alejandro-alvarez-sonarsource edited https://github.com/llvm/llvm-project/pull/79470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Fix argument invalidations in StreamChecker. (PR #79470)
https://github.com/alejandro-alvarez-sonarsource edited https://github.com/llvm/llvm-project/pull/79470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Fix argument invalidations in StreamChecker. (PR #79470)
@@ -763,6 +779,11 @@ void StreamChecker::evalFreadFwrite(const FnDescription
*Desc,
return;
}
+ // At read, invalidate the buffer in any case of error or success,
+ // except if EOF was already present.
+ if (IsFread && (OldSS->ErrorState != ErrorFEof))
+State = escapeArgs(State, C, Call, {0});
alejandro-alvarez-sonarsource wrote:
It turns out I was comparing with our downstream copy of v17, where
`UpdateBufferRegionForFread` is defined.
So it can be done for the case of arrays. I have double-checked with the
original author and he agrees it can be upstreamed. Let me uplift the patch and
then I will share it with you.
https://github.com/llvm/llvm-project/pull/79470
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Fix argument invalidations in StreamChecker. (PR #79470)
@@ -763,6 +779,11 @@ void StreamChecker::evalFreadFwrite(const FnDescription
*Desc,
return;
}
+ // At read, invalidate the buffer in any case of error or success,
+ // except if EOF was already present.
+ if (IsFread && (OldSS->ErrorState != ErrorFEof))
+State = escapeArgs(State, C, Call, {0});
alejandro-alvarez-sonarsource wrote:
Here it is:
https://gist.github.com/alejandro-alvarez-sonarsource/48edec4debc8912a6485f989b2a6f0db
I have rebased it on top of 4f12f47550eee85447c9ec37d27a20c6593d3d40 and run
check-clang to make sure I didn't break anything.
https://github.com/llvm/llvm-project/pull/79470
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Fix argument invalidations in StreamChecker. (PR #79470)
@@ -763,6 +779,11 @@ void StreamChecker::evalFreadFwrite(const FnDescription
*Desc,
return;
}
+ // At read, invalidate the buffer in any case of error or success,
+ // except if EOF was already present.
+ if (IsFread && (OldSS->ErrorState != ErrorFEof))
+State = escapeArgs(State, C, Call, {0});
alejandro-alvarez-sonarsource wrote:
Ok, let’s do that, and send this patch as a separate pr. @steakhal LGTM.
https://github.com/llvm/llvm-project/pull/79470
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Fix crash in Sema::FindInstantiatedDecl (PR #96509)
alejandro-alvarez-sonarsource wrote: Ping (@cor3ntin, @Endilll ?) https://github.com/llvm/llvm-project/pull/96509 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Fix crash in Sema::FindInstantiatedDecl (PR #96509)
https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/96509
From 96b0c2c18c197a1ce03d31b01c14d1b18348e9ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Thu, 1 Jun 2023 16:30:54 +0200
Subject: [PATCH 1/2] [Sema] Fix crash in Sema::FindInstantiatedDecl
when looking for a template instantiation with a non-type
parameter of unknown type and with a default value.
This can happen when a template non-type parameter has a broken
expression that gets replaced by a `RecoveryExpr`.
---
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 2 +-
.../instantiate-template-broken-nontype-default.cpp | 12
2 files changed, 13 insertions(+), 1 deletion(-)
create mode 100644
clang/test/SemaCXX/instantiate-template-broken-nontype-default.cpp
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 0681520764d9a..999fec0ebb259 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6300,7 +6300,7 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc,
NamedDecl *D,
getTrivialTemplateArgumentLoc(UnpackedArg, QualType(), Loc));
}
QualType T = CheckTemplateIdType(TemplateName(TD), Loc, Args);
- if (T.isNull())
+ if (T.isNull() || T->containsErrors())
return nullptr;
CXXRecordDecl *SubstRecord = T->getAsCXXRecordDecl();
diff --git a/clang/test/SemaCXX/instantiate-template-broken-nontype-default.cpp
b/clang/test/SemaCXX/instantiate-template-broken-nontype-default.cpp
new file mode 100644
index 0..a644cc3d057cb
--- /dev/null
+++ b/clang/test/SemaCXX/instantiate-template-broken-nontype-default.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+
+constexpr Missing a = 0; // expected-error {{unknown type name 'Missing'}}
+
+template < typename T, Missing b = a> // expected-error {{unknown type name
'Missing'}}
+class Klass { // expected-note {{candidate template ignored: could not match
'Klass' against 'int'}}
+ Klass(T); // expected-note {{candidate template ignored: substitution
failure [with T = int, b = ()]}}
+};
+
+Klass foo{5}; // no-crash \
+ expected-error {{no viable constructor or deduction guide for
deduction of template arguments of 'Klass'}}
+
From 9111797355ba860d0c6b62fde3a7b1e347a71dc6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Fri, 5 Jul 2024 15:12:47 +0200
Subject: [PATCH 2/2] Move the check to `CheckTemplateIdType()`
---
clang/lib/Sema/SemaTemplate.cpp| 3 ++-
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index a032e3ec6f635..2ea18ec82b958 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -4700,7 +4700,8 @@ QualType Sema::CheckTemplateIdType(TemplateName Name,
// template struct A;
CanonType = Context.getCanonicalTemplateSpecializationType(
Name, CanonicalConverted);
-
+if (CanonType->containsErrors())
+ return QualType();
// This might work out to be a current instantiation, in which
// case the canonical type needs to be the InjectedClassNameType.
//
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 999fec0ebb259..0681520764d9a 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6300,7 +6300,7 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc,
NamedDecl *D,
getTrivialTemplateArgumentLoc(UnpackedArg, QualType(), Loc));
}
QualType T = CheckTemplateIdType(TemplateName(TD), Loc, Args);
- if (T.isNull() || T->containsErrors())
+ if (T.isNull())
return nullptr;
CXXRecordDecl *SubstRecord = T->getAsCXXRecordDecl();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Fix crash in Sema::FindInstantiatedDecl (PR #96509)
@@ -6300,7 +6300,7 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, NamedDecl *D, getTrivialTemplateArgumentLoc(UnpackedArg, QualType(), Loc)); } QualType T = CheckTemplateIdType(TemplateName(TD), Loc, Args); - if (T.isNull()) + if (T.isNull() || T->containsErrors()) alejandro-alvarez-sonarsource wrote: There is no reason besides that it is where I saw the crash. I have moved into `CheckTemplateIdType`. https://github.com/llvm/llvm-project/pull/96509 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix wrong assumption in `tryDiagnoseOverloadedCast` (PR #108021)
https://github.com/alejandro-alvarez-sonarsource created
https://github.com/llvm/llvm-project/pull/108021
A constructor may have failed to be used due to a broken templated dependent
parameter. The `InitializationSequence` itself can succeed.
Due to the assumption that it is in a failed state, it can either cause an
assertion failure, or undefined behavior in release mode, since
`sequence.Failure` is not initialized.
From b414b6e184fc566efd6a2dbe631ee68a25b461a1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Fri, 16 Jun 2023 18:01:33 +0200
Subject: [PATCH] [clang][Sema] Fix wrong assumption in
`tryDiagnoseOverloadedCast`
A constructor may have failed to be used due to a broken templated
dependent parameter. The `InitializationSequence` itself can
succeed.
Due to the assumption that it is in a failed state, it can
either cause an assertion failure, or undefined behavior in release
mode, since `sequence.Failure` is not initialized.
---
clang/lib/Sema/SemaCast.cpp | 7 -
.../cxx-bad-cast-diagnose-broken-template.cpp | 31 +++
2 files changed, 37 insertions(+), 1 deletion(-)
create mode 100644 clang/test/Parser/cxx-bad-cast-diagnose-broken-template.cpp
diff --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index f01b22a72915c8..6ac6201843476b 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -446,7 +446,12 @@ static bool tryDiagnoseOverloadedCast(Sema &S, CastType CT,
: InitializationKind::CreateCast(/*type range?*/ range);
InitializationSequence sequence(S, entity, initKind, src);
- assert(sequence.Failed() && "initialization succeeded on second try?");
+ // It could happen that a constructor failed to be used because
+ // it requires a temporary of a broken type. Still, it will be found when
+ // looking for a match.
+ if (!sequence.Failed())
+return false;
+
switch (sequence.getFailureKind()) {
default: return false;
diff --git a/clang/test/Parser/cxx-bad-cast-diagnose-broken-template.cpp
b/clang/test/Parser/cxx-bad-cast-diagnose-broken-template.cpp
new file mode 100644
index 00..f9fc3e6082da10
--- /dev/null
+++ b/clang/test/Parser/cxx-bad-cast-diagnose-broken-template.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -verify %s
+
+template< typename>
+struct IsConstCharArray
+{
+ static const bool value = false;
+};
+
+template< int N >
+struct IsConstCharArray< const char[ N ] >
+{
+ typedef char CharType;
+ static const bool value = true;
+ static const missing_int_t length = N - 1; // expected-error {{unknown type
name 'missing_int_t'}}
+};
+
+class String {
+public:
+ template
+ String(T& str, typename IsConstCharArray::CharType = 0);
+};
+
+
+class Exception {
+public:
+ Exception(String const&);
+};
+
+void foo() {
+ throw Exception("some error"); // expected-error {{functional-style cast
from 'const char[11]' to 'Exception' is not allowed}}
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix assertion in `tryDiagnoseOverloadedCast` (PR #108021)
https://github.com/alejandro-alvarez-sonarsource edited https://github.com/llvm/llvm-project/pull/108021 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
