C++: Fix up after merge from master

The one interesting piece that needed to be fixed up was the type of an `Indirect[Read|Write]SideEffect` operand/result. If the parameter type is a pointer or reference to an incomplete type, we need to set the type of the side effect memory access to `Unknown`, because we don't model incomplete types in the IR type system. I also added minimal support for `__assume` (generated as a `NoOp`), because lack of `__assume` support got in the way of debugging the other issue above.
2026-04-30 11:15:13 +02:00 · 2019-10-16 15:55:56 -07:00
parent 167d2289c4
commit 6e61b1dcd0
9 changed files with 93 additions and 45 deletions
--- a/cpp/ql/src/semmle/code/cpp/exprs/Expr.qll
+++ b/cpp/ql/src/semmle/code/cpp/exprs/Expr.qll
@@ -540,6 +540,13 @@ class ErrorExpr extends Expr, @errorexpr {
 */
 class AssumeExpr extends Expr, @assume {
  override string toString() { result = "__assume(...)" }
+
+  override string getCanonicalQLClass() { result = "AssumeExpr" }
+
+  /**
+   * Gets the operand of the `__assume` expressions.
+   */
+  Expr getOperand() { this.hasChild(result, 0) }
 }

 /**
--- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll
+++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll
@@ -340,9 +340,7 @@ class TranslatedSideEffects extends TranslatedElement, TTranslatedSideEffects {
    )
  }

-  override predicate hasInstruction(Opcode opcode, InstructionTag tag, Type t, boolean isGLValue) {
-    none()
-  }
+  override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType type) { none() }

  override Instruction getFirstInstruction() { result = getChild(0).getFirstInstruction() }

@@ -350,7 +348,9 @@ class TranslatedSideEffects extends TranslatedElement, TTranslatedSideEffects {

  override Instruction getInstructionOperand(InstructionTag tag, OperandTag operandTag) { none() }

-  override Type getInstructionOperandType(InstructionTag tag, TypedOperandTag operandTag) { none() }
+  override CppType getInstructionOperandType(InstructionTag tag, TypedOperandTag operandTag) {
+    none()
+  }

  /**
   * Gets the `TranslatedFunction` containing this expression.
@@ -397,34 +397,30 @@ class TranslatedSideEffect extends TranslatedElement, TTranslatedArgumentSideEff

  override Instruction getFirstInstruction() { result = getInstruction(OnlyInstructionTag()) }

-  override predicate hasInstruction(Opcode opcode, InstructionTag tag, Type t, boolean isGLValue) {
+  override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType type) {
    isWrite() and
    hasSpecificWriteSideEffect(opcode) and
    tag = OnlyInstructionTag() and
    (
      opcode instanceof BufferAccessOpcode and
-      t instanceof UnknownType
+      type = getUnknownType()
      or
      not opcode instanceof BufferAccessOpcode and
-      (
-        t = arg.getUnspecifiedType().(DerivedType).getBaseType() and
-        not t instanceof VoidType
-        or
-        arg.getUnspecifiedType().(DerivedType).getBaseType() instanceof VoidType and
-        t instanceof UnknownType
+      exists(Type baseType | baseType = arg.getUnspecifiedType().(DerivedType).getBaseType() |
+        if baseType instanceof VoidType
+        then type = getUnknownType()
+        else type = getTypeForPRValueOrUnknown(baseType)
      )
      or
      index = -1 and
      not arg.getUnspecifiedType() instanceof DerivedType and
-      t = arg.getUnspecifiedType()
-    ) and
-    isGLValue = false
+      type = getTypeForPRValueOrUnknown(arg.getUnspecifiedType())
+    )
    or
    not isWrite() and
    hasSpecificReadSideEffect(opcode) and
    tag = OnlyInstructionTag() and
-    t instanceof VoidType and
-    isGLValue = false
+    type = getVoidType()
  }

  override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
@@ -450,15 +446,20 @@ class TranslatedSideEffect extends TranslatedElement, TTranslatedArgumentSideEff
            .getFullyConverted()).getResult()
  }

-  override Type getInstructionOperandType(InstructionTag tag, TypedOperandTag operandTag) {
-    tag instanceof OnlyInstructionTag and
-    result = arg.getType().getUnspecifiedType().(DerivedType).getBaseType() and
-    operandTag instanceof SideEffectOperandTag
-    or
-    tag instanceof OnlyInstructionTag and
-    result = arg.getType().getUnspecifiedType() and
-    not result instanceof DerivedType and
-    operandTag instanceof SideEffectOperandTag
+  override CppType getInstructionOperandType(InstructionTag tag, TypedOperandTag operandTag) {
+    exists(Type operandType |
+      tag instanceof OnlyInstructionTag and
+      operandType = arg.getType().getUnspecifiedType().(DerivedType).getBaseType() and
+      operandTag instanceof SideEffectOperandTag
+      or
+      tag instanceof OnlyInstructionTag and
+      operandType = arg.getType().getUnspecifiedType() and
+      not operandType instanceof DerivedType and
+      operandTag instanceof SideEffectOperandTag |
+      // If the type we select is an incomplete type (e.g. a forward-declared `struct`), there will
+      // not be a `CppType` that represents that type. In that case, fall back to `UnknownCppType`.
+      result = getTypeForPRValueOrUnknown(operandType)
+    )
  }

  predicate hasSpecificWriteSideEffect(Opcode op) {
--- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll
+++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll
@@ -49,6 +49,9 @@ private predicate ignoreExprAndDescendants(Expr expr) {
  // constant value.
  isIRConstant(getRealParent(expr))
  or
+  // Ignore descendants of `__assume` expressions, since we translated these to `NoOp`.
+  getRealParent(expr) instanceof AssumeExpr
+  or
  // The `DestructorCall` node for a `DestructorFieldDestruction` has a `FieldAccess`
  // node as its qualifier, but that `FieldAccess` does not have a child of its own.
  // We'll ignore that `FieldAccess`, and supply the receiver as part of the calling
--- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll
+++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll
@@ -2409,3 +2409,26 @@ class TranslatedErrorExpr extends TranslatedSingleInstructionExpr {

  final override Opcode getOpcode() { result instanceof Opcode::Error }
 }
+
+/**
+ * The IR translation of an `__assume` expression. We currently translate these as `NoOp`. In the
+ * future, we will probably want to do something better. At a minimum, we can model `__assume(0)` as
+ * `Unreached`.
+ */
+class TranslatedAssumeExpr extends TranslatedSingleInstructionExpr {
+  override AssumeExpr expr;
+
+  final override Opcode getOpcode() { result instanceof Opcode::NoOp }
+
+  final override Instruction getFirstInstruction() { result = getInstruction(OnlyInstructionTag()) }
+
+  final override TranslatedElement getChild(int id) { none() }
+
+  final override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
+    tag = OnlyInstructionTag() and
+    result = getParent().getChildSuccessor(this) and
+    kind instanceof GotoEdge
+  }
+
+  final override Instruction getChildSuccessor(TranslatedElement child) { none() }
+}
--- a/cpp/ql/src/semmle/code/cpp/ir/internal/CppType.qll
+++ b/cpp/ql/src/semmle/code/cpp/ir/internal/CppType.qll
@@ -21,6 +21,13 @@ private int getTypeSizeWorkaround(Type type) {
          else result = any(NullPointerType t).getSize()
        )
      )
+      or
+      exists(ArrayType arrayType |
+        // Treat `T[]` as `T*`.
+        arrayType = unspecifiedType and
+        not arrayType.hasArraySize() and
+        result = any(NullPointerType t).getSize()
+      )
    )
  )
 }
@@ -88,7 +95,11 @@ predicate hasFloatingPointType(int byteSize) {
 private predicate isPointerIshType(Type type) {
  type instanceof PointerType or
  type instanceof ReferenceType or
-  type instanceof NullPointerType
+  type instanceof NullPointerType or
+  // Treat `T[]` as a pointer. The only place we should see these is as the type of a parameter. If
+  // the corresponding decayed `T*` type is available, we'll use that, but if it's not available,
+  // we're stuck with `T[]`. Just treat it as a pointer.
+  type instanceof ArrayType and not exists(type.getSize())
 }

 /**
@@ -147,7 +158,7 @@ private IRType getIRTypeForPRValue(Type type) {
    isSignedIntegerType(unspecifiedType) and result.(IRSignedIntegerType).getByteSize() = type.getSize() or
    isUnsignedIntegerType(unspecifiedType) and result.(IRUnsignedIntegerType).getByteSize() = type.getSize() or
    unspecifiedType instanceof FloatingPointType and result.(IRFloatingPointType).getByteSize() = type.getSize() or
-    isPointerIshType(unspecifiedType) and result.(IRAddressType).getByteSize() = type.getSize() or
+    isPointerIshType(unspecifiedType) and result.(IRAddressType).getByteSize() = getTypeSize(type) or
    unspecifiedType instanceof FunctionPointerIshType and result.(IRFunctionAddressType).getByteSize() = getTypeSize(type) or
    unspecifiedType instanceof VoidType and result instanceof IRVoidType or
    unspecifiedType instanceof ErroneousType and result instanceof IRErrorType or
@@ -343,6 +354,15 @@ CppType getTypeForPRValue(Type type) {
  else result.hasType(type, false)
 }

+/**
+ * Gets the `CppType` that represents a prvalue of type `type`, if such a `CppType` exists.
+ * Otherwise, gets `CppUnknownType`.
+ */
+CppType getTypeForPRValueOrUnknown(Type type) {
+  result = getTypeForPRValue(type) or
+  not exists(getTypeForPRValue(type)) and result = getUnknownType()
+}
+
 /**
 * Gets the `CppType` that represents a glvalue of type `type`.
 */
--- a/cpp/ql/test/library-tests/syntax-zoo/aliased_ssa_sanity.expected
+++ b/cpp/ql/test/library-tests/syntax-zoo/aliased_ssa_sanity.expected
@@ -12,13 +12,12 @@ missingPhiOperand
 missingOperandType
 instructionWithoutSuccessor
 | VacuousDestructorCall.cpp:2:29:2:29 | InitializeParameter: y |
-| assume0.cpp:7:2:7:2 | Chi: call to f |
 | condition_decls.cpp:16:19:16:20 | Chi: call to BoxedInt |
 | condition_decls.cpp:26:23:26:24 | Chi: call to BoxedInt |
 | condition_decls.cpp:41:22:41:23 | Chi: call to BoxedInt |
 | condition_decls.cpp:48:52:48:53 | Chi: call to BoxedInt |
 | cpp17.cpp:15:11:15:21 | Convert: (void *)... |
-| misc.c:171:10:171:13 | VariableAddress: definition of str2 |
+| misc.c:171:10:171:13 | Uninitialized: definition of str2 |
 | misc.c:219:47:219:48 | InitializeParameter: sp |
 | ms_try_except.cpp:3:9:3:9 | Uninitialized: definition of x |
 | ms_try_mix.cpp:11:12:11:15 | Chi: call to C |
@@ -26,7 +25,7 @@ instructionWithoutSuccessor
 | ms_try_mix.cpp:48:10:48:13 | Chi: call to C |
 | pointer_to_member.cpp:36:11:36:30 | FieldAddress: {...} |
 | stmt_expr.cpp:27:5:27:15 | Store: ... = ... |
-| vla.c:5:9:5:14 | VariableAddress: definition of matrix |
+| vla.c:5:9:5:14 | Uninitialized: definition of matrix |
 | vla.c:11:6:11:16 | UnmodeledDefinition: vla_typedef |
 ambiguousSuccessors
 | allocators.cpp:14:5:14:8 | UnmodeledDefinition: main | Goto | 4 | allocators.cpp:16:8:16:10 | VariableAddress: definition of foo |
--- a/cpp/ql/test/library-tests/syntax-zoo/raw_sanity.expected
+++ b/cpp/ql/test/library-tests/syntax-zoo/raw_sanity.expected
@@ -8,6 +8,7 @@ missingOperand
 | misc.c:220:9:223:3 | FieldAddress: {...} | Instruction 'FieldAddress' is missing an expected operand with tag 'Unary' in function '$@'. | misc.c:219:5:219:26 | IR: assign_designated_init | int assign_designated_init(someStruct*) |
 | misc.c:220:9:223:3 | FieldAddress: {...} | Instruction 'FieldAddress' is missing an expected operand with tag 'Unary' in function '$@'. | misc.c:219:5:219:26 | IR: assign_designated_init | int assign_designated_init(someStruct*) |
 | pointer_to_member.cpp:36:13:36:19 | FieldAddress: x1 | Instruction 'FieldAddress' is missing an expected operand with tag 'Unary' in function '$@'. | pointer_to_member.cpp:32:6:32:14 | IR: pmIsConst | void pmIsConst() |
+| pointer_to_member.cpp:36:22:36:28 | Store: f1 | Instruction 'Store' is missing an expected operand with tag 'StoreValue' in function '$@'. | pointer_to_member.cpp:32:6:32:14 | IR: pmIsConst | void pmIsConst() |
 | range_analysis.c:368:10:368:21 | Store: ... ? ... : ... | Instruction 'Store' is missing an expected operand with tag 'StoreValue' in function '$@'. | range_analysis.c:355:14:355:27 | IR: test_ternary01 | unsigned int test_ternary01(unsigned int) |
 | range_analysis.c:369:10:369:36 | Store: ... ? ... : ... | Instruction 'Store' is missing an expected operand with tag 'StoreValue' in function '$@'. | range_analysis.c:355:14:355:27 | IR: test_ternary01 | unsigned int test_ternary01(unsigned int) |
 | range_analysis.c:370:10:370:38 | Store: ... ? ... : ... | Instruction 'Store' is missing an expected operand with tag 'StoreValue' in function '$@'. | range_analysis.c:355:14:355:27 | IR: test_ternary01 | unsigned int test_ternary01(unsigned int) |
@@ -24,8 +25,6 @@ instructionWithoutSuccessor
 | VacuousDestructorCall.cpp:2:29:2:29 | InitializeParameter: y |
 | VacuousDestructorCall.cpp:3:3:3:3 | VariableAddress: x |
 | VacuousDestructorCall.cpp:4:3:4:3 | Load: y |
-| assume0.cpp:7:2:7:2 | CallSideEffect: call to f |
-| assume0.cpp:9:11:9:11 | Constant: (bool)... |
 | condition_decls.cpp:16:19:16:20 | CallSideEffect: call to BoxedInt |
 | condition_decls.cpp:26:19:26:20 | IndirectMayWriteSideEffect: bi |
 | condition_decls.cpp:26:23:26:24 | CallSideEffect: call to BoxedInt |
@@ -35,19 +34,16 @@ instructionWithoutSuccessor
 | file://:0:0:0:0 | CompareNE: (bool)... |
 | file://:0:0:0:0 | CompareNE: (bool)... |
 | file://:0:0:0:0 | CompareNE: (bool)... |
-| misc.c:171:10:171:13 | VariableAddress: definition of str2 |
+| misc.c:171:10:171:13 | Uninitialized: definition of str2 |
 | misc.c:171:15:171:31 | Add: ... + ... |
-| misc.c:173:10:173:12 | VariableAddress: definition of buf |
 | misc.c:173:14:173:26 | Mul: ... * ... |
 | misc.c:173:37:173:39 | Store: array to pointer conversion |
-| misc.c:174:10:174:15 | VariableAddress: definition of matrix |
 | misc.c:174:17:174:22 | CallSideEffect: call to getInt |
 | misc.c:174:30:174:35 | CallSideEffect: call to getInt |
 | misc.c:174:55:174:60 | Store: (char ****)... |
 | misc.c:219:47:219:48 | InitializeParameter: sp |
 | misc.c:221:10:221:10 | Store: 1 |
 | misc.c:222:10:222:10 | Store: 2 |
-| ms_assume.cpp:20:12:20:12 | Constant: (bool)... |
 | ms_try_except.cpp:3:9:3:9 | Uninitialized: definition of x |
 | ms_try_except.cpp:7:13:7:17 | Store: ... = ... |
 | ms_try_except.cpp:9:19:9:19 | Load: j |
@@ -69,6 +65,7 @@ instructionWithoutSuccessor
 | ms_try_mix.cpp:51:5:51:11 | ThrowValue: throw ... |
 | ms_try_mix.cpp:53:13:54:3 | NoOp: { ... } |
 | pointer_to_member.cpp:36:11:36:30 | FieldAddress: {...} |
+| pointer_to_member.cpp:36:11:36:30 | FieldAddress: {...} |
 | static_init_templates.cpp:80:27:80:36 | Convert: (void *)... |
 | static_init_templates.cpp:80:27:80:36 | Convert: (void *)... |
 | static_init_templates.cpp:89:27:89:36 | Convert: (void *)... |
@@ -80,7 +77,7 @@ instructionWithoutSuccessor
 | stmt_expr.cpp:27:5:27:15 | Store: ... = ... |
 | stmt_expr.cpp:29:11:32:11 | CopyValue: (statement expression) |
 | stmt_in_type.cpp:5:53:5:53 | Constant: 1 |
-| vla.c:5:9:5:14 | VariableAddress: definition of matrix |
+| vla.c:5:9:5:14 | Uninitialized: definition of matrix |
 | vla.c:5:16:5:19 | Load: argc |
 | vla.c:5:22:5:25 | CallReadSideEffect: call to atoi |
 | vla.c:11:6:11:16 | UnmodeledDefinition: vla_typedef |
@@ -89,7 +86,6 @@ instructionWithoutSuccessor
 | vla.c:13:12:13:14 | Uninitialized: definition of var |
 | vla.c:14:36:14:47 | Add: ... + ... |
 | vla.c:14:53:14:65 | Mul: ... * ... |
-| vla.c:14:69:14:72 | VariableAddress: definition of buf2 |
 | vla.c:14:74:14:79 | CallSideEffect: call to getInt |
 | vla.c:14:92:14:94 | Store: (char *)... |
 ambiguousSuccessors
@@ -614,7 +610,6 @@ lostReachability
 backEdgeCountMismatch
 useNotDominatedByDefinition
 | VacuousDestructorCall.cpp:4:3:4:3 | Load | Operand 'Load' is not dominated by its definition in function '$@'. | VacuousDestructorCall.cpp:2:6:2:6 | IR: CallDestructor | void CallDestructor<int>(int, int*) |
-| assume0.cpp:11:2:11:2 | Operand | Operand 'Operand' is not dominated by its definition in function '$@'. | assume0.cpp:5:6:5:6 | IR: h | void h() |
 | condition_decls.cpp:16:15:16:15 | Operand | Operand 'Operand' is not dominated by its definition in function '$@'. | condition_decls.cpp:15:6:15:17 | IR: if_decl_bind | void if_decl_bind(int) |
 | condition_decls.cpp:16:15:16:16 | Load | Operand 'Load' is not dominated by its definition in function '$@'. | condition_decls.cpp:15:6:15:17 | IR: if_decl_bind | void if_decl_bind(int) |
 | condition_decls.cpp:16:15:16:16 | Operand | Operand 'Operand' is not dominated by its definition in function '$@'. | condition_decls.cpp:15:6:15:17 | IR: if_decl_bind | void if_decl_bind(int) |
@@ -683,6 +678,7 @@ useNotDominatedByDefinition
 | ms_try_mix.cpp:51:5:51:11 | Load | Operand 'Load' is not dominated by its definition in function '$@'. | ms_try_mix.cpp:47:6:47:28 | IR: ms_empty_finally_at_end | void ms_empty_finally_at_end() |
 | pointer_to_member.cpp:36:11:36:30 | Unary | Operand 'Unary' is not dominated by its definition in function '$@'. | pointer_to_member.cpp:32:6:32:14 | IR: pmIsConst | void pmIsConst() |
 | pointer_to_member.cpp:36:13:36:19 | Address | Operand 'Address' is not dominated by its definition in function '$@'. | pointer_to_member.cpp:32:6:32:14 | IR: pmIsConst | void pmIsConst() |
+| pointer_to_member.cpp:36:22:36:28 | Address | Operand 'Address' is not dominated by its definition in function '$@'. | pointer_to_member.cpp:32:6:32:14 | IR: pmIsConst | void pmIsConst() |
 | stmt_expr.cpp:30:20:30:21 | Operand | Operand 'Operand' is not dominated by its definition in function '$@'. | stmt_expr.cpp:21:6:21:6 | IR: g | void stmtexpr::g(int) |
 | stmt_expr.cpp:31:16:31:18 | Load | Operand 'Load' is not dominated by its definition in function '$@'. | stmt_expr.cpp:21:6:21:6 | IR: g | void stmtexpr::g(int) |
 | try_catch.cpp:21:13:21:24 | Address | Operand 'Address' is not dominated by its definition in function '$@'. | try_catch.cpp:19:6:19:23 | IR: throw_from_nonstmt | void throw_from_nonstmt(int) |
--- a/cpp/ql/test/library-tests/syntax-zoo/unaliased_ssa_sanity.expected
+++ b/cpp/ql/test/library-tests/syntax-zoo/unaliased_ssa_sanity.expected
@@ -21,13 +21,12 @@ missingPhiOperand
 missingOperandType
 instructionWithoutSuccessor
 | VacuousDestructorCall.cpp:2:29:2:29 | InitializeParameter: y |
-| assume0.cpp:7:2:7:2 | CallSideEffect: call to f |
 | condition_decls.cpp:16:19:16:20 | CallSideEffect: call to BoxedInt |
 | condition_decls.cpp:26:23:26:24 | CallSideEffect: call to BoxedInt |
 | condition_decls.cpp:41:22:41:23 | CallSideEffect: call to BoxedInt |
 | condition_decls.cpp:48:52:48:53 | CallSideEffect: call to BoxedInt |
 | cpp17.cpp:15:11:15:21 | Convert: (void *)... |
-| misc.c:171:10:171:13 | VariableAddress: definition of str2 |
+| misc.c:171:10:171:13 | Uninitialized: definition of str2 |
 | misc.c:219:47:219:48 | InitializeParameter: sp |
 | ms_try_except.cpp:3:9:3:9 | Uninitialized: definition of x |
 | ms_try_mix.cpp:11:12:11:15 | CallSideEffect: call to C |
@@ -35,7 +34,7 @@ instructionWithoutSuccessor
 | ms_try_mix.cpp:48:10:48:13 | CallSideEffect: call to C |
 | pointer_to_member.cpp:36:11:36:30 | FieldAddress: {...} |
 | stmt_expr.cpp:27:5:27:15 | Store: ... = ... |
-| vla.c:5:9:5:14 | VariableAddress: definition of matrix |
+| vla.c:5:9:5:14 | Uninitialized: definition of matrix |
 | vla.c:11:6:11:16 | UnmodeledDefinition: vla_typedef |
 ambiguousSuccessors
 | allocators.cpp:14:5:14:8 | UnmodeledDefinition: main | Goto | 4 | allocators.cpp:16:8:16:10 | VariableAddress: definition of foo |
--- a/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/gvn/ValueNumbering.qll
+++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/gvn/ValueNumbering.qll
@@ -136,14 +136,14 @@ private predicate variableAddressValueNumber(
  VariableAddressInstruction instr, IRFunction irFunc, IRVariable var
 ) {
  instr.getEnclosingIRFunction() = irFunc and
-  instr.getVariable() = var
+  instr.getIRVariable() = var
 }

 private predicate initializeParameterValueNumber(
  InitializeParameterInstruction instr, IRFunction irFunc, IRVariable var
 ) {
  instr.getEnclosingIRFunction() = irFunc and
-  instr.getVariable() = var
+  instr.getIRVariable() = var
 }

 private predicate initializeThisValueNumber(InitializeThisInstruction instr, IRFunction irFunc) {