Merge pull request github#3637 from jbj/dispatch-global-perf

C++: Fix data-flow dispatch perf with globals

Author: Dave Bartolomeo

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowDispatch.qll

@@ -82,47 +82,45 @@ private module VirtualDispatch {
         exists(LoadInstruction load, GlobalOrNamespaceVariable var |
           var = src.asVariable() and
           other.asInstruction() = load and
+          addressOfGlobal(load.getSourceAddress(), var) and
           // The `allowFromArg` concept doesn't play a role when `src` is a
           // global variable, so we just set it to a single arbitrary value for
           // performance.
           allowFromArg = true
-        |
-          // Load directly from the global variable
-          load.getSourceAddress().(VariableAddressInstruction).getASTVariable() = var
-          or
-          // Load from a field on a global union
-          exists(FieldAddressInstruction fa |
-            fa = load.getSourceAddress() and
-            fa.getObjectAddress().(VariableAddressInstruction).getASTVariable() = var and
-            fa.getField().getDeclaringType() instanceof Union
-          )
         )
         or
-        // Flow from store to global variable. These cases are similar to the
-        // above but have `StoreInstruction` instead of `LoadInstruction` and
-        // have the roles swapped between `other` and `src`.
+        // Flow from store to global variable.
         exists(StoreInstruction store, GlobalOrNamespaceVariable var |
           var = other.asVariable() and
           store = src.asInstruction() and
+          storeIntoGlobal(store, var) and
           // Setting `allowFromArg` to `true` like in the base case means we
           // treat a store to a global variable like the dispatch itself: flow
           // may come from anywhere.
           allowFromArg = true
-        |
-          // Store directly to the global variable
-          store.getDestinationAddress().(VariableAddressInstruction).getASTVariable() = var
-          or
-          // Store to a field on a global union
-          exists(FieldAddressInstruction fa |
-            fa = store.getDestinationAddress() and
-            fa.getObjectAddress().(VariableAddressInstruction).getASTVariable() = var and
-            fa.getField().getDeclaringType() instanceof Union
-          )
         )
       )
     }
   }
 
+  pragma[noinline]
+  private predicate storeIntoGlobal(StoreInstruction store, GlobalOrNamespaceVariable var) {
+    addressOfGlobal(store.getDestinationAddress(), var)
+  }
+
+  /** Holds if `addressInstr` is an instruction that produces the address of `var`. */
+  private predicate addressOfGlobal(Instruction addressInstr, GlobalOrNamespaceVariable var) {
+    // Access directly to the global variable
+    addressInstr.(VariableAddressInstruction).getASTVariable() = var
+    or
+    // Access to a field on a global union
+    exists(FieldAddressInstruction fa |
+      fa = addressInstr and
+      fa.getObjectAddress().(VariableAddressInstruction).getASTVariable() = var and
+      fa.getField().getDeclaringType() instanceof Union
+    )
+  }
+
   /**
    * A ReturnNode with its ReturnKind and its enclosing callable.
    *