C++: Use StackVariable, remove not v.isStatic()

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.

Author: jbj

cpp/ql/src/Critical/ReturnStackAllocatedObject.ql

@@ -20,11 +20,10 @@ class ReturnPointsToExpr extends PointsToExpr {
   ReturnStmt getReturnStmt() { result.getExpr().getFullyConverted() = this }
 }
 
-from ReturnPointsToExpr ret, LocalVariable local, float confidence
+from ReturnPointsToExpr ret, StackVariable local, float confidence
 where
   ret.pointsTo() = local and
   ret.getReturnStmt().getEnclosingFunction() = local.getFunction() and
-  not local.isStatic() and
   confidence = ret.confidence() and
   confidence > 0.01
 select ret,

cpp/ql/src/Critical/Unused.ql

@@ -20,11 +20,10 @@ class ScopeUtilityClass extends Class {
   Call getAUse() { result = this.getAConstructor().getACallToThisFunction() }
 }
 
-from LocalScopeVariable v, ControlFlowNode def
+from StackVariable v, ControlFlowNode def
 where
   definition(v, def) and
   not definitionUsePair(v, def, _) and
-  not v.isStatic() and
   not v.getAnAccess().isAddressOfAccess() and
   // parameter initializers are not in the call-graph at the moment
   not v.(Parameter).getInitializer().getExpr() = def and

cpp/ql/src/Likely Bugs/Memory Management/ReturnStackAllocatedMemory.ql

@@ -42,10 +42,8 @@ predicate hasNontrivialConversion(Expr e) {
   hasNontrivialConversion(e.getConversion())
 }
 
-from LocalScopeVariable var, VariableAccess va, ReturnStmt r
+from StackVariable var, VariableAccess va, ReturnStmt r
 where
-  not var.isStatic() and
-  not var.isThreadLocal() and
   not var.getUnspecifiedType() instanceof ReferenceType and
   not r.isFromUninstantiatedTemplate(_) and
   va = var.getAnAccess() and

cpp/ql/src/Likely Bugs/Memory Management/UninitializedLocal.ql

@@ -12,7 +12,7 @@
  */
 
 import cpp
-import semmle.code.cpp.controlflow.LocalScopeVariableReachability
+import semmle.code.cpp.controlflow.StackVariableReachability
 
 /**
  * Auxiliary predicate: Types that don't require initialization
@@ -40,23 +40,17 @@ DeclStmt declWithNoInit(LocalVariable v) {
   result.getADeclaration() = v and
   not exists(v.getInitializer()) and
   /* The type of the variable is not stack-allocated. */
-  not allocatedType(v.getType()) and
-  /* The variable is not static (otherwise it is zeroed). */
-  not v.isStatic() and
-  /* The variable is not extern (otherwise it is zeroed). */
-  not v.hasSpecifier("extern")
+  not allocatedType(v.getType())
 }
 
