Skip to content

Commit

Permalink
Add incorrect-rounding-division.ql
Browse files Browse the repository at this point in the history
  • Loading branch information
Marcono1234 committed Feb 22, 2024
1 parent 102663e commit 940134a
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ private predicate accessSameField(FieldAccess a, FieldAccess b) {
or accessSameVariable(a.getQualifier(), b.getQualifier())
}

// TODO: Already declared in other queries, reduce code duplication
// TODO: Use `accessSameVarOfSameOwner` from VarAccess?
predicate accessSameVariable(VarAccess a, VarAccess b) {
exists (Variable var | var = a.getVariable() |
var = b.getVariable()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/**
* Finds expressions which seem to try rounding down a value `a` to the next smaller
* multiple of `b`, but which seem to be incorrectly implemented.
*
* The correct expression is `a / b * b` (where `a` and `b` are integral numbers).
* The following incorrect expressions are detected:
* - `a / b` producing a floating point number; in that case `a / b * b` is most
* likely the same as `a` (except for some floating point precision loss),
* or possibly brackets are missing and it should have been `a / (b * b)`
* - `a / b * a`; the `* a` seems incorrect and should probably be `* b`
*
* @id todo
* @kind problem
*/

import java
import lib.VarAccess

predicate haveSameValue(Expr a, Expr b) {
a.getType() = b.getType() and a.(Literal).getValue() = b.(Literal).getValue()
or
accessSameVarOfSameOwner(a, b)
}

from DivExpr div, Expr divisor, MulExpr mul, Expr mulOperand
where
// Match `... / divisor * mulOperand`
mul.getLeftOperand() = div and
mulOperand = mul.getRightOperand() and
div.getRightOperand() = divisor and
(
// Redundant division because result is floating point, e.g. `(a / 2.0) * 2` (= `a`, with some floating point precision loss)
div.getType() instanceof FloatingPointType and
haveSameValue(divisor, mulOperand)
or
// TODO: Maybe remove this case (also from documentation of query)? At least Variant Analysis did not find any case of this

// Or accidental `(a / b) * a` when it should probably have been `(a / b) * b`
haveSameValue(div.getLeftOperand(), mulOperand) and
// Ignore if this calculates square of expression, e.g. `a / b * a / b`
not exists(DivExpr rightDiv |
rightDiv.getLeftOperand() = mul and
haveSameValue(rightDiv.getRightOperand(), divisor)
)
)
select mul, "Performs incorrect rounding division"
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ import java
import lib.Literals
import lib.Operations

// TODO: Also cover assign-op, e.g. `x += 0`
// TODO: Also cover bitwise operations, e.g. `| 0`, `| -1`, `& 0` `& -1`
// (don't have to check if literal is `int` or `long`; e.g. `-1` is implicitly converted to `-1L`)

abstract class PointlessOperation extends Expr {
abstract string getDescription();
}
Expand Down

0 comments on commit 940134a

Please sign in to comment.