https://github.com/balazske updated
https://github.com/llvm/llvm-project/pull/147766
From f8dc303029c68762cdbd19b217730192d26f6fca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com>
Date: Wed, 9 Jul 2025 16:55:07 +0200
Subject: [PATCH 1/3] [clang][analyzer] Add C standard streams to the internal
memory space
If variables `stdin`, `stdout`, `stderr` are added to the system memory
space, they are invalidated at any call to system (or C library) functions.
These variables are not expected to be changed by system functions,
therefore should not belong to the system memory space.
This change moves these to the "internal memory space" which is not
invalidated at system calls (but is at non-system calls). This
eliminates some false positives coming from StreamChecker.
---
clang/lib/StaticAnalyzer/Core/MemRegion.cpp | 20 +++++++--
clang/test/Analysis/stream.c | 45 +++++++++++++++++++--
2 files changed, 59 insertions(+), 6 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index 63ad567ab7151..4e946f39c1cef 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1054,10 +1054,24 @@ const VarRegion *MemRegionManager::getVarRegion(const
VarDecl *D,
assert(!Ty.isNull());
if (Ty.isConstQualified()) {
sReg = getGlobalsRegion(MemRegion::GlobalImmutableSpaceRegionKind);
- } else if (Ctx.getSourceManager().isInSystemHeader(D->getLocation())) {
- sReg = getGlobalsRegion(MemRegion::GlobalSystemSpaceRegionKind);
} else {
- sReg = getGlobalsRegion(MemRegion::GlobalInternalSpaceRegionKind);
+ StringRef N = D->getNameAsString();
+ QualType FILETy = D->getASTContext().getFILEType();
+ if (!FILETy.isNull())
+ FILETy = FILETy.getCanonicalType();
+ Ty = Ty.getCanonicalType();
+ bool IsStdStreamVar = Ty->isPointerType() &&
+ Ty->getPointeeType() == FILETy &&
+ (N == "stdin" || N == "stdout" || N == "stderr");
+ // Pointer value of C standard streams is usually not modified by system
+ // calls. This means they should not get invalidated at system calls and
+ // can not belong to the system memory space.
+ if (Ctx.getSourceManager().isInSystemHeader(D->getLocation()) &&
+ !IsStdStreamVar) {
+ sReg = getGlobalsRegion(MemRegion::GlobalSystemSpaceRegionKind);
+ } else {
+ sReg = getGlobalsRegion(MemRegion::GlobalInternalSpaceRegionKind);
+ }
}
// Finally handle static locals.
diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index 758b40cca4931..781c023ea248a 100644
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -519,14 +519,53 @@ void reopen_std_stream(void) {
if (!fp) return;
stdout = fp; // Let's make them alias.
- clang_analyzer_eval(fp == oldStdout); // expected-warning {{UNKNOWN}}
- clang_analyzer_eval(fp == stdout); // expected-warning {{TRUE}}
no-FALSE
- clang_analyzer_eval(oldStdout == stdout); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(fp == oldStdout); // expected-warning {{FALSE}}
+ clang_analyzer_eval(fp == stdout); // expected-warning {{TRUE}}
+ clang_analyzer_eval(oldStdout == stdout); // expected-warning {{FALSE}}
}
void only_success_path_does_not_alias_with_stdout(void) {
if (stdout) return;
FILE *f = fopen("/tmp/foof", "r"); // no-crash
+ clang_analyzer_eval(f == 0);// expected-warning {{TRUE}} expected-warning
{{FALSE}}
if (!f) return;
fclose(f);
}
+
+extern void do_something();
+
+void test_no_invalidate_at_system_call(int use_std) {
+ FILE *fd;
+ char *buf;
+
+ if (use_std) {
+ fd = stdin;
+ } else {
+ if ((fd = fopen("x/y/z", "r")) == NULL)
+ return;
+
+ clang_analyzer_eval(fd == stdin); // expected-warning{{FALSE}}
+ buf = (char *)malloc(100);
+ clang_analyzer_eval(fd == stdin); // expected-warning{{FALSE}}
+ }
+
+ if (fd != stdin)
+ fclose(fd);
+}
+
+void test_invalidate_at_non_system_call(int use_std) {
+ FILE *fd;
+
+ if (use_std) {
+ fd = stdin;
+ } else {
+ if ((fd = fopen("x/y/z", "r")) == NULL)
+ return;
+ }
+
+ do_something();
+ clang_analyzer_eval(fd == stdin); // expected-warning{{UNKNOWN}}
+
+ if (fd != stdin)
+ fclose(fd);
+} // expected-warning{{Opened stream never closed. Potential resource leak}}
From 3117a648fe537b11a51fcb1e6b4ba19f49f1c4bc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com>
Date: Thu, 10 Jul 2025 10:04:29 +0200
Subject: [PATCH 2/3] updated code comment
---
clang/lib/StaticAnalyzer/Core/MemRegion.cpp | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index 4e946f39c1cef..53f421640c555 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1063,9 +1063,11 @@ const VarRegion *MemRegionManager::getVarRegion(const
VarDecl *D,
bool IsStdStreamVar = Ty->isPointerType() &&
Ty->getPointeeType() == FILETy &&
(N == "stdin" || N == "stdout" || N == "stderr");
- // Pointer value of C standard streams is usually not modified by system
- // calls. This means they should not get invalidated at system calls and
- // can not belong to the system memory space.
+ // Pointer value of C standard streams is usually not modified by calls
+ // to functions declared in system headers. This means that they should
+ // not get invalidated by calls to functions declared in system headers,
+ // so they are placed in the global internal space, which is not
+ // invalidated by calls to functions declared in system headers.
if (Ctx.getSourceManager().isInSystemHeader(D->getLocation()) &&
!IsStdStreamVar) {
sReg = getGlobalsRegion(MemRegion::GlobalSystemSpaceRegionKind);
From 0845fa2168eeea11e00d5ebee1eda733f8a843de Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com>
Date: Fri, 11 Jul 2025 16:52:39 +0200
Subject: [PATCH 3/3] moved test for std stream into function, simplified test
---
clang/lib/StaticAnalyzer/Core/MemRegion.cpp | 27 ++++++++----
clang/test/Analysis/stream.c | 47 +++++++--------------
2 files changed, 33 insertions(+), 41 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index 53f421640c555..9c7b11b7947b1 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1022,6 +1022,23 @@ getStackOrCaptureRegionForDeclContext(const
LocationContext *LC,
return (const StackFrameContext *)nullptr;
}
+static bool isStdStreamVar(const VarDecl *D) {
+ const auto *ND = dyn_cast<NamedDecl>(D);
+ if (!ND)
+ return false;
+ DeclarationName DeclN = ND->getDeclName();
+ if (!DeclN.isIdentifier())
+ return false;
+ StringRef N = DeclN.getAsIdentifierInfo()->getName();
+ QualType FILETy = D->getASTContext().getFILEType();
+ if (FILETy.isNull())
+ return false;
+ FILETy = FILETy.getCanonicalType();
+ QualType Ty = D->getType().getCanonicalType();
+ return Ty->isPointerType() && Ty->getPointeeType() == FILETy &&
+ (N == "stdin" || N == "stdout" || N == "stderr");
+}
+
const VarRegion *MemRegionManager::getVarRegion(const VarDecl *D,
const LocationContext *LC) {
const auto *PVD = dyn_cast<ParmVarDecl>(D);
@@ -1055,21 +1072,13 @@ const VarRegion *MemRegionManager::getVarRegion(const
VarDecl *D,
if (Ty.isConstQualified()) {
sReg = getGlobalsRegion(MemRegion::GlobalImmutableSpaceRegionKind);
} else {
- StringRef N = D->getNameAsString();
- QualType FILETy = D->getASTContext().getFILEType();
- if (!FILETy.isNull())
- FILETy = FILETy.getCanonicalType();
- Ty = Ty.getCanonicalType();
- bool IsStdStreamVar = Ty->isPointerType() &&
- Ty->getPointeeType() == FILETy &&
- (N == "stdin" || N == "stdout" || N == "stderr");
// Pointer value of C standard streams is usually not modified by calls
// to functions declared in system headers. This means that they should
// not get invalidated by calls to functions declared in system headers,
// so they are placed in the global internal space, which is not
// invalidated by calls to functions declared in system headers.
if (Ctx.getSourceManager().isInSystemHeader(D->getLocation()) &&
- !IsStdStreamVar) {
+ !isStdStreamVar(D)) {
sReg = getGlobalsRegion(MemRegion::GlobalSystemSpaceRegionKind);
} else {
sReg = getGlobalsRegion(MemRegion::GlobalInternalSpaceRegionKind);
diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index 781c023ea248a..10e6e5381ee6d 100644
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -534,38 +534,21 @@ void only_success_path_does_not_alias_with_stdout(void) {
extern void do_something();
-void test_no_invalidate_at_system_call(int use_std) {
- FILE *fd;
- char *buf;
-
- if (use_std) {
- fd = stdin;
- } else {
- if ((fd = fopen("x/y/z", "r")) == NULL)
- return;
-
- clang_analyzer_eval(fd == stdin); // expected-warning{{FALSE}}
- buf = (char *)malloc(100);
- clang_analyzer_eval(fd == stdin); // expected-warning{{FALSE}}
- }
-
- if (fd != stdin)
- fclose(fd);
+void test_no_invalidate_at_system_call() {
+ FILE *old_stdin = stdin;
+ if ((stdin = fopen("x/y/z", "r")) == NULL)
+ return;
+ clang_analyzer_eval(old_stdin == stdin); // expected-warning{{FALSE}}
+ free(malloc(100));
+ clang_analyzer_eval(old_stdin == stdin); // expected-warning{{FALSE}}
+ fclose(stdin);
}
-void test_invalidate_at_non_system_call(int use_std) {
- FILE *fd;
-
- if (use_std) {
- fd = stdin;
- } else {
- if ((fd = fopen("x/y/z", "r")) == NULL)
- return;
- }
-
+void test_invalidate_at_non_system_call() {
+ FILE *old_stdin = stdin;
+ if ((stdin = fopen("x/y/z", "r")) == NULL)
+ return;
+ clang_analyzer_eval(old_stdin == stdin); // expected-warning{{FALSE}}
do_something();
- clang_analyzer_eval(fd == stdin); // expected-warning{{UNKNOWN}}
-
- if (fd != stdin)
- fclose(fd);
-} // expected-warning{{Opened stream never closed. Potential resource leak}}
+ clang_analyzer_eval(old_stdin == stdin); // expected-warning{{UNKNOWN}}
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
