donat.nagy marked an inline comment as done.
donat.nagy added a comment.
@steakhal Thanks for the review!
I answered some of your questions; and I'll handle the rest soon.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:150
+ SVB.evalBinOp(State, Comparison, Val, LimitVal, SVB.getConditionType());
+ if (auto DURes = ResultVal.getAs<DefinedOrUnknownSVal>()) {
+ auto [StTrue, StFalse] = State->assume(DURes.value());
----------------
steakhal wrote:
> How about swapping these branches on the `ResultVal.isUndef()` predicate?
The argument of `ProgramState::assume()` must be a `DefinedOrUnknownSVal`, so I
need to cast the return value of `evalBinOp` (which may be Undefined) to this
type.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:157-158
+ }
+ // The code may be valid, so let's assume that it's valid:
+ State = StTrue;
+ if (StFalse) {
----------------
steakhal wrote:
> I wonder if we should add a message here that we introduced a new constraint:
> "X assumed to be non-negative".
> To add such a message, you should do something similar to what is done in the
> `StdLibraryFunctionsChecker`.
> Take the `C.getPredecessor()` and chain all your NoteTags by calling the
> `Pred = C.addTransition(NewState, Pred, Tag)` like this. This way. This is
> how one "sequence of ExplodedNodes" can be made.
This is already done by `BitwiseShiftValidator::createNoteTag()`. It's a bit
complex because the checker merges all its assumptions into a single note tag
(because I didn't want to emit two or three notes directly after each other.
================
Comment at: clang/test/Analysis/analyzer-config.c:36
// CHECK-NEXT: cfg-temporary-dtors = true
+// CHECK-NEXT: consider-single-element-arrays-as-flexible-array-members = true
+// CHECK-NEXT: core.BitwiseShift:Pedantic = false
----------------
steakhal wrote:
> I'm pretty sure I got rid of this one :D
> Consider rebasing on top of `llvm/main`.
Oops, this might be an error introduced during a rebase... Thanks for catching
it!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156312/new/
https://reviews.llvm.org/D156312
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