-class UninitialisedLocalReachability extends LocalScopeVariableReachability {
+class UninitialisedLocalReachability extends StackVariableReachability {
   UninitialisedLocalReachability() { this = "UninitialisedLocal" }
 
-  override predicate isSource(ControlFlowNode node, LocalScopeVariable v) {
-    node = declWithNoInit(v)
-  }
+  override predicate isSource(ControlFlowNode node, StackVariable v) { node = declWithNoInit(v) }
 
-  override predicate isSink(ControlFlowNode node, LocalScopeVariable v) { useOfVarActual(v, node) }
+  override predicate isSink(ControlFlowNode node, StackVariable v) { useOfVarActual(v, node) }
 
-  override predicate isBarrier(ControlFlowNode node, LocalScopeVariable v) {
+  override predicate isBarrier(ControlFlowNode node, StackVariable v) {
     // only report the _first_ possibly uninitialized use
     useOfVarActual(v, node) or
     definitionBarrier(v, node)

cpp/ql/src/jsf/4.13 Functions/AV Rule 111.ql

@@ -20,10 +20,9 @@ class ReturnPointsToExpr extends PointsToExpr {
   ReturnStmt getReturnStmt() { result.getExpr() = this }
 }
 
-from ReturnPointsToExpr ret, LocalVariable dest
+from ReturnPointsToExpr ret, StackVariable dest
 where
   ret.pointsTo() = dest and
-  ret.getReturnStmt().getParentStmt().getEnclosingFunction() = dest.getFunction() and
-  not dest.isStatic()
+  ret.getReturnStmt().getParentStmt().getEnclosingFunction() = dest.getFunction()
 select ret.getReturnStmt(),
   "AV Rule 111: A function shall not return a pointer or reference to a non-static local object."

cpp/ql/src/semmle/code/cpp/dataflow/StackAddress.qll

@@ -61,7 +61,7 @@ predicate stackPointerFlowsToUse(Expr use, Type useType, Expr source, boolean is
   stackPointerFlowsToUse(use.(PointerAddExpr).getAnOperand(), useType, source, isLocal)
   or
   // Indirect use of a stack address.
-  exists(SsaDefinition def, LocalScopeVariable var |
+  exists(SsaDefinition def, StackVariable var |
     stackPointerFlowsToDef(def, var, useType, source, isLocal) and
     use = def.getAUse(var)
   )
@@ -97,8 +97,7 @@ private PointerType getExprPtrType(Expr use) { result = use.getUnspecifiedType()
 
 predicate stackReferenceFlowsToUse(Expr use, Type useType, Expr source, boolean isLocal) {
   // Stack variables
-  exists(LocalScopeVariable var |
-    not var.isStatic() and
+  exists(StackVariable var |
     use = source and
     source = var.getAnAccess() and
     isLocal = true and
@@ -140,7 +139,7 @@ predicate stackReferenceFlowsToUse(Expr use, Type useType, Expr source, boolean
   stackPointerFlowsToUse(use.(PointerDereferenceExpr).getOperand(), useType, source, isLocal)
   or
   // Indirect use of a stack reference, via a reference variable.
-  exists(SsaDefinition def, LocalScopeVariable var |
+  exists(SsaDefinition def, StackVariable var |
     stackReferenceFlowsToDef(def, var, useType, source, isLocal) and
     use = def.getAUse(var)
   )
@@ -162,7 +161,7 @@ predicate stackReferenceFlowsToUse(Expr use, Type useType, Expr source, boolean
  * addresses through SSA definitions.
  */
 predicate stackPointerFlowsToDef(
-  SsaDefinition def, LocalScopeVariable var, Type useType, Expr source, boolean isLocal
+  SsaDefinition def, StackVariable var, Type useType, Expr source, boolean isLocal
 ) {
   stackPointerFlowsToUse(def.getDefiningValue(var), useType, source, isLocal)
   or
@@ -184,7 +183,7 @@ predicate stackPointerFlowsToDef(
  * int&, rather than pointers.
  */
 predicate stackReferenceFlowsToDef(
-  SsaDefinition def, LocalScopeVariable var, Type useType, Expr source, boolean isLocal
+  SsaDefinition def, StackVariable var, Type useType, Expr source, boolean isLocal
 ) {
   // Check that the type of the variable is a reference type and delegate
   // the rest of the work to stackReferenceFlowsToDef_Impl.
@@ -197,7 +196,7 @@ predicate stackReferenceFlowsToDef(
  * predicate.
  */
 predicate stackReferenceFlowsToDef_Impl(
-  SsaDefinition def, LocalScopeVariable var, Type useType, Expr source, boolean isLocal
+  SsaDefinition def, StackVariable var, Type useType, Expr source, boolean isLocal
 ) {
   stackReferenceFlowsToUse(def.getDefiningValue(var), useType, source, isLocal)
   or
@@ -213,7 +212,7 @@ predicate stackReferenceFlowsToDef_Impl(
 }
 
 /** The type of the variable is a reference type, such as int&. */
-predicate isReferenceVariable(LocalScopeVariable var) {
+predicate isReferenceVariable(StackVariable var) {
   var.getUnspecifiedType() instanceof ReferenceType
 }
 
@@ -284,7 +283,7 @@ predicate memberFcnMightRunOnStack(MemberFunction fcn, Type useType) {
 predicate constructorMightRunOnStack(Constructor constructor) {
   exists(ConstructorCall call | call.getTarget() = constructor |
     // Call to a constructor from a stack variable's initializer.
-    exists(LocalScopeVariable var | var.getInitializer().getExpr() = call)
+    exists(StackVariable var | var.getInitializer().getExpr() = call)
     or
     // Call to a constructor from another constructor which might
     // also run on the stack.

cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll

@@ -88,7 +88,7 @@ class FlowVar extends TFlowVar {
    *   `FlowVar` instance for the uninitialized value of that variable.
    */
   cached
-  abstract predicate definedByInitialValue(LocalScopeVariable v);
+  abstract predicate definedByInitialValue(StackVariable v);
 
   /** Gets a textual representation of this element. */
   cached
@@ -269,7 +269,7 @@ module FlowVar_internal {
    * Holds if `sbb` is the `SubBasicBlock` where `v` receives its initial value.
    * See the documentation for `FlowVar.definedByInitialValue`.
    */
-  predicate blockVarDefinedByVariable(SubBasicBlock sbb, LocalScopeVariable v) {
+  predicate blockVarDefinedByVariable(SubBasicBlock sbb, StackVariable v) {
     sbb = v.(Parameter).getFunction().getEntryPoint()
     or
     exists(DeclStmt declStmt |
@@ -280,7 +280,7 @@ module FlowVar_internal {
   }
 
   newtype TFlowVar =
-    TSsaVar(SsaDefinition def, LocalScopeVariable v) {
+    TSsaVar(SsaDefinition def, StackVariable v) {
       fullySupportedSsaVariable(v) and
       v = def.getAVariable()
     } or
@@ -304,7 +304,7 @@ module FlowVar_internal {
    */
   class SsaVar extends TSsaVar, FlowVar {
     SsaDefinition def;
-    LocalScopeVariable v;
+    StackVariable v;
 
     SsaVar() { this = TSsaVar(def, v) }
 
@@ -344,7 +344,7 @@ module FlowVar_internal {
 
     override predicate definedPartiallyAt(Expr e) { none() }
 
-    override predicate definedByInitialValue(LocalScopeVariable param) {
+    override predicate definedByInitialValue(StackVariable param) {
       def.definedByParameter(param) and
       param = v
     }
@@ -408,7 +408,7 @@ module FlowVar_internal {
       getAReachedBlockVarSBB(this).getANode() = p.getFunction()
     }
 
-    override predicate definedByInitialValue(LocalScopeVariable lsv) {
+    override predicate definedByInitialValue(StackVariable lsv) {
       blockVarDefinedByVariable(sbb, lsv) and
       lsv = v
     }
@@ -648,11 +648,8 @@ module FlowVar_internal {
   /**
    * A local variable that is uninitialized immediately after its declaration.
    */
-  class UninitializedLocalVariable extends LocalVariable {
-    UninitializedLocalVariable() {
-      not this.hasInitializer() and
-      not this.isStatic()
-    }
+  class UninitializedLocalVariable extends LocalVariable, StackVariable {
+    UninitializedLocalVariable() { not this.hasInitializer() }
   }
 
   /** Holds if `va` may be an uninitialized access to `v`. */

cpp/ql/src/semmle/code/cpp/exprs/ArithmeticOperation.qll

@@ -62,11 +62,10 @@ abstract class CrementOperation extends UnaryArithmeticOperation {
   override predicate mayBeImpure() { any() }
 
   override predicate mayBeGloballyImpure() {
-    not exists(VariableAccess va, LocalScopeVariable v |
+    not exists(VariableAccess va, StackVariable v |
       va = this.getOperand() and
       v = va.getTarget() and
-      not va.getConversion+() instanceof ReferenceDereferenceExpr and
-      not v.isStatic()
+      not va.getConversion+() instanceof ReferenceDereferenceExpr
     )
   }
 }

cpp/ql/src/semmle/code/cpp/exprs/Assignment.qll

@@ -21,11 +21,10 @@ abstract class Assignment extends Operation {
   override predicate mayBeGloballyImpure() {
     this.getRValue().mayBeGloballyImpure()
     or
-    not exists(VariableAccess va, LocalScopeVariable v |
+    not exists(VariableAccess va, StackVariable v |
       va = this.getLValue() and
       v = va.getTarget() and
-      not va.getConversion+() instanceof ReferenceDereferenceExpr and
-      not v.isStatic()
+      not va.getConversion+() instanceof ReferenceDereferenceExpr
     )
   }
 }

cpp/ql/src/semmle/code/cpp/ir/internal/IRCppLanguage.qll

@@ -32,7 +32,7 @@ class StringLiteral = Cpp::StringLiteral;
 
 class Variable = Cpp::Variable;
 
-class AutomaticVariable = Cpp::LocalScopeVariable;
+class AutomaticVariable = Cpp::StackVariable;
 
 class StaticVariable = Cpp::Variable;
 
@@ -66,10 +66,7 @@ int getTypeSize(Type type) { result = type.getSize() }
 
 int getPointerSize() { exists(Cpp::NullPointerType nullptr | result = nullptr.getSize()) }
 
-predicate isVariableAutomatic(Variable var) {
-  var instanceof Cpp::LocalScopeVariable and
-  not var.(Cpp::LocalScopeVariable).isStatic()
-}
+predicate isVariableAutomatic(Cpp::StackVariable var) { any() }
 
 string getStringLiteralText(StringLiteral s) {
   result = s.getValueText().replaceAll("\n", " ").replaceAll("\r", "").replaceAll("\t", " ")