[PATCH] D42745: [analyzer] Add support for __builtin_constant_p to BuiltinFunctionChecker

[Felix Kostenzer via Phabricator via cfe-commits]

sp4r74n-117 updated this revision to Diff 133447.
sp4r74n-117 added a comment.

Thanks for the explanation, seems like I was on the wrong track.
I now do realize that my initial approach was in fact duplicating the logic of 
'BI__builtin_object_size'.
I've removed most of the changes applied to the checker and extended the 'case' 
instead.
What remains is the regression test tho.


https://reviews.llvm.org/D42745

Files:
  lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
  test/Analysis/builtin-functions.cpp


Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -64,3 +64,20 @@
 // We give up the analysis on this path.
   }
 }
+
+void test_constant_p() {
+  int i = 1;
+  const int j = 2;
+  constexpr int k = 3;
+  clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning 
{{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning 
{{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning 
{{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning 
{{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning 
{{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning 
{{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning 
{{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning 
{{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // 
expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning 
{{FALSE}}
+  clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning 
{{TRUE}}
+}
Index: lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -97,7 +97,8 @@
 return true;
   }
 
-  case Builtin::BI__builtin_object_size: {
+  case Builtin::BI__builtin_object_size:
+  case Builtin::BI__builtin_constant_p: {
 // This must be resolvable at compile time, so we defer to the constant
 // evaluator for a value.
 SVal V = UnknownVal();


Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -64,3 +64,20 @@
 // We give up the analysis on this path.
   }
 }
+
+void test_constant_p() {
+  int i = 1;
+  const int j = 2;
+  constexpr int k = 3;
+  clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
+  clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
+}
Index: lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -97,7 +97,8 @@
 return true;
   }
 
-  case Builtin::BI__builtin_object_size: {
+  case Builtin::BI__builtin_object_size:
+  case Builtin::BI__builtin_constant_p: {
 // This must be resolvable at compile time, so we defer to the constant
 // evaluator for a value.
 SVal V = UnknownVal();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42745: [analyzer] Add support for __builtin_constant_p to BuiltinFunctionChecker

[Felix Kostenzer via Phabricator via cfe-commits]

sp4r74n-117 added a comment.

I do not have commit access so it would be great if you could do it for me. 
Thanks.


https://reviews.llvm.org/D42745



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42745: [analyzer] Add support for __builtin_constant_p to BuiltinFunctionChecker

[Felix Kostenzer via Phabricator via cfe-commits]

sp4r74n-117 created this revision.
sp4r74n-117 added reviewers: george.karpenkov, dcoughlin, dergachev.a.
sp4r74n-117 created this object with visibility "All Users".
Herald added subscribers: cfe-commits, a.sidorin, szepet, xazax.hun.

Added evaluation of __builtin_constant_p to the dedicated StaticAnalyzer 
checker along with a test case.

Tested via: make clang-test


Repository:
  rC Clang

https://reviews.llvm.org/D42745

Files:
  lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
  test/Analysis/builtin-functions.cpp


Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -64,3 +64,18 @@
 // We give up the analysis on this path.
   }
 }
+
+void test_constant_p() {
+  int i = 1;
+  const int j = 2;
+  constexpr int k = 3;
+  clang_analyzer_eval(__builtin_constant_p(42)); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i)); // expected-warning {{FALSE}}
+  clang_analyzer_eval(__builtin_constant_p(j)); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(k)); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42)); // expected-warning 
{{FALSE}}
+  clang_analyzer_eval(__builtin_constant_p(j + 42)); // expected-warning 
{{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(k + 42)); // expected-warning 
{{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(" ")); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p)); // 
expected-warning {{FALSE}}
+}
Index: lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -113,6 +113,24 @@
 C.addTransition(state->BindExpr(CE, LCtx, V));
 return true;
   }
+
+  case Builtin::BI__builtin_constant_p: {
+SVal V;
+SValBuilder& SVB = C.getSValBuilder();
+
+llvm::APSInt Result;
+// Model semantics as 'A return of 0 does not indicate that the value is
+// not a constant, but merely that GCC cannot prove it is a constant [...]'
+if (CE->EvaluateAsInt(Result, C.getASTContext(), Expr::SE_NoSideEffects)) {
+  SVB.getBasicValueFactory().getAPSIntType(CE->getType()).apply(Result);
+  V = SVB.makeIntVal(Result);
+}
+else
+  V = SVB.makeZeroVal(CE->getType());
+
+C.addTransition(state->BindExpr(CE, LCtx, V));
+return true;
+  }
   }
 }
 


Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -64,3 +64,18 @@
 // We give up the analysis on this path.
   }
 }
+
+void test_constant_p() {
+  int i = 1;
+  const int j = 2;
+  constexpr int k = 3;
+  clang_analyzer_eval(__builtin_constant_p(42)); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i)); // expected-warning {{FALSE}}
+  clang_analyzer_eval(__builtin_constant_p(j)); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(k)); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42)); // expected-warning {{FALSE}}
+  clang_analyzer_eval(__builtin_constant_p(j + 42)); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(k + 42)); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(" ")); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p)); // expected-warning {{FALSE}}
+}
Index: lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -113,6 +113,24 @@
 C.addTransition(state->BindExpr(CE, LCtx, V));
 return true;
   }
+
+  case Builtin::BI__builtin_constant_p: {
+SVal V;
+SValBuilder& SVB = C.getSValBuilder();
+
+llvm::APSInt Result;
+// Model semantics as 'A return of 0 does not indicate that the value is
+// not a constant, but merely that GCC cannot prove it is a constant [...]'
+if (CE->EvaluateAsInt(Result, C.getASTContext(), Expr::SE_NoSideEffects)) {
+  SVB.getBasicValueFactory().getAPSIntType(CE->getType()).apply(Result);
+  V = SVB.makeIntVal(Result);
+}
+else
+  V = SVB.makeZeroVal(CE->getType());
+
+C.addTransition(state->BindExpr(CE, LCtx, V));
+return true;
+  }
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42745: [analyzer] Add support for __builtin_constant_p to BuiltinFunctionChecker

[Felix Kostenzer via Phabricator via cfe-commits]

sp4r74n-117 updated this revision to Diff 132738.
sp4r74n-117 added a comment.

Thanks for the feedback.
I've added two additional tests to 'test_constant_p' for the case where an 
expression evaluating to a constant 0 is passed to the builtin.
The existing tests have been refactored to check explicitly for 0 or 1 instead 
of relying on the implicit boolean conversion.


https://reviews.llvm.org/D42745

Files:
  lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
  test/Analysis/builtin-functions.cpp


Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -64,3 +64,20 @@
 // We give up the analysis on this path.
   }
 }
+
+void test_constant_p() {
+  int i = 1;
+  const int j = 2;
+  constexpr int k = 3;
+  clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning 
{{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning 
{{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning 
{{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning 
{{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning 
{{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning 
{{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning 
{{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning 
{{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // 
expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning 
{{FALSE}}
+  clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning 
{{TRUE}}
+}
Index: lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -113,6 +113,24 @@
 C.addTransition(state->BindExpr(CE, LCtx, V));
 return true;
   }
+
+  case Builtin::BI__builtin_constant_p: {
+SVal V;
+SValBuilder& SVB = C.getSValBuilder();
+
+llvm::APSInt Result;
+// Model semantics as 'A return of 0 does not indicate that the value is
+// not a constant, but merely that GCC cannot prove it is a constant [...]'
+if (CE->EvaluateAsInt(Result, C.getASTContext(), Expr::SE_NoSideEffects)) {
+  SVB.getBasicValueFactory().getAPSIntType(CE->getType()).apply(Result);
+  V = SVB.makeIntVal(Result);
+}
+else
+  V = SVB.makeZeroVal(CE->getType());
+
+C.addTransition(state->BindExpr(CE, LCtx, V));
+return true;
+  }
   }
 }
 


Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -64,3 +64,20 @@
 // We give up the analysis on this path.
   }
 }
+
+void test_constant_p() {
+  int i = 1;
+  const int j = 2;
+  constexpr int k = 3;
+  clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
+  clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
+}
Index: lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -113,6 +113,24 @@
 C.addTransition(state->BindExpr(CE, LCtx, V));
 return true;
   }
+
+  case Builtin::BI__builtin_constant_p: {
+SVal V;
+SValBuilder& SVB = C.getSValBuilder();
+
+llvm::APSInt Result;
+// Model semantics as 'A return of 0 does not indicate that the value is
+// not a constant, but merely that GCC cannot prove it is a constant [...]'
+if (CE->EvaluateAsInt(Result, C.getASTContext(), Expr::SE_NoSideEffects)) {
+  SVB.getBasicValueFactory().getAPSIntType(CE->getType()).apply(Result);
+  V = SVB.makeIntVal(Resul