[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID: In-Reply-To: https://github.com/steakhal closed 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 =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?= Message-ID: In-Reply-To: steakhal wrote: Rebase merged. 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: 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] [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 ,
BugReporter ) const;
- void checkPreStmt(const CallExpr *CE, CheckerContext ) const;
+ void checkPreCall(const CallEvent , CheckerContext ) const;
- void CheckOpen(CheckerContext , const CallExpr *CE) const;
- void CheckOpenAt(CheckerContext , const CallExpr *CE) const;
- void CheckPthreadOnce(CheckerContext , const CallExpr *CE) const;
+ void CheckOpen(CheckerContext , const CallEvent ) const;
+ void CheckOpenAt(CheckerContext , const CallEvent ) const;
+ void CheckPthreadOnce(CheckerContext , const CallEvent ) const;
- void CheckOpenVariant(CheckerContext ,
-const CallExpr *CE, OpenVariant Variant) const;
+ void CheckOpenVariant(CheckerContext , const CallEvent ,
+OpenVariant Variant) const;
void ReportOpenBug(CheckerContext , 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 ,
CheckerContext ) 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 ,
ProgramStateRef State,
@@ -152,17 +152,17 @@ void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext
,
}
void UnixAPIMisuseChecker::CheckOpen(CheckerContext ,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::Open);
+ const CallEvent ) const {
+ CheckOpenVariant(C, Call, OpenVariant::Open);
}
void UnixAPIMisuseChecker::CheckOpenAt(CheckerContext ,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::OpenAt);
+ const CallEvent ) const {
+ CheckOpenVariant(C, Call, OpenVariant::OpenAt);
}
void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext ,
-const CallExpr *CE,
+const CallEvent ,
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 ,
ProgramStateRef state = C.getState();
- if (CE->getNumArgs() < MinArgCount) {
+ if (Call.getNumArgs() < MinArgCount) {
// The frontend should issue a warning for this case. Just return.
return;
- }
[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(, , NULL); // expected-warning {{Stream pointer might be
NULL}}
+}
+
+void getdelim_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getdelim(, , '\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(, , 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(, , 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(, , file);
+
+ if (r > -1) {
+clang_analyzer_eval((ssize_t)n > r); // expected-warning{{TRUE}}
+ clang_analyzer_eval(buffer != NULL); // expected-warning{{TRUE}}
balazske wrote:
This looks like an indentation problem.
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,75 @@ void fflush_on_open_failed_stream(void) {
}
fclose(F);
}
+
+void getline_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getline(, , NULL); // expected-warning {{Stream pointer might be
NULL}}
+}
+
+void getdelim_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getdelim(, , '\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(, , 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(, , 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(, , 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 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 ,
BugReporter ) const;
- void checkPreStmt(const CallExpr *CE, CheckerContext ) const;
+ void checkPreCall(const CallEvent , CheckerContext ) const;
- void CheckOpen(CheckerContext , const CallExpr *CE) const;
- void CheckOpenAt(CheckerContext , const CallExpr *CE) const;
- void CheckPthreadOnce(CheckerContext , const CallExpr *CE) const;
+ void CheckOpen(CheckerContext , const CallEvent ) const;
+ void CheckOpenAt(CheckerContext , const CallEvent ) const;
+ void CheckPthreadOnce(CheckerContext , const CallEvent ) const;
- void CheckOpenVariant(CheckerContext ,
-const CallExpr *CE, OpenVariant Variant) const;
+ void CheckOpenVariant(CheckerContext , const CallEvent ,
+OpenVariant Variant) const;
void ReportOpenBug(CheckerContext , 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 ,
CheckerContext ) 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 ,
ProgramStateRef State,
@@ -152,17 +152,17 @@ void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext
,
}
void UnixAPIMisuseChecker::CheckOpen(CheckerContext ,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::Open);
+ const CallEvent ) const {
+ CheckOpenVariant(C, Call, OpenVariant::Open);
}
void UnixAPIMisuseChecker::CheckOpenAt(CheckerContext ,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::OpenAt);
+ const CallEvent ) const {
+ CheckOpenVariant(C, Call, OpenVariant::OpenAt);
}
void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext ,
-const CallExpr *CE,
+const CallEvent ,
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 ,
ProgramStateRef state = C.getState();
- if (CE->getNumArgs() < MinArgCount) {
+ if (Call.getNumArgs() < MinArgCount) {
// The frontend should issue a warning for this case. Just return.
return;
- } else if
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
https://github.com/balazske approved this pull request. I did not find more issues (at least in `StreamChecker` and its tests). But did not check in detail the `UnixAPIChecker` part and 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)
@@ -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(, , NULL); // expected-warning {{Stream pointer might be
NULL}}
+}
+
+void getdelim_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getdelim(, , '\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(, , 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(, , 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(, , 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(, , 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(, , 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/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 ,
BugReporter ) const;
- void checkPreStmt(const CallExpr *CE, CheckerContext ) const;
+ void checkPreCall(const CallEvent , CheckerContext ) const;
- void CheckOpen(CheckerContext , const CallExpr *CE) const;
- void CheckOpenAt(CheckerContext , const CallExpr *CE) const;
- void CheckPthreadOnce(CheckerContext , const CallExpr *CE) const;
+ void CheckOpen(CheckerContext , const CallEvent ) const;
+ void CheckOpenAt(CheckerContext , const CallEvent ) const;
+ void CheckPthreadOnce(CheckerContext , const CallEvent ) const;
- void CheckOpenVariant(CheckerContext ,
-const CallExpr *CE, OpenVariant Variant) const;
+ void CheckOpenVariant(CheckerContext , const CallEvent ,
+OpenVariant Variant) const;
void ReportOpenBug(CheckerContext , 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 ,
CheckerContext ) 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 ,
ProgramStateRef State,
@@ -152,17 +152,17 @@ void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext
,
}
void UnixAPIMisuseChecker::CheckOpen(CheckerContext ,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::Open);
+ const CallEvent ) const {
+ CheckOpenVariant(C, Call, OpenVariant::Open);
}
void UnixAPIMisuseChecker::CheckOpenAt(CheckerContext ,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::OpenAt);
+ const CallEvent ) const {
+ CheckOpenVariant(C, Call, OpenVariant::OpenAt);
}
void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext ,
-const CallExpr *CE,
+const CallEvent ,
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 ,
ProgramStateRef state = C.getState();
- if (CE->getNumArgs() < MinArgCount) {
+ if (Call.getNumArgs() < MinArgCount) {
// The frontend should issue a warning for this case. Just return.
return;
- } else if
[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(, , NULL); // expected-warning {{Stream pointer might be
NULL}}
+}
+
+void getdelim_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getdelim(, , '\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(, , 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(, , 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(, , 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(, , 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(, , 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) {
balazske wrote:
My concern was mostly that `buffer` is an argument and `n` is not and this
looks not relevant for this test case, so if a `char *buffer = NULL` would be
added (and the argument removed) it would look better. Otherwise an `assert`
can be added to the code after the place where the second assumption on return
value is made. I think too that the invalidation of `n` solves this problem.
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(, , NULL); // expected-warning {{Stream pointer might be
NULL}}
+}
+
+void getdelim_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getdelim(, , '\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(, , 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(, , 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(, , 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(, , 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(, , 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)
@@ -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(, , NULL); // expected-warning {{Stream pointer might be
NULL}}
+}
+
+void getdelim_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getdelim(, , '\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(, , 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(, , NULL); // expected-warning {{Stream pointer might be
NULL}}
+}
+
+void getdelim_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getdelim(, , '\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(, , 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(, , 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(, , 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(, , 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(, , 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(, , NULL); // expected-warning {{Stream pointer might be
NULL}}
+}
+
+void getdelim_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getdelim(, , '\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(, , 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)
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 ,
BugReporter ) const;
- void checkPreStmt(const CallExpr *CE, CheckerContext ) const;
+ void checkPreCall(const CallEvent , CheckerContext ) const;
- void CheckOpen(CheckerContext , const CallExpr *CE) const;
- void CheckOpenAt(CheckerContext , const CallExpr *CE) const;
- void CheckPthreadOnce(CheckerContext , const CallExpr *CE) const;
+ void CheckOpen(CheckerContext , const CallEvent ) const;
+ void CheckOpenAt(CheckerContext , const CallEvent ) const;
+ void CheckPthreadOnce(CheckerContext , const CallEvent ) const;
- void CheckOpenVariant(CheckerContext ,
-const CallExpr *CE, OpenVariant Variant) const;
+ void CheckOpenVariant(CheckerContext , const CallEvent ,
+OpenVariant Variant) const;
void ReportOpenBug(CheckerContext , 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 ,
CheckerContext ) 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 ,
ProgramStateRef State,
@@ -152,17 +152,17 @@ void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext
,
}
void UnixAPIMisuseChecker::CheckOpen(CheckerContext ,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::Open);
+ const CallEvent ) const {
+ CheckOpenVariant(C, Call, OpenVariant::Open);
}
void UnixAPIMisuseChecker::CheckOpenAt(CheckerContext ,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::OpenAt);
+ const CallEvent ) const {
+ CheckOpenVariant(C, Call, OpenVariant::OpenAt);
}
void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext ,
-const CallExpr *CE,
+const CallEvent ,
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 ,
ProgramStateRef state = C.getState();
- if (CE->getNumArgs() < MinArgCount) {
+ if (Call.getNumArgs() < MinArgCount) {
// The frontend should issue a warning for this case. Just return.
return;
- } else if
[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(, , NULL); // expected-warning {{Stream pointer might be
NULL}}
+}
+
+void getdelim_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getdelim(, , '\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(, , file);
+
+ if (line[0] == '\0') {} // expected-warning {{The left operand of '==' is a
garbage value}}
balazske wrote:
I like better if this check is included only in the failure case:
```
if (getline(, , file) == -1) {
if (line[0] == '\0') {} // expected-warning {{The left operand of '==' is a
garbage value}}
} else {
if (line[0] == '\0') {} // no warning
}
```
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(, , NULL); // expected-warning {{Stream pointer might be
NULL}}
+}
+
+void getdelim_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getdelim(, , '\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(, , file);
+
+ if (line[0] == '\0') {} // expected-warning {{The left operand of '==' is a
garbage value}}
+
+ free(line);
+ fclose(file);
+}
+
+void getline_after_eof() {
balazske wrote:
I think `getline_after_eof`, `getline_feof`, `getline_feof_check` are checking
not much more cases than `error_getline` (and `error_getdelim`) in
_stream_error.c_ and can be removed. Even if not removed, such tests (that
check `feof` and `ferror` and related warnings) should put into
_stream_error.c_.
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(, , NULL); // expected-warning {{Stream pointer might be
NULL}}
+}
+
+void getdelim_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getdelim(, , '\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(, , 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(, , 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(, , 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(, , 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(, , 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) {
balazske wrote:
This test looks interesting: It is not really different from the previous one,
except that the buffer is not an initialized value. Because `n` is -1 the
buffer should be assumed to be NULL before the call. This is not a realistic
code because `n` should be an indication of the buffer size but this value is
not known to the function. I think this test (in this form) is not needed.
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(, , NULL); // expected-warning {{Stream pointer might be
NULL}}
+}
+
+void getdelim_null_file() {
+ char *buffer = NULL;
+ size_t n = 0;
+ getdelim(, , '\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(, , 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(, , 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(, , 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(, , 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(, , 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}}
balazske wrote:
Should check that `buffer != NULL` is true.
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)
@@ -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)
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)
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 ,
BugReporter ) const;
- void checkPreStmt(const CallExpr *CE, CheckerContext ) const;
+ void checkPreCall(const CallEvent , CheckerContext ) const;
- void CheckOpen(CheckerContext , const CallExpr *CE) const;
- void CheckOpenAt(CheckerContext , const CallExpr *CE) const;
- void CheckPthreadOnce(CheckerContext , const CallExpr *CE) const;
+ void CheckOpen(CheckerContext , const CallEvent ) const;
+ void CheckOpenAt(CheckerContext , const CallEvent ) const;
+ void CheckPthreadOnce(CheckerContext , const CallEvent ) const;
- void CheckOpenVariant(CheckerContext ,
-const CallExpr *CE, OpenVariant Variant) const;
+ void CheckOpenVariant(CheckerContext , const CallEvent ,
+OpenVariant Variant) const;
void ReportOpenBug(CheckerContext , 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 ,
CheckerContext ) 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 ,
ProgramStateRef State,
@@ -152,17 +152,17 @@ void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext
,
}
void UnixAPIMisuseChecker::CheckOpen(CheckerContext ,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::Open);
+ const CallEvent ) const {
+ CheckOpenVariant(C, Call, OpenVariant::Open);
}
void UnixAPIMisuseChecker::CheckOpenAt(CheckerContext ,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::OpenAt);
+ const CallEvent ) const {
+ CheckOpenVariant(C, Call, OpenVariant::OpenAt);
}
void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext ,
-const CallExpr *CE,
+const CallEvent ,
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 ,
ProgramStateRef state = C.getState();
- if (CE->getNumArgs() < MinArgCount) {
+ if (Call.getNumArgs() < MinArgCount) {
// The frontend should issue a warning for this case. Just return.
return;
- } else if
[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)
@@ -1179,6 +1195,113 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent ,
C.addTransition(StateFailed);
}
+ProgramStateRef StreamChecker::ensureGetdelimBufferAndSizeCorrect(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext , 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); 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)
@@ -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);
balazske wrote:
These calls can be replaced with `E.assumeBinOpNN` and `E.bindReturnValue`.
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());
+ }
balazske wrote:
Braces are not needed a this place.
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);
+}
balazske wrote:
I do not see a test that checks for the relation between return value and the
"size" value.
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 ,
C.addTransition(StateFailed);
}
+ProgramStateRef StreamChecker::ensureGetdelimBufferAndSizeCorrect(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext , ProgramStateRef State) const {
+ static constexpr char SizeGreaterThanBufferSize[] =
balazske wrote:
The `static` keyword is not needed here?
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); balazske wrote: Probably this should not have type `auto` (it is used only if the real type is visible or the type name is very long). 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 ,
BugReporter ) const;
- void checkPreStmt(const CallExpr *CE, CheckerContext ) const;
+ void checkPreCall(const CallEvent , CheckerContext ) const;
- void CheckOpen(CheckerContext , const CallExpr *CE) const;
- void CheckOpenAt(CheckerContext , const CallExpr *CE) const;
- void CheckPthreadOnce(CheckerContext , const CallExpr *CE) const;
+ void CheckOpen(CheckerContext , const CallEvent ) const;
+ void CheckOpenAt(CheckerContext , const CallEvent ) const;
+ void CheckPthreadOnce(CheckerContext , const CallEvent ) const;
- void CheckOpenVariant(CheckerContext ,
-const CallExpr *CE, OpenVariant Variant) const;
+ void CheckOpenVariant(CheckerContext , const CallEvent ,
+OpenVariant Variant) const;
void ReportOpenBug(CheckerContext , 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 ,
CheckerContext ) 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 ,
ProgramStateRef State,
@@ -152,17 +152,17 @@ void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext
,
}
void UnixAPIMisuseChecker::CheckOpen(CheckerContext ,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::Open);
+ const CallEvent ) const {
+ CheckOpenVariant(C, Call, OpenVariant::Open);
}
void UnixAPIMisuseChecker::CheckOpenAt(CheckerContext ,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::OpenAt);
+ const CallEvent ) const {
+ CheckOpenVariant(C, Call, OpenVariant::OpenAt);
}
void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext ,
-const CallExpr *CE,
+const CallEvent ,
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 ,
ProgramStateRef state = C.getState();
- if (CE->getNumArgs() < MinArgCount) {
+ if (Call.getNumArgs() < MinArgCount) {
// The frontend should issue a warning for this case. Just return.
return;
- } else if
[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)
@@ -1179,6 +1195,113 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent ,
C.addTransition(StateFailed);
}
+ProgramStateRef StreamChecker::ensureGetdelimBufferAndSizeCorrect(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext , 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, , SizePtrExpr,
+LinePtrPtrExpr](const ProgramStateRef ,
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 ,
C.addTransition(StateFailed);
}
+ProgramStateRef StreamChecker::ensureGetdelimBufferAndSizeCorrect(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext , 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)
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 ,
BugReporter ) const;
- void checkPreStmt(const CallExpr *CE, CheckerContext ) const;
+ void checkPreCall(const CallEvent , CheckerContext ) const;
- void CheckOpen(CheckerContext , const CallExpr *CE) const;
- void CheckOpenAt(CheckerContext , const CallExpr *CE) const;
- void CheckPthreadOnce(CheckerContext , const CallExpr *CE) const;
+ void CheckOpen(CheckerContext , const CallEvent ) const;
+ void CheckOpenAt(CheckerContext , const CallEvent ) const;
+ void CheckPthreadOnce(CheckerContext , const CallEvent ) const;
- void CheckOpenVariant(CheckerContext ,
-const CallExpr *CE, OpenVariant Variant) const;
+ void CheckOpenVariant(CheckerContext , const CallEvent ,
+OpenVariant Variant) const;
void ReportOpenBug(CheckerContext , 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 ,
CheckerContext ) 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 ,
ProgramStateRef State,
@@ -152,17 +152,17 @@ void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext
,
}
void UnixAPIMisuseChecker::CheckOpen(CheckerContext ,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::Open);
+ const CallEvent ) const {
+ CheckOpenVariant(C, Call, OpenVariant::Open);
}
void UnixAPIMisuseChecker::CheckOpenAt(CheckerContext ,
- const CallExpr *CE) const {
- CheckOpenVariant(C, CE, OpenVariant::OpenAt);
+ const CallEvent ) const {
+ CheckOpenVariant(C, Call, OpenVariant::OpenAt);
}
void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext ,
-const CallExpr *CE,
+const CallEvent ,
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 ,
ProgramStateRef state = C.getState();
- if (CE->getNumArgs() < MinArgCount) {
+ if (Call.getNumArgs() < MinArgCount) {
// The frontend should issue a warning for this case. Just return.
return;
- } else if
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
balazske wrote: > @balazske Are you interested in refactoring the logic of > `StdLibraryFunctionsChecker` into an API that can be used by separate > checkers? I could try it. It would solve at least the (dependency) difficulties related to this checker. Probably the checker can remain and contain the functions that are evaluated with `evalCall`, but no others (the others can be moved to a relevant checker, or `UnixAPIChecker`). But I have still concerns if this is the best solution or the current is good too. The current checkers often do not check all aspects of a function, for example `MallocChecker` and `UnixAPIChecker` checks `malloc` like calls, and a checker becomes always more difficult if new things are added to it (all pre- and postconditions of checked functions) that are somewhat irrelevant to the scope of the checker. The question requires more discussion. 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 =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID: In-Reply-To: https://github.com/NagyDonat edited 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 =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -1179,6 +1195,113 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent ,
C.addTransition(StateFailed);
}
+ProgramStateRef StreamChecker::ensureGetdelimBufferAndSizeCorrect(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext , 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, , SizePtrExpr,
+LinePtrPtrExpr](const ProgramStateRef ,
+const char *ErrMsg) {
+if (ExplodedNode *N = C.generateErrorNode(BugState)) {
+ auto R =
+ std::make_unique(BT_IllegalSize, ErrMsg, N);
+ bugreporter::trackExpressionValue(N, SizePtrExpr, *R);
+ bugreporter::trackExpressionValue(N, LinePtrPtrExpr, *R);
+ C.emitReport(std::move(R));
+}
+ };
+
+ // We have a pointer to a pointer to the buffer, and a pointer to the size.
+ // We want what they point at.
+ auto LinePtrSVal = getPointeeVal(LinePtrPtrSVal,
State)->getAs();
+ auto NSVal = getPointeeVal(SizePtrSVal, State);
+ if (!LinePtrSVal || !NSVal || NSVal->isUnknown())
+return nullptr;
+
+ assert(LinePtrPtrExpr && SizePtrExpr);
+
+ const auto [LinePtrNotNull, LinePtrNull] = State->assume(*LinePtrSVal);
+ if (LinePtrNotNull && !LinePtrNull) {
+// If `*lineptr` is not null, but `*n` is undefined, there is UB.
+if (NSVal->isUndef()) {
+ EmitBugReport(LinePtrNotNull, SizeUndef);
+ return nullptr;
+}
+
+// If it is defined, and known, its size must be less than or equal to
+// the buffer size.
+auto NDefSVal = NSVal->getAs();
+auto = C.getSValBuilder();
+auto LineBufSize =
+getDynamicExtent(LinePtrNotNull, LinePtrSVal->getAsRegion(), SVB);
+auto LineBufSizeGtN = SVB.evalBinOp(LinePtrNotNull, BO_GE, LineBufSize,
+*NDefSVal, SVB.getConditionType())
+ .getAs();
+if (!LineBufSizeGtN) {
+ return LinePtrNotNull;
+}
+if (auto LineBufSizeOk = LinePtrNotNull->assume(*LineBufSizeGtN, true)) {
+ return LineBufSizeOk;
+}
NagyDonat wrote:
```suggestion
if (!LineBufSizeGtN)
return LinePtrNotNull;
if (auto LineBufSizeOk = LinePtrNotNull->assume(*LineBufSizeGtN, true))
return LineBufSizeOk;
```
Unbrace simple conditionals.
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 =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID: In-Reply-To: https://github.com/NagyDonat edited 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 =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -1179,6 +1195,113 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent ,
C.addTransition(StateFailed);
}
+ProgramStateRef StreamChecker::ensureGetdelimBufferAndSizeCorrect(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext , 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.";
NagyDonat wrote:
For `constexpr` strings consider using the type `llvm::StringLiteral` which is
a subclass of `StringRef` and computes the length in compile time.
(And don't confuse it with `clang::StringLiteral` which is an AST node :wink: .)
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 =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -1179,6 +1195,113 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent ,
C.addTransition(StateFailed);
}
+ProgramStateRef StreamChecker::ensureGetdelimBufferAndSizeCorrect(
+SVal LinePtrPtrSVal, SVal SizePtrSVal, const Expr *LinePtrPtrExpr,
+const Expr *SizePtrExpr, CheckerContext , 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, , SizePtrExpr,
+LinePtrPtrExpr](const ProgramStateRef ,
NagyDonat wrote:
```suggestion
LinePtrPtrExpr](ProgramStateRef BugState,
```
`ProgramStateRef` is almost always passed by value in existing code, so
consider using that style. (By the way, `ProgramStateRef` a typedef for
`IntrusiveRefCntPtr`; so I think it's premature
optimization to pass it by const reference.)
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 =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID: In-Reply-To: https://github.com/NagyDonat commented: And now that I'm here I also give a quick review for the commit itself: I think that it's good overall (but I didn't analyze its logic too deeply). I have a few minor suggestions, but they're not mandatory/blocking. 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 =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID: In-Reply-To: NagyDonat wrote: Just chiming in to cast a supporting vote for the idea that > Probably it would be better if StdLibraryFunctionsChecker would be an API > (instead of checker) and in this way any checker can use it for the specific > functions. But with the current solution the checks in > StdLibraryFunctionsChecker can be changed for specific needs. (For example if > a buffer size check is needed in a checker like StreamChecker it could use a > simple API to do this. With the current implementation it can not use an API, > but we can add the check into StdLibraryFunctionsChecker instead.) The checks > like sufficient buffer size or NULL pointer arguments are common to many > checkers and implementing these separately is code repetition and makes > checker code more difficult. @balazske Are you interested in refactoring the logic of `StdLibraryFunctionsChecker` into an API that can be used by separate checkers? 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)
balazske wrote: > 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. I like more if the new checks are moved to `UnixAPIChecker`, or into `StdLibraryFunctionsChecker`. The mentioned FIXME comment is about that these functions should be moved into the `ModelPOSIX` part in `StdLibraryFunctionsChecker`. Probably it would be better if `StdLibraryFunctionsChecker` would be an API (instead of checker) and in this way any checker can use it for the specific functions. But with the current solution the checks in `StdLibraryFunctionsChecker` can be changed for specific needs. (For example if a buffer size check is needed in a checker like `StreamChecker` it could use a simple API to do this. With the current implementation it can not use an API, but we can add the check into `StdLibraryFunctionsChecker` instead.) The checks like sufficient buffer size or NULL pointer arguments are common to many checkers and implementing these separately is code repetition and makes checker code more difficult. 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: @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)
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] 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)
@@ -1158,6 +1173,118 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent ,
C.addTransition(StateFailed);
}
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+CheckerContext , 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)
@@ -1158,6 +1173,118 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent ,
C.addTransition(StateFailed);
}
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+CheckerContext , 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 , 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 ,
+CheckerContext ) 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 ,
C.addTransition(StateFailed);
}
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+CheckerContext , 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 , 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)
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 , 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 ,
+CheckerContext ) 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)
@@ -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)
@@ -1158,6 +1173,118 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent ,
C.addTransition(StateFailed);
}
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+CheckerContext , 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 , 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)
Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID: In-Reply-To: steakhal wrote: > I want to avoid that some functions have null pointer checks in > `StreamChecker`, some not. If this change is merged then it would be good to > add null pointer checks to other functions like `fread` and `fwrite`. (Until > now only the NULL stream pointer was checked.) I can really think of having these checks at both checkers, so that both parties are sort of happy - except for of course having this precondition checked at both places. But to me, the question is really as follow: is it better with this, or not? I'm also open for suggestions for resolving this conflict, so let me know if you have ideas. 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)
balazske wrote: I want to avoid that some functions have null pointer checks in `StreamChecker`, some not. If this change is merged then it would be good to add null pointer checks to other functions like `fread` and `fwrite`. (Until now only the NULL stream pointer was checked.) 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 =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID: In-Reply-To: steakhal wrote: > This functionality could be added to this checker, but to > `StdLibraryFunctionsChecker` too, and probably will be added at a time > (summary of `getdelim` is not accurate now in that checker). The same bug > condition is checked by two different checkers in that case. Otherwise I > prefer to add such checks (for NULL arguments) to > `StdLibraryFunctionsChecker`. Thanks for the review! Awesome recommendations. One note about why we would prefer to implement null checks in this checker is because we don't have `StdLibraryFunctionsChecker` enabled downstream and it would either force us to enable it (which we would only do after a really careful evaluation given the scope of that checker), or keep this patch downstream, which is also fine but we think it wouldn't be so wildly out of space here. 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 =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -1158,6 +1173,118 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent ,
C.addTransition(StateFailed);
}
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+CheckerContext , 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 , 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.");
steakhal wrote:
I agree. We could just do the assertion without any string attached. It's clear
what they express anyways.
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 ,
C.addTransition(StateFailed);
}
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+CheckerContext , 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 , 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.");
balazske wrote:
These assert messages look too long, really it checks only if the argument is
there, not that if it points to a correct value.
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)
@@ -1158,6 +1173,118 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent ,
C.addTransition(StateFailed);
}
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+CheckerContext , 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 , 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 ,
+CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+
+ auto AddTransitionOnReturn = llvm::make_scope_exit([&] {
balazske wrote:
This looks not needed because the transition is added only at the end of the
function, all other return statements are with a null `State` pointer. It is
really shorter to have a single `addTransition` call at the end.
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 ,
C.addTransition(StateFailed);
}
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+CheckerContext , ProgramStateRef State,
+const StringRef PtrDescr) const {
balazske wrote:
This looks like a generally usable function, for example if buffer in `fread`
or `fwrite` is checked for null pointer. For this a special bug report for
"NULL size" does not work, but a generic NULL pointer bug report can work.
Probably this function can be merged with `ensureStreamNonNull` too.
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 ,
C.addTransition(StateFailed);
}
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+CheckerContext , 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 , ProgramStateRef State) const {
+ static constexpr char SizeNotZeroMsg[] =
+ "Line pointer might be null while n value is not zero";
balazske wrote:
Instead of "Line pointer" and "n" using "argument X" (X=index of argument) may
be better, if the user does not see the function declaration.
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/balazske commented: This functionality could be added to this checker, but to `StdLibraryFunctionsChecker` too, and probably will be added at a time (summary of `getdelim` is not accurate now in that checker). The same bug condition is checked by two different checkers in that case. Otherwise I prefer to add such checks (for NULL arguments) to `StdLibraryFunctionsChecker`. 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/balazske edited 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 =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID: In-Reply-To: https://github.com/steakhal edited 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 =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?=, Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?=, Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?=, Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?=, Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?=, Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?= Message-ID: In-Reply-To: https://github.com/steakhal approved this pull request. I'm good with this change. 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)
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)
balazske 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). 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)
balazske wrote: `StreamChecker` still does not check for all possible NULL pointer errors. At `fread` and `fwrite` for example there is no check for NULL buffer pointer. The reason is that these are checked in `StdLibraryFunctionsChecker`. Probably it would be better to add the checks for preconditions to that checker instead. Buffer size conditions for `fread` and `fwrite` are already checked only in that checker. (Or add all other checks to `StreamChecker` too.) (Goal of `StreamChecker` was more to check the stream state related things. Probably even the buffer invalidations could be moved into the other checker, and there are more functions where the invalidation code can be used.) 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/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 , 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 ,
+CheckerContext ) 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/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 , 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 ,
+CheckerContext ) 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)
@@ -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)
@@ -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)
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 , 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 ,
+CheckerContext ) 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/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 , 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 ,
+CheckerContext ) 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 =?utf-8?q?Álvarez_Ayllón?Message-ID: In-Reply-To: https://github.com/steakhal edited 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 =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -1158,6 +1173,123 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent ,
C.addTransition(StateFailed);
}
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+CheckerContext , 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);
steakhal wrote:
```suggestion
auto R = std::make_unique(BT_SizeNull, (PtrDescr
+ " pointer might be NULL.").str(), N);
```
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 =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -1158,6 +1173,123 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent ,
C.addTransition(StateFailed);
}
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+CheckerContext , 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 , 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 ,
+CheckerContext ) 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;
+
+ // The parameter `n` must not be NULL.
+ SVal SizePtrSval = Call.getArgSVal(1);
+ State = ensurePtrNotNull(SizePtrSval, Call.getArgExpr(1), C, State, "Size");
+ if (!State)
+return;
+
+ // The parameter `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 {
+C.addTransition(State);
+ }
steakhal wrote:
```suggestion
}
```
You add this transition anyways via the scope_exit.
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 =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -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());
+ }
steakhal wrote:
Did you test that reading from the line buffer after a `getdelim` fails, would
trigger a "garbage read" sink?
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 =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -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);
steakhal wrote:
```suggestion
StateNotFailed = StateNotFailed->assume(
E.SVB
.evalBinOp(StateNotFailed, BO_GT, *NVal, RetVal,
E.SVB.getConditionType())
.castAs(),
true);
```
I'm worried a bit that we already constrained retVal: `0 <= retVal`, with the
previous assumption.
And here `NVal` comes from user code, which we use as an upperbound: `retVal <
NVal`, which we assume `true`.
This makes me wonder if we could make them contradict and return us a null
state; which we then unconditionally dereference.
Could you have a test for like this?
```
size_t n = -1;
getline(, , F1); // no-crash
```
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 =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?= Message-ID: In-Reply-To: https://github.com/steakhal requested changes to this pull request. 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 =?utf-8?q?Álvarez_Ayllón?Message-ID: In-Reply-To: https://github.com/steakhal edited 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: 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/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 , 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 ,
+CheckerContext ) 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)
@@ -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)
@@ -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)
@@ -1158,6 +1173,123 @@ void StreamChecker::evalUngetc(const FnDescription
*Desc, const CallEvent ,
C.addTransition(StateFailed);
}
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+CheckerContext , 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 , 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 ,
+CheckerContext ) 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 , 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 ,
+CheckerContext ) 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)
@@ -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(); steakhal wrote: Why do you have these `extern` while not the rest? 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)
@@ -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" + steakhal wrote: ```suggestion ``` 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 ,
C.addTransition(StateFailed);
}
+ProgramStateRef
+StreamChecker::ensurePtrNotNull(SVal PtrVal, const Expr *PtrExpr,
+CheckerContext , 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 , 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 ,
+CheckerContext ) 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
steakhal wrote:
We use capitalized and punctuated comments in LLVM.
Check the rest of the new comments as well.
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: 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)
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 , 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 ,
+CheckerContext ) 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)
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 , 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 ,
+CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+
+ auto AddTransitionOnReturn = llvm::make_scope_exit([&] {
+if (State != nullptr) {
+
[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 , 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 ,
+CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+
+ auto AddTransitionOnReturn = llvm::make_scope_exit([&] {
+if (State != nullptr) {
+
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff d0b1fec9e1510d01dad2c9c429573eaa75f0963c 5e62483adf6620d481bdb873dd91f81b8a95f6fc -- clang/test/Analysis/getline-stream.c clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp clang/test/Analysis/Inputs/system-header-simulator-for-malloc.h `` View the diff from clang-format here. ``diff diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index b4b828784c..f83d4e436d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -1209,7 +1209,7 @@ void StreamChecker::preGetdelim(const FnDescription *Desc, // If lineptr points to a NULL pointer, *n must be 0 State = ensureSizeZeroIfLineNull(LinePtrPtrSVal, SizePtrSval, Call.getArgExpr(0), -Call.getArgExpr(1), C, State); + Call.getArgExpr(1), C, State); if (!State) return; `` 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)
llvmbot wrote:
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Alejandro Álvarez Ayllón (alejandro-alvarez-sonarsource)
Changes
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/83027.diff
5 Files Affected:
- (modified)
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h (+6)
- (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+153-2)
- (modified) clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp (+9)
- (modified) clang/test/Analysis/Inputs/system-header-simulator-for-malloc.h
(+1)
- (added) clang/test/Analysis/getline-stream.c (+327)
``diff
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 , 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
