97 lines
3.9 KiB
Diff
97 lines
3.9 KiB
Diff
From f9b19c9d4caaf870b30cce8a3d6be79eda099c4e Mon Sep 17 00:00:00 2001
|
|
From: Frantisek Sumsal <frantisek@sumsal.cz>
|
|
Date: Sun, 5 Dec 2021 16:11:35 +0100
|
|
Subject: [PATCH] lgtm: detect more possible problematic scenarios
|
|
|
|
1) don't ignore stack-allocated variables, since they may hide
|
|
heap-allocated stuff (compound types)
|
|
2) check if there's a return between the variable declaration and its
|
|
initialization; if so, treat the variable as uninitialized
|
|
3) introduction of 2) increased the query runtime exponentially, so
|
|
introduce some optimizations to bring it back to some reasonable
|
|
values
|
|
|
|
(cherry picked from commit c8fec8bf9b086f9fc7638db0f1a613a00d7c63a3)
|
|
|
|
Related: #2017033
|
|
---
|
|
.../UninitializedVariableWithCleanup.ql | 48 ++++++++++---------
|
|
1 file changed, 25 insertions(+), 23 deletions(-)
|
|
|
|
diff --git a/.lgtm/cpp-queries/UninitializedVariableWithCleanup.ql b/.lgtm/cpp-queries/UninitializedVariableWithCleanup.ql
|
|
index 8c24b6d8f1..6b3b62f8bc 100644
|
|
--- a/.lgtm/cpp-queries/UninitializedVariableWithCleanup.ql
|
|
+++ b/.lgtm/cpp-queries/UninitializedVariableWithCleanup.ql
|
|
@@ -16,24 +16,6 @@
|
|
import cpp
|
|
import semmle.code.cpp.controlflow.StackVariableReachability
|
|
|
|
-/**
|
|
- * Auxiliary predicate: Types that don't require initialization
|
|
- * before they are used, since they're stack-allocated.
|
|
- */
|
|
-predicate allocatedType(Type t) {
|
|
- /* Arrays: "int foo[1]; foo[0] = 42;" is ok. */
|
|
- t instanceof ArrayType
|
|
- or
|
|
- /* Structs: "struct foo bar; bar.baz = 42" is ok. */
|
|
- t instanceof Class
|
|
- or
|
|
- /* Typedefs to other allocated types are fine. */
|
|
- allocatedType(t.(TypedefType).getUnderlyingType())
|
|
- or
|
|
- /* Type specifiers don't affect whether or not a type is allocated. */
|
|
- allocatedType(t.getUnspecifiedType())
|
|
-}
|
|
-
|
|
/** Auxiliary predicate: List cleanup functions we want to explicitly ignore
|
|
* since they don't do anything illegal even when the variable is uninitialized
|
|
*/
|
|
@@ -47,13 +29,11 @@ predicate cleanupFunctionDenyList(string fun) {
|
|
*/
|
|
DeclStmt declWithNoInit(LocalVariable v) {
|
|
result.getADeclaration() = v and
|
|
- not exists(v.getInitializer()) and
|
|
+ not v.hasInitializer() and
|
|
/* The variable has __attribute__((__cleanup__(...))) set */
|
|
v.getAnAttribute().hasName("cleanup") and
|
|
/* Check if the cleanup function is not on a deny list */
|
|
- not exists(Attribute a | a = v.getAnAttribute() and a.getName() = "cleanup" | cleanupFunctionDenyList(a.getAnArgument().getValueText())) and
|
|
- /* The type of the variable is not stack-allocated. */
|
|
- exists(Type t | t = v.getType() | not allocatedType(t))
|
|
+ not cleanupFunctionDenyList(v.getAnAttribute().getAnArgument().getValueText())
|
|
}
|
|
|
|
class UninitialisedLocalReachability extends StackVariableReachability {
|
|
@@ -78,7 +58,29 @@ class UninitialisedLocalReachability extends StackVariableReachability {
|
|
override predicate isBarrier(ControlFlowNode node, StackVariable v) {
|
|
// only report the _first_ possibly uninitialized use
|
|
useOfVar(v, node) or
|
|
- definitionBarrier(v, node)
|
|
+ (
|
|
+ /* If there's an return statement somewhere between the variable declaration
|
|
+ * and a possible definition, don't accept is as a valid initialization.
|
|
+ *
|
|
+ * E.g.:
|
|
+ * _cleanup_free_ char *x;
|
|
+ * ...
|
|
+ * if (...)
|
|
+ * return;
|
|
+ * ...
|
|
+ * x = malloc(...);
|
|
+ *
|
|
+ * is not a valid initialization, since we might return from the function
|
|
+ * _before_ the actual iniitialization (emphasis on _might_, since we
|
|
+ * don't know if the return statement might ever evaluate to true).
|
|
+ */
|
|
+ definitionBarrier(v, node) and
|
|
+ not exists(ReturnStmt rs |
|
|
+ /* The attribute check is "just" a complexity optimization */
|
|
+ v.getFunction() = rs.getEnclosingFunction() and v.getAnAttribute().hasName("cleanup") |
|
|
+ rs.getLocation().isBefore(node.getLocation())
|
|
+ )
|
|
+ )
|
|
}
|
|
}
|
|
|