Merge pull request github#3612 from dbartol/github/codeql-c-analysis-team/69_union

C++: Share `TInstruction` across IR stages

Author: jbj

config/identical-files.json

@@ -96,10 +96,18 @@
     "cpp/ql/src/semmle/code/cpp/ir/implementation/UseSoundEscapeAnalysis.qll",
     "csharp/ql/src/experimental/ir/implementation/UseSoundEscapeAnalysis.qll"
   ],
+  "IR IRFunctionBase": [
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/internal/IRFunctionBase.qll",
+    "csharp/ql/src/experimental/ir/implementation/internal/IRFunctionBase.qll"
+  ],
   "IR Operand Tag": [
     "cpp/ql/src/semmle/code/cpp/ir/implementation/internal/OperandTag.qll",
     "csharp/ql/src/experimental/ir/implementation/internal/OperandTag.qll"
   ],
+  "IR TInstruction":[
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/internal/TInstruction.qll",
+    "csharp/ql/src/experimental/ir/implementation/internal/TInstruction.qll"
+  ],
   "IR TIRVariable":[
     "cpp/ql/src/semmle/code/cpp/ir/implementation/internal/TIRVariable.qll",
     "csharp/ql/src/experimental/ir/implementation/internal/TIRVariable.qll"
@@ -177,6 +185,11 @@
     "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/IRBlockImports.qll",
     "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/IRBlockImports.qll"
   ],
+  "C++ IR IRFunctionImports": [
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/IRFunctionImports.qll",
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/IRFunctionImports.qll",
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/IRFunctionImports.qll"
+  ],
   "C++ IR IRVariableImports": [
     "cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/IRVariableImports.qll",
     "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/IRVariableImports.qll",
@@ -287,6 +300,10 @@
     "csharp/ql/src/experimental/ir/implementation/raw/internal/IRBlockImports.qll",
     "csharp/ql/src/experimental/ir/implementation/unaliased_ssa/internal/IRBlockImports.qll"
   ],
+  "C# IR IRFunctionImports": [
+    "csharp/ql/src/experimental/ir/implementation/raw/internal/IRFunctionImports.qll",
+    "csharp/ql/src/experimental/ir/implementation/unaliased_ssa/internal/IRFunctionImports.qll"
+  ],
   "C# IR IRVariableImports": [
     "csharp/ql/src/experimental/ir/implementation/raw/internal/IRVariableImports.qll",
     "csharp/ql/src/experimental/ir/implementation/unaliased_ssa/internal/IRVariableImports.qll"

cpp/ql/src/Likely Bugs/RedundantNullCheckSimple.ql

@@ -23,7 +23,7 @@ import semmle.code.cpp.ir.ValueNumbering
 class NullInstruction extends ConstantValueInstruction {
   NullInstruction() {
     this.getValue() = "0" and
-    this.getResultType().getUnspecifiedType() instanceof PointerType
+    this.getResultIRType() instanceof IRAddressType
   }
 }
 
@@ -44,8 +44,8 @@ predicate explicitNullTestOfInstruction(Instruction checked, Instruction bool) {
   bool =
     any(ConvertInstruction convert |
       checked = convert.getUnary() and
-      convert.getResultType() instanceof BoolType and
-      checked.getResultType() instanceof PointerType
+      convert.getResultIRType() instanceof IRBooleanType and
+      checked.getResultIRType() instanceof IRAddressType
     )
 }
 

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

@@ -128,8 +128,13 @@ private class SideEffectOutNode extends OutNode {
  * `kind`.
  */
 OutNode getAnOutNode(DataFlowCall call, ReturnKind kind) {
-  result.getCall() = call and
-  result.getReturnKind() = kind
+  // There should be only one `OutNode` for a given `(call, kind)` pair. Showing the optimizer that
+  // this is true helps it make better decisions downstream, especially in virtual dispatch.
+  result =
+    unique(OutNode outNode |
+      outNode.getCall() = call and
+      outNode.getReturnKind() = kind
+    )
 }
 
 /**

cpp/ql/src/semmle/code/cpp/ir/implementation/IRType.qll

@@ -111,6 +111,8 @@ private class IRSizedType extends IRType {
     this = TIRFunctionAddressType(byteSize) or
     this = TIROpaqueType(_, byteSize)
   }
+  // Don't override `getByteSize()` here. The optimizer seems to generate better code when this is
+  // overridden only in the leaf classes.
 }
 
 /**
@@ -128,7 +130,7 @@ class IRBooleanType extends IRSizedType, TIRBooleanType {
 }
 
 /**
- * A numberic type. This includes `IRSignedIntegerType`, `IRUnsignedIntegerType`, and
+ * A numeric type. This includes `IRSignedIntegerType`, `IRUnsignedIntegerType`, and
  * `IRFloatingPointType`.
  */
 class IRNumericType extends IRSizedType {
@@ -137,13 +139,27 @@ class IRNumericType extends IRSizedType {
     this = TIRUnsignedIntegerType(byteSize) or
     this = TIRFloatingPointType(byteSize, _, _)
   }
+  // Don't override `getByteSize()` here. The optimizer seems to generate better code when this is
+  // overridden only in the leaf classes.
+}
+
+/**
+ * An integer type. This includes `IRSignedIntegerType` and `IRUnsignedIntegerType`.
+ */
+class IRIntegerType extends IRNumericType {
+  IRIntegerType() {
+    this = TIRSignedIntegerType(byteSize) or
+    this = TIRUnsignedIntegerType(byteSize)
+  }
+  // Don't override `getByteSize()` here. The optimizer seems to generate better code when this is
+  // overridden only in the leaf classes.
 }
 
 /**
  * A signed two's-complement integer. Also used to represent enums whose underlying type is a signed
  * integer, as well as character types whose representation is signed.
  */
-class IRSignedIntegerType extends IRNumericType, TIRSignedIntegerType {
+class IRSignedIntegerType extends IRIntegerType, TIRSignedIntegerType {
   final override string toString() { result = "int" + byteSize.toString() }
 
   final override Language::LanguageType getCanonicalLanguageType() {
@@ -158,7 +174,7 @@ class IRSignedIntegerType extends IRNumericType, TIRSignedIntegerType {
  * An unsigned two's-complement integer. Also used to represent enums whose underlying type is an
  * unsigned integer, as well as character types whose representation is unsigned.
  */
-class IRUnsignedIntegerType extends IRNumericType, TIRUnsignedIntegerType {
+class IRUnsignedIntegerType extends IRIntegerType, TIRUnsignedIntegerType {
   final override string toString() { result = "uint" + byteSize.toString() }
 
   final override Language::LanguageType getCanonicalLanguageType() {

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRFunction.qll

@@ -1,29 +1,12 @@
 private import internal.IRInternal
+private import internal.IRFunctionImports as Imports
+import Imports::IRFunctionBase
 import Instruction
 
-private newtype TIRFunction =
-  MkIRFunction(Language::Function func) { Construction::functionHasIR(func) }
-
 /**
- * Represents the IR for a function.
+ * The IR for a function.
  */
-class IRFunction extends TIRFunction {
-  Language::Function func;
-
-  IRFunction() { this = MkIRFunction(func) }
-
-  final string toString() { result = "IR: " + func.toString() }
-
-  /**
-   * Gets the function whose IR is represented.
-   */
-  final Language::Function getFunction() { result = func }
-
-  /**
-   * Gets the location of the function.
-   */
-  final Language::Location getLocation() { result = func.getLocation() }
-
+class IRFunction extends IRFunctionBase {
   /**
    * Gets the entry point for this function.
    */

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll

@@ -29,7 +29,13 @@ private Instruction getAnInstructionAtLine(IRFunction irFunc, Language::File fil
 /**
  * Represents a single operation in the IR.
  */
-class Instruction extends Construction::TInstruction {
+class Instruction extends Construction::TStageInstruction {
+  Instruction() {
+    // The base `TStageInstruction` type is a superset of the actual instructions appearing in this
+    // stage. This call lets the stage filter out the ones that are not reused from raw IR.
+    Construction::hasInstruction(this)
+  }
+
   final string toString() { result = getOpcode().toString() + ": " + getAST().toString() }
 
   /**
@@ -194,14 +200,14 @@ class Instruction extends Construction::TInstruction {
    * conversion.
    */
   final Language::Expr getConvertedResultExpression() {
-    result = Construction::getInstructionConvertedResultExpression(this)
+    result = Raw::getInstructionConvertedResultExpression(this)
   }
 
   /**
    * Gets the unconverted form of the `Expr` whose result is computed by this instruction, if any.
    */
   final Language::Expr getUnconvertedResultExpression() {
-    result = Construction::getInstructionUnconvertedResultExpression(this)
+    result = Raw::getInstructionUnconvertedResultExpression(this)
   }
 
   final Language::LanguageType getResultLanguageType() {
@@ -212,6 +218,7 @@ class Instruction extends Construction::TInstruction {
    * Gets the type of the result produced by this instruction. If the instruction does not produce
    * a result, its result type will be `IRVoidType`.
    */
+  cached
   final IRType getResultIRType() { result = getResultLanguageType().getIRType() }
 
   /**
@@ -250,7 +257,7 @@ class Instruction extends Construction::TInstruction {
    * result of the `Load` instruction is a prvalue of type `int`, representing
    * the integer value loaded from variable `x`.
    */
-  final predicate isGLValue() { Construction::getInstructionResultType(this).hasType(_, true) }
+  final predicate isGLValue() { getResultLanguageType().hasType(_, true) }
 
   /**
    * Gets the size of the result produced by this instruction, in bytes. If the
@@ -259,7 +266,7 @@ class Instruction extends Construction::TInstruction {
    * If `this.isGLValue()` holds for this instruction, the value of
    * `getResultSize()` will always be the size of a pointer.
    */
-  final int getResultSize() { result = Construction::getInstructionResultType(this).getByteSize() }
+  final int getResultSize() { result = getResultLanguageType().getByteSize() }
 
   /**
    * Gets the opcode that specifies the operation performed by this instruction.
@@ -395,7 +402,7 @@ class Instruction extends Construction::TInstruction {
 class VariableInstruction extends Instruction {
   IRVariable var;
 
-  VariableInstruction() { var = Construction::getInstructionVariable(this) }
+  VariableInstruction() { var = Raw::getInstructionVariable(this) }
 
   override string getImmediateString() { result = var.toString() }
 
@@ -410,7 +417,7 @@ class VariableInstruction extends Instruction {
 class FieldInstruction extends Instruction {
   Language::Field field;
 
-  FieldInstruction() { field = Construction::getInstructionField(this) }
+  FieldInstruction() { field = Raw::getInstructionField(this) }
 
   final override string getImmediateString() { result = field.toString() }
 
@@ -420,7 +427,7 @@ class FieldInstruction extends Instruction {
 class FunctionInstruction extends Instruction {
   Language::Function funcSymbol;
 
-  FunctionInstruction() { funcSymbol = Construction::getInstructionFunction(this) }
+  FunctionInstruction() { funcSymbol = Raw::getInstructionFunction(this) }
 
   final override string getImmediateString() { result = funcSymbol.toString() }
 
@@ -430,7 +437,7 @@ class FunctionInstruction extends Instruction {
 class ConstantValueInstruction extends Instruction {
   string value;
 
-  ConstantValueInstruction() { value = Construction::getInstructionConstantValue(this) }
+  ConstantValueInstruction() { value = Raw::getInstructionConstantValue(this) }
 
   final override string getImmediateString() { result = value }
 
@@ -440,7 +447,7 @@ class ConstantValueInstruction extends Instruction {
 class IndexedInstruction extends Instruction {
   int index;
 
-  IndexedInstruction() { index = Construction::getInstructionIndex(this) }
+  IndexedInstruction() { index = Raw::getInstructionIndex(this) }
 
   final override string getImmediateString() { result = index.toString() }
 
@@ -603,11 +610,16 @@ class ConstantInstruction extends ConstantValueInstruction {
 }
 
 class IntegerConstantInstruction extends ConstantInstruction {
-  IntegerConstantInstruction() { getResultType() instanceof Language::IntegralType }
+  IntegerConstantInstruction() {
+    exists(IRType resultType |
+      resultType = getResultIRType() and
+      (resultType instanceof IRIntegerType or resultType instanceof IRBooleanType)
+    )
+  }
 }
 
 class FloatConstantInstruction extends ConstantInstruction {
-  FloatConstantInstruction() { getResultType() instanceof Language::FloatingPointType }
+  FloatConstantInstruction() { getResultIRType() instanceof IRFloatingPointType }
 }
 
 class StringConstantInstruction extends VariableInstruction {
@@ -704,7 +716,7 @@ class PointerArithmeticInstruction extends BinaryInstruction {
 
   PointerArithmeticInstruction() {
     getOpcode() instanceof PointerArithmeticOpcode and
-    elementSize = Construction::getInstructionElementSize(this)
+    elementSize = Raw::getInstructionElementSize(this)
   }
 
   final override string getImmediateString() { result = elementSize.toString() }
@@ -753,7 +765,7 @@ class InheritanceConversionInstruction extends UnaryInstruction {
   Language::Class derivedClass;
 
   InheritanceConversionInstruction() {
-    Construction::getInstructionInheritance(this, baseClass, derivedClass)
+    Raw::getInstructionInheritance(this, baseClass, derivedClass)
   }
 
   final override string getImmediateString() {
@@ -1216,7 +1228,7 @@ class CatchByTypeInstruction extends CatchInstruction {
 
   CatchByTypeInstruction() {
     getOpcode() instanceof Opcode::CatchByType and
-    exceptionType = Construction::getInstructionExceptionType(this)
+    exceptionType = Raw::getInstructionExceptionType(this)
   }
 
   final override string getImmediateString() { result = exceptionType.toString() }
@@ -1362,7 +1374,7 @@ class BuiltInOperationInstruction extends Instruction {
 
   BuiltInOperationInstruction() {
     getOpcode() instanceof BuiltInOperationOpcode and
-    operation = Construction::getInstructionBuiltInOperation(this)
+    operation = Raw::getInstructionBuiltInOperation(this)
   }
 
   final Language::BuiltInOperation getBuiltInOperation() { result = operation }

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/IRFunctionImports.qll

@@ -0,0 +1 @@
+import semmle.code.cpp.ir.implementation.internal.IRFunctionBase as IRFunctionBase

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/IRInternal.qll

@@ -1,3 +1,4 @@
 import semmle.code.cpp.ir.internal.IRCppLanguage as Language
 import SSAConstruction as Construction
 import semmle.code.cpp.ir.implementation.IRConfiguration as IRConfiguration
+import semmle.code.cpp.ir.implementation.raw.internal.IRConstruction::Raw as Raw