steakhal added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1359 + return cache(S, + SVB.evalCast(Op, S->getType(), S->getOperand()->getType())); + } ---------------- Hoist this common subexpression. ================ Comment at: clang/test/Analysis/svalbuilder-casts.cpp:9-10 + +// Test that the SValBuilder is able to look up and use a constraint for an +// operand of a SymbolCast, when the operand is constrained to a const value. + ---------------- Please ass a test case where these events happen in the execution path: 1) Have an unconstrained variable, such as a fn parameter `x`. 2) Produce a Symbolic cast of `x`, let's call it `s`. 3) Constrain `x` to some value range, let's say it must be positive. 4) Verify that the value of the Symbolic cast `s` is also constrained to be positive. ================ Comment at: clang/test/Analysis/svalbuilder-casts.cpp:30 + // the entire `x` is zero. We are not able to know the exact value of x. + // It can be one of 65536 possible values like [0, 65536, 131072, ...] + // and so on. To avoid huge range sets we still assume `x` in the range ---------------- ================ Comment at: clang/test/Analysis/svalbuilder-casts.cpp:34-37 + if (!x) + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + else + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} ---------------- `clang_analyzer_eval(x == 0); // expected-warning {{UNKNOWN}}` ================ Comment at: clang/test/Analysis/svalbuilder-casts.cpp:43 + // If two lower bytes of `x` equal to zero, and we know x to be 65537, + // which is not truncated to short as zero. Thus the branch is infisible. + short s = x; ---------------- ================ Comment at: clang/test/Analysis/svalbuilder-casts.cpp:44-45 + // which is not truncated to short as zero. Thus the branch is infisible. + short s = x; + if (!s) { + if (x == 65537 || x == 131073) ---------------- It took me a while to get that the `short` variable has nothing to do with the test. I would recommend extending the previous example with the //short variable// to demonstrate that the same thing (narrowing cast) can occur by either an explicit cast or some implicit casts like a variable initialization/assignment. ================ Comment at: clang/test/Analysis/svalbuilder-casts.cpp:46-49 + if (x == 65537 || x == 131073) + clang_analyzer_warnIfReached(); // no-warning + else + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} ---------------- ```lang=C++ clang_analyzer_eval(x == 1); // expected-warning {{UNKNOWN}} clang_analyzer_eval(x == -1); // expected-warning {{UNKNOWN}} clang_analyzer_eval(x == 0); // expected-warning {{FALSE}} clang_analyzer_eval(x == 65537); // expected-warning {{FALSE}} clang_analyzer_eval(x == -65537); // expected-warning {{FALSE}} clang_analyzer_eval(x == 131073); // expected-warning {{FALSE}} clang_analyzer_eval(x == -131073); // expected-warning {{FALSE}} ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126481/new/ https://reviews.llvm.org/D126481 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits