[clang][analyzer] use unqualified canonical type during merging equivalence class #95729
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…alence class Fixes: llvm#95658 Unqualified canonical type should be used instead of normal QualType for type equality comparison
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Congcong Cai (HerrCai0907) ChangesFixes: #95658 Full diff: https://github.com/llvm/llvm-project/pull/95729.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
index c6f87b45ab887..d8c257dbd731e 100644
--- a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -2333,7 +2333,8 @@ inline ProgramStateRef EquivalenceClass::merge(RangeSet::Factory &F,
//
// The moment we introduce symbolic casts, this restriction can be
// lifted.
- if (getType() != Other.getType())
+ if (getType()->getCanonicalTypeUnqualified() !=
+ Other.getType()->getCanonicalTypeUnqualified())
return State;
SymbolSet Members = getClassMembers(State);
diff --git a/clang/test/Analysis/errno-stdlibraryfunctions.c b/clang/test/Analysis/errno-stdlibraryfunctions.c
index a28efb764edfd..657aa37a42670 100644
--- a/clang/test/Analysis/errno-stdlibraryfunctions.c
+++ b/clang/test/Analysis/errno-stdlibraryfunctions.c
@@ -75,6 +75,30 @@ void errno_mkdtemp(char *template) {
}
}
+typedef char* CHAR_PTR;
+void errno_mkdtemp2(CHAR_PTR template) {
+ CHAR_PTR Dir = mkdtemp(template);
+ if (Dir == NULL) {
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+ if (errno) {} // no warning
+ } else {
+ clang_analyzer_eval(Dir == template); // expected-warning{{TRUE}}
+ if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+ }
+}
+
+typedef char const* CONST_CHAR_PTR;
+void errno_mkdtemp3(CHAR_PTR template) {
+ CONST_CHAR_PTR Dir = mkdtemp(template);
+ if (Dir == NULL) {
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+ if (errno) {} // no warning
+ } else {
+ clang_analyzer_eval(Dir == template); // expected-warning{{TRUE}}
+ if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+ }
+}
+
void errno_getcwd(char *Buf, size_t Sz) {
char *Path = getcwd(Buf, Sz);
if (Sz == 0) {
|
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Congcong Cai (HerrCai0907) ChangesFixes: #95658 Full diff: https://github.com/llvm/llvm-project/pull/95729.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
index c6f87b45ab887..d8c257dbd731e 100644
--- a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -2333,7 +2333,8 @@ inline ProgramStateRef EquivalenceClass::merge(RangeSet::Factory &F,
//
// The moment we introduce symbolic casts, this restriction can be
// lifted.
- if (getType() != Other.getType())
+ if (getType()->getCanonicalTypeUnqualified() !=
+ Other.getType()->getCanonicalTypeUnqualified())
return State;
SymbolSet Members = getClassMembers(State);
diff --git a/clang/test/Analysis/errno-stdlibraryfunctions.c b/clang/test/Analysis/errno-stdlibraryfunctions.c
index a28efb764edfd..657aa37a42670 100644
--- a/clang/test/Analysis/errno-stdlibraryfunctions.c
+++ b/clang/test/Analysis/errno-stdlibraryfunctions.c
@@ -75,6 +75,30 @@ void errno_mkdtemp(char *template) {
}
}
+typedef char* CHAR_PTR;
+void errno_mkdtemp2(CHAR_PTR template) {
+ CHAR_PTR Dir = mkdtemp(template);
+ if (Dir == NULL) {
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+ if (errno) {} // no warning
+ } else {
+ clang_analyzer_eval(Dir == template); // expected-warning{{TRUE}}
+ if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+ }
+}
+
+typedef char const* CONST_CHAR_PTR;
+void errno_mkdtemp3(CHAR_PTR template) {
+ CONST_CHAR_PTR Dir = mkdtemp(template);
+ if (Dir == NULL) {
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+ if (errno) {} // no warning
+ } else {
+ clang_analyzer_eval(Dir == template); // expected-warning{{TRUE}}
+ if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+ }
+}
+
void errno_getcwd(char *Buf, size_t Sz) {
char *Path = getcwd(Buf, Sz);
if (Sz == 0) {
|
steakhal
reviewed
Jun 17, 2024
There was a problem hiding this comment.
Awesome. I have one remark though.
It would be nice to see tests that does not depend on the errno, as this eqclass merging is a major vore feature, unlike the errno modeling which is an optional feature. If have something in mind, you could add that test in addition you already propose.
Again, cool stuff!
NagyDonat
approved these changes
Jun 17, 2024
steakhal
approved these changes
Jun 17, 2024
Yes. |
HerrCai0907
deleted the
95658-static-analyzer-false-positive-in-unixerrno
branch
Should I fix the other type comparison in static analysis? It looks like not consider this case at lots of position. |
I think you are right. And in most cases within CSA, we should usually compare canonical types. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #95658
Unqualified canonical type should be used instead of normal QualType for type equality comparison