|
| 1 | +/** |
| 2 | + * Finds loop statements such as `while (...) { ... }` which seem to loop infinitely |
| 3 | + * because the loop condition depends on variables which are not updated in the loop. |
| 4 | + * |
| 5 | + * This also covers cases where the loop condition depends on a non-`volatile` field |
| 6 | + * which might therefore indicate thread- and memory-safety issues if updates to |
| 7 | + * that field are not guaranteed to be fully or consistently visible to the reading |
| 8 | + * thread. |
| 9 | + * |
| 10 | + * @id todo |
| 11 | + * @kind problem |
| 12 | + */ |
| 13 | + |
| 14 | +// TODO: Seems to have false positives for Kotlin because it does not recognize |
| 15 | +// variable updates in loop body? |
| 16 | + |
| 17 | +// Note: Slightly similar to standard CodeQL query `java/unreachable-exit-in-loop`; |
| 18 | +// might also have some overlap with standard query `java/spin-on-field` |
| 19 | +import java |
| 20 | + |
| 21 | +Expr getAWrite(Variable v) { |
| 22 | + result.(LValue) = v.getAnAccess() |
| 23 | + or |
| 24 | + exists(ArrayAccess arrayAccess | arrayAccess.getArray() = v.getAnAccess() | |
| 25 | + result.(Assignment).getDest() = arrayAccess or |
| 26 | + result.(UnaryAssignExpr).getExpr() = arrayAccess |
| 27 | + ) |
| 28 | +} |
| 29 | + |
| 30 | +predicate isWrittenInLoop(Variable v, LoopStmt loop) { |
| 31 | + exists(Expr write | write = getAWrite(v) | |
| 32 | + write.getAnEnclosingStmt() = loop.getBody() or |
| 33 | + write.getParent*() = [loop.getCondition(), loop.(ForStmt).getAnUpdate()] |
| 34 | + ) |
| 35 | +} |
| 36 | + |
| 37 | +Stmt getAnEnclosingStmt(Expr e) { |
| 38 | + result = e.getAnEnclosingStmt() |
| 39 | + or |
| 40 | + exists(FunctionalExpr f | |
| 41 | + e.getEnclosingCallable() = f.asMethod() and |
| 42 | + result = getAnEnclosingStmt(f) |
| 43 | + ) |
| 44 | +} |
| 45 | + |
| 46 | +from LoopStmt loop |
| 47 | +where |
| 48 | + // Exclude 'for each loop' which is limited implicitly by the number of elements |
| 49 | + exists(loop.getCondition()) and |
| 50 | + // Ignore explicit infinite loops |
| 51 | + not loop.getCondition().(BooleanLiteral).getBooleanValue() = true and |
| 52 | + // If the condition checks local variables, they are not updated in the loop |
| 53 | + forall(LocalScopeVariable localVar | localVar.getAnAccess().getParent*() = loop.getCondition() | |
| 54 | + not isWrittenInLoop(localVar, loop) |
| 55 | + ) and |
| 56 | + // If the condition checks fields, they are not updated in the loop (also not |
| 57 | + // implicitly by calling a method) |
| 58 | + forall(Field f | f.getAnAccess().getParent*() = loop.getCondition() | |
| 59 | + // If `volatile` then it might be updated concurrently by different thread |
| 60 | + not f.isVolatile() and |
| 61 | + // Even if non-volatile, ignore `boolean` fields, maybe there is implicit synchronization |
| 62 | + // logic which makes sure value change is seen eventually by reading thread; however, |
| 63 | + // for other types there might be other thread-safety issues, so don't consider them safe |
| 64 | + not f.getType().hasName("boolean") and |
| 65 | + not isWrittenInLoop(f, loop) and |
| 66 | + not exists(MethodAccess call | |
| 67 | + getAnEnclosingStmt(call) = loop.getBody() |
| 68 | + or |
| 69 | + call.getParent*() = [loop.getCondition(), loop.(ForStmt).getAnUpdate()] |
| 70 | + | |
| 71 | + // But only consider methods in source which might update field, ignore for example |
| 72 | + // calls to JDK methods |
| 73 | + call.getMethod().getSourceDeclaration().fromSource() |
| 74 | + ) |
| 75 | + ) and |
| 76 | + // Ignore if condition depends on method, in that case it is very likely not infinite |
| 77 | + not exists(MethodAccess call | call.getParent*() = loop.getCondition()) and |
| 78 | + // Ignore if loop is left manually using e.g. `break` |
| 79 | + not ( |
| 80 | + any(BreakStmt b).getTarget() = loop or |
| 81 | + any(ReturnStmt r).getEnclosingStmt*() = loop.getBody() |
| 82 | + ) |
| 83 | +select loop, "Potential infinite loop" |
0 commit comments