This fixes a failing qltest and makes the exclusion similar to what's in
`PointerOverflow.ql`. It's possible we should exclude based on both `+`
and `<`, but we can revisit that if false positives show up.
Before this change, evaluating `cpp/constant-comparison` followed by
`cpp/signed-overflow-check` would result in re-evaluation of almost all
the cached stages they share: CFG, basic blocks, SSA, and range
analysis. The same effect could be seen on `cpp/bad-strncpy-size`, which
also uses the GVN library.
We were privately importing `semmle.code.<lang>.ir.internal.Overlap`, but `PrintSSA.qll` was depending on it being public. This is made a little more complicated by the presence of cross-langage pyrameterized modules.
The `SignedOverflowCheck.ql` query was very slow on certain snapshots
(jluttine/suitesparse and Chromium) due to bad magic in
`MacroInvocation::getAnAffectedElement_dispred#fb`. This commit doesn't
fix the bad magic but changes the exclusion mechanism to use a predicate
where we can better control the magic and optimization.
The query should also give more good results due to this new exclusion
mechanism, which is the same one used in its sibling,
`PointerOverflow.ql`.
This might cause fewer variables to be analysed because not every use of
`LocalScopeVariable` was constrained by the def-use library. Hopefully
this leads to an improved nullness analysis since it avoids treating
`static T *x = nullptr;` the same as `static T *x; x = nullptr;`.
These changes should not affect semantics since these uses of
`LocalScopeVariable` were already constrained to stack variables by
their use of SSA or def-use.
In these files it was possible to remove calls to `isStatic` by
switching from `LocalScopeVariable` to `StackVariable`. This changes
semantics, hopefully for the better, to treat `thread_local` locals the
same as `static` locals.
This is now a copy of `LocalScopeVariableReachability.qll`, just with
`s/LocalScopeVariable/StackVariable/g`. It can be used as a drop-in
replacement since the `LocalScopeVariableReachability.qll` library
implementation was already restricted to `SemanticStackVariable`.
1. The two last examples were misleading at best. The first of those two
recommended casting to non-negative `int`s to `unsigned int` and then
checking if their addition would overflow, but overflow was
impossible because their sum (on 32-bit two's complement) could be at
most 2^32 - 2. The second example could lead to the wrong condition
(unsigned overflow) being checked if taken literally. Instead of
keeping that example, I reworeded the first paragraph of the
Recommendation section.
2. The assumptions about `delta` being positive was relaxed to
non-negative.
3. There was no need to assume that an unsigned short was non-negative.
4. Some of the suggestions were missing `i >`.
