NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs,
mikhail.ramalho, Szelethus, baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, jdoerfert, dkrupp, donat.nagy, kristina,
a.sidorin, szepet.
Herald added a project: clang.
`__builtin_constant_p(x)` is a compiler builtin that evaluates to `1` when its
argument `x` is a compile-time constant and to `0` otherwise. In CodeGen it is
simply lowered to the respective LLVM intrinsic
<https://llvm.org/docs/LangRef.html#llvm-is-constant-intrinsic>. In the
Analyzer we've been trying to delegate modeling to `Expr::EvaluateAsInt`, which
can sometimes fail simply because the expression is too complicated.
When it fails, let's conservatively return `false`. The behavior of this
builtin is so unpredictable (in fact, it even depends on optimization level)
that any code that's using the builtin should expect anything except maybe a
concrete integer to be described by it as "not a constant". Therefore,
returning "false" when we're unsure is unlikely to trigger weird execution
paths in the code, while returning "unknown (potentially true)" may do so.
Which is something i've just seen on some real-world code within a weird macro
expansion that tries to emulate compile-time assertions in C by declaring
arrays of size -1 when it fails, which triggers the VLA checker to warn if "-1"
wasn't in fact a constant. Something like this:
if (__builtin_constant_p((p == 0))) { // EvaluateAsInt returns Unknown here.
char __compile_time_assert__[((p == 0)) ? 1 : -1]; // Warning: array of
size -1.
(void)__compile_time_assert__;
}
Repository:
rC Clang
https://reviews.llvm.org/D60110
Files:
clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
clang/test/Analysis/builtin-functions.cpp
Index: clang/test/Analysis/builtin-functions.cpp
===================================================================
--- clang/test/Analysis/builtin-functions.cpp
+++ clang/test/Analysis/builtin-functions.cpp
@@ -65,19 +65,20 @@
}
}
-void test_constant_p() {
+void test_constant_p(void *ptr) {
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
{{UNKNOWN}}
+ 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
{{UNKNOWN}}
+ 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 {{UNKNOWN}}
+ 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}}
+ clang_analyzer_eval(__builtin_constant_p(ptr == 0)); // expected-warning
{{FALSE}}
}
Index: clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -100,17 +100,25 @@
case Builtin::BI__builtin_constant_p: {
// This must be resolvable at compile time, so we defer to the constant
// evaluator for a value.
+ SValBuilder &SVB = C.getSValBuilder();
SVal V = UnknownVal();
Expr::EvalResult EVResult;
if (CE->EvaluateAsInt(EVResult, C.getASTContext(),
Expr::SE_NoSideEffects)) {
// Make sure the result has the correct type.
llvm::APSInt Result = EVResult.Val.getInt();
- SValBuilder &SVB = C.getSValBuilder();
BasicValueFactory &BVF = SVB.getBasicValueFactory();
BVF.getAPSIntType(CE->getType()).apply(Result);
V = SVB.makeIntVal(Result);
}
+ if (FD->getBuiltinID() == Builtin::BI__builtin_constant_p) {
+ // If we didn't manage to figure out if the value is constant or not,
+ // it is safe to assume that it's not constant and unsafe to assume
+ // that it's constant.
+ if (V.isUnknown())
+ V = SVB.makeIntVal(0, CE->getType());
+ }
+
C.addTransition(state->BindExpr(CE, LCtx, V));
return true;
}
Index: clang/test/Analysis/builtin-functions.cpp
===================================================================
--- clang/test/Analysis/builtin-functions.cpp
+++ clang/test/Analysis/builtin-functions.cpp
@@ -65,19 +65,20 @@
}
}
-void test_constant_p() {
+void test_constant_p(void *ptr) {
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 {{UNKNOWN}}
+ 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 {{UNKNOWN}}
+ 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 {{UNKNOWN}}
+ 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}}
+ clang_analyzer_eval(__builtin_constant_p(ptr == 0)); // expected-warning {{FALSE}}
}
Index: clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -100,17 +100,25 @@
case Builtin::BI__builtin_constant_p: {
// This must be resolvable at compile time, so we defer to the constant
// evaluator for a value.
+ SValBuilder &SVB = C.getSValBuilder();
SVal V = UnknownVal();
Expr::EvalResult EVResult;
if (CE->EvaluateAsInt(EVResult, C.getASTContext(), Expr::SE_NoSideEffects)) {
// Make sure the result has the correct type.
llvm::APSInt Result = EVResult.Val.getInt();
- SValBuilder &SVB = C.getSValBuilder();
BasicValueFactory &BVF = SVB.getBasicValueFactory();
BVF.getAPSIntType(CE->getType()).apply(Result);
V = SVB.makeIntVal(Result);
}
+ if (FD->getBuiltinID() == Builtin::BI__builtin_constant_p) {
+ // If we didn't manage to figure out if the value is constant or not,
+ // it is safe to assume that it's not constant and unsafe to assume
+ // that it's constant.
+ if (V.isUnknown())
+ V = SVB.makeIntVal(0, CE->getType());
+ }
+
C.addTransition(state->BindExpr(CE, LCtx, V));
return true;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
