Merge branch 'master' of github.com:Semmle/ql into rdmarsh/cpp/uninit-string-initializers

cpp/ql/src/Security/CWE/CWE-457/ConditionallyUninitializedVariableBad.c

@@ -19,7 +19,7 @@ int notify(int deviceNumber) {
   DeviceConfig config;
   initDeviceConfig(&config, deviceNumber);
   // BAD: Using config without checking the status code that is returned
-  if (config->isEnabled) {
-    notifyChannel(config->channel);
+  if (config.isEnabled) {
+    notifyChannel(config.channel);
   }
-}
+}

cpp/ql/src/Security/CWE/CWE-457/ConditionallyUninitializedVariableGood.c

@@ -20,8 +20,8 @@ void notify(int deviceNumber) {
   int statusCode = initDeviceConfig(&config, deviceNumber);
   if (statusCode == 0) {
     // GOOD: Status code returned by initialization function is checked, so this is safe
-    if (config->isEnabled) {
-      notifyChannel(config->channel);
+    if (config.isEnabled) {
+      notifyChannel(config.channel);
     }
   }
-}
+}

cpp/ql/src/Security/CWE/CWE-457/InitializationFunctions.qll

@@ -620,32 +620,47 @@ Function getAPossibleDefinition(Function undefinedFunction) {
 }
 
 /**
- * Gets a possible target for the Call, using the name and parameter matching if we did not associate
+ * Helper predicate for `getTarget`, that computes possible targets of a `Call`.
+ *
+ * If there is at least one defined target after performing some simple virtual dispatch
+ * resolution, then the result is all the defined targets.
+ */
+private Function getTarget1(Call c) {
+  result = VirtualDispatch::getAViableTarget(c) and
+  result.isDefined()
+}
+
+/**
+ * Helper predicate for `getTarget`, that computes possible targets of a `Call`.
+ *
+ * If we can use the heuristic matching of functions to find definitions for some of the viable
+ * targets, return those.
+ */
+private Function getTarget2(Call c) {
+  not exists(getTarget1(c)) and
+  result = getAPossibleDefinition(VirtualDispatch::getAViableTarget(c))
+}
+
+/**
+ * Helper predicate for `getTarget`, that computes possible targets of a `Call`.
+ *
+ * Otherwise, the result is the undefined `Function` instances.
+ */
+private Function getTarget3(Call c) {
+  not exists(getTarget1(c)) and
+  not exists(getTarget2(c)) and
+  result = VirtualDispatch::getAViableTarget(c)
+}
+
+/**
+ * Gets a possible target for the `Call`, using the name and parameter matching if we did not associate
  * this call with a specific definition at link or compile time, and performing simple virtual
  * dispatch resolution.
  */
 Function getTarget(Call c) {
-  if VirtualDispatch::getAViableTarget(c).isDefined()
-  then
-    /*
-     * If there is at least one defined target after performing some simple virtual dispatch
-     * resolution, then the result is all the defined targets.
-     */
-
-    result = VirtualDispatch::getAViableTarget(c) and
-    result.isDefined()
-  else
-    if exists(getAPossibleDefinition(VirtualDispatch::getAViableTarget(c)))
-    then
-      /*
-       * If we can use the heuristic matching of functions to find definitions for some of the viable
-       * targets, return those.
-       */
-
-      result = getAPossibleDefinition(VirtualDispatch::getAViableTarget(c))
-    else
-      // Otherwise, the result is the undefined `Function` instances
-      result = VirtualDispatch::getAViableTarget(c)
+  result = getTarget1(c) or
+  result = getTarget2(c) or
+  result = getTarget3(c)
 }
 
 /**

cpp/ql/src/semmle/code/cpp/Class.qll

@@ -605,15 +605,6 @@ class Class extends UserType {
     class_instantiation(underlyingElement(this), unresolveElement(c))
   }
 
-  /**
-   * Gets the `i`th template argument used to instantiate this class from a
-   * class template. When called on a class template, this will return the
-   * `i`th template parameter.
-   */
-  override Type getTemplateArgument(int i) {
-    class_template_argument(underlyingElement(this), i, unresolveElement(result))
-  }
-
   /**
    * Holds if this class/struct is polymorphic (has a virtual function, or
    * inherits one).
@@ -623,7 +614,7 @@ class Class extends UserType {
   }
 
   override predicate involvesTemplateParameter() {
-    getATemplateArgument().involvesTemplateParameter()
+    getATemplateArgument().(Type).involvesTemplateParameter()
   }
 
   /** Holds if this class, struct or union was declared 'final'. */

cpp/ql/src/semmle/code/cpp/Declaration.qll

@@ -193,20 +193,83 @@ abstract class Declaration extends Locatable, @declaration {
 
   /**
    * Gets a template argument used to instantiate this declaration from a template.
-   * When called on a template, this will return a template parameter.
+   * When called on a template, this will return a template parameter type for
+   * both typed and non-typed parameters.
    */
-  final Type getATemplateArgument() { result = getTemplateArgument(_) }
+  final Locatable getATemplateArgument() { result = getTemplateArgument(_) }
+
+  /**
+   * Gets a template argument used to instantiate this declaration from a template.
+   * When called on a template, this will return a non-typed template
+   * parameter value.
+   */
+  final Locatable getATemplateArgumentKind() { result = getTemplateArgumentKind(_) }
 
   /**
    * Gets the `i`th template argument used to instantiate this declaration from a
-   * template. When called on a template, this will return the `i`th template parameter.
+   * template.
+   *
+   * For example:
+   *
+   * `template<typename T, T X> class Foo;`
+   *
+   * Will have `getTemplateArgument(0)` return `T`, and
+   * `getTemplateArgument(1)` return `X`.
+   *
+   * `Foo<int, 1> bar;
+   *
+   * Will have `getTemplateArgument())` return `int`, and
+   * `getTemplateArgument(1)` return `1`.
    */
-  Type getTemplateArgument(int index) { none() }
+  final Locatable getTemplateArgument(int index) {
+    if exists(getTemplateArgumentValue(index))
+    then result = getTemplateArgumentValue(index)
+    else result = getTemplateArgumentType(index)
+  }
+
+  /**
+   * Gets the `i`th template argument value used to instantiate this declaration
+   * from a template. When called on a template, this will return the `i`th template
+   * parameter value if it exists.
+   *
+   * For example:
+   *
+   * `template<typename T, T X> class Foo;`
+   *
+   * Will have `getTemplateArgumentKind(1)` return `T`, and no result for
+   * `getTemplateArgumentKind(0)`.
+   *
+   * `Foo<int, 10> bar;
+   *
+   * Will have `getTemplateArgumentKind(1)` return `int`, and no result for
+   * `getTemplateArgumentKind(0)`.
+   */
+  final Locatable getTemplateArgumentKind(int index) {
+    if exists(getTemplateArgumentValue(index))
+    then result = getTemplateArgumentType(index)
+    else none()
+  }
 
   /** Gets the number of template arguments for this declaration. */
   final int getNumberOfTemplateArguments() {
     result = count(int i | exists(getTemplateArgument(i)))
   }
+
+  private Type getTemplateArgumentType(int index) {
+    class_template_argument(underlyingElement(this), index, unresolveElement(result))
+    or
+    function_template_argument(underlyingElement(this), index, unresolveElement(result))
+    or
+    variable_template_argument(underlyingElement(this), index, unresolveElement(result))
+  }
+
+  private Expr getTemplateArgumentValue(int index) {
+    class_template_argument_value(underlyingElement(this), index, unresolveElement(result))
+    or
+    function_template_argument_value(underlyingElement(this), index, unresolveElement(result))
+    or
+    variable_template_argument_value(underlyingElement(this), index, unresolveElement(result))
+  }
 }
 
 /**

cpp/ql/src/semmle/code/cpp/Function.qll

@@ -343,15 +343,6 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
     function_instantiation(underlyingElement(this), unresolveElement(f))
   }
 
-  /**
-   * Gets the `i`th template argument used to instantiate this function from a
-   * function template. When called on a function template, this will return the
-   * `i`th template parameter.
-   */
-  override Type getTemplateArgument(int index) {
-    function_template_argument(underlyingElement(this), index, unresolveElement(result))
-  }
-
   /**
    * Holds if this function is defined in several files. This is illegal in
    * C (though possible in some C++ compilers), and likely indicates that

cpp/ql/src/semmle/code/cpp/Print.qll

@@ -35,6 +35,14 @@ private string getParameterTypeString(Type parameterType) {
   else result = parameterType.(DumpType).getTypeIdentityString()
 }
 
+private string getTemplateArgumentString(Declaration d, int i) {
+  if exists(d.getTemplateArgumentKind(i))
+  then
+    result = d.getTemplateArgumentKind(i).(DumpType).getTypeIdentityString() + " " +
+        d.getTemplateArgument(i)
+  else result = d.getTemplateArgument(i).(DumpType).getTypeIdentityString()
+}
+
 /**
  * A `Declaration` extended to add methods for generating strings useful only for dumps and debugging.
  */
@@ -56,7 +64,7 @@ abstract private class DumpDeclaration extends Declaration {
           strictconcat(int i |
             exists(this.getTemplateArgument(i))
           |
-            this.getTemplateArgument(i).(DumpType).getTypeIdentityString(), ", " order by i
+            getTemplateArgumentString(this, i), ", " order by i
           ) + ">"
     else result = ""
   }

cpp/ql/src/semmle/code/cpp/Type.qll

@@ -210,7 +210,7 @@ class Type extends Locatable, @type {
     // A function call that provides an explicit template argument that refers to T uses T.
     // We exclude calls within instantiations, since they do not appear directly in the source.
     exists(FunctionCall c |
-      c.getAnExplicitTemplateArgument().refersTo(this) and
+      c.getAnExplicitTemplateArgument().(Type).refersTo(this) and
       result = c and
       not c.getEnclosingFunction().isConstructedFrom(_)
     )

cpp/ql/src/semmle/code/cpp/Variable.qll

@@ -155,15 +155,6 @@ class Variable extends Declaration, @variable {
     variable_instantiation(underlyingElement(this), unresolveElement(v))
   }
 
-  /**
-   * Gets the `i`th template argument used to instantiate this variable from a
-   * variable template. When called on a variable template, this will return the
-   * `i`th template parameter.
-   */
-  override Type getTemplateArgument(int index) {
-    variable_template_argument(underlyingElement(this), index, unresolveElement(result))
-  }
-
   /**
    * Holds if this is a compiler-generated variable. For example, a
    * [range-based for loop](http://en.cppreference.com/w/cpp/language/range-for)

cpp/ql/src/semmle/code/cpp/dispatch/VirtualDispatch.qll

@@ -28,6 +28,19 @@ module VirtualDispatch {
     not result.hasName("IUnknown")
   }
 
+  /**
+   * Helper predicate for `getAViableTarget`, which computes the viable targets for
+   * virtual calls based on the qualifier type.
+   */
+  private Function getAViableVirtualCallTarget(Class qualifierType, MemberFunction staticTarget) {
+    exists(Class qualifierSubType |
+      result = getAPossibleImplementation(staticTarget) and
+      qualifierType = qualifierSubType.getABaseClass*() and
+      mayInherit(qualifierSubType, result) and
+      not cannotInherit(qualifierSubType, result)
+    )
+  }
+
   /**
    * Gets a viable target for the given function call.
    *
@@ -42,18 +55,9 @@ module VirtualDispatch {
    * If `c` is not a virtual call, the result will be `c.getTarget()`.
    */
   Function getAViableTarget(Call c) {
-    exists(Function staticTarget | staticTarget = c.getTarget() |
-      if c.(FunctionCall).isVirtual() and staticTarget instanceof MemberFunction
-      then
-        exists(Class qualifierType, Class qualifierSubType |
-          result = getAPossibleImplementation(staticTarget) and
-          qualifierType = getCallQualifierType(c) and
-          qualifierType = qualifierSubType.getABaseClass*() and
-          mayInherit(qualifierSubType, result) and
-          not cannotInherit(qualifierSubType, result)
-        )
-      else result = staticTarget
-    )
+    if c.(FunctionCall).isVirtual() and c.getTarget() instanceof MemberFunction
+    then result = getAViableVirtualCallTarget(getCallQualifierType(c), c.getTarget())
+    else result = c.getTarget()
   }
 
   /** Holds if `f` is declared in `c` or a transitive base class of `c`. */

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

@@ -139,17 +139,29 @@ class FunctionCall extends Call, @funbindexpr {
   override string getCanonicalQLClass() { result = "FunctionCall" }
 
   /** Gets an explicit template argument for this call. */
-  Type getAnExplicitTemplateArgument() { result = getExplicitTemplateArgument(_) }
+  Locatable getAnExplicitTemplateArgument() { result = getExplicitTemplateArgument(_) }
+
+  /** Gets an explicit template argument value for this call. */
+  Locatable getAnExplicitTemplateArgumentKind() { result = getExplicitTemplateArgumentKind(_) }
 
   /** Gets a template argument for this call. */
-  Type getATemplateArgument() { result = getTarget().getATemplateArgument() }
+  Locatable getATemplateArgument() { result = getTarget().getATemplateArgument() }
+
+  /** Gets a template argument value for this call. */
+  Locatable getATemplateArgumentKind() { result = getTarget().getATemplateArgumentKind() }
 
   /** Gets the nth explicit template argument for this call. */
-  Type getExplicitTemplateArgument(int n) {
+  Locatable getExplicitTemplateArgument(int n) {
     n < getNumberOfExplicitTemplateArguments() and
     result = getTemplateArgument(n)
   }
 
+  /** Gets the nth explicit template argument value for this call. */
+  Locatable getExplicitTemplateArgumentKind(int n) {
+    n < getNumberOfExplicitTemplateArguments() and
+    result = getTemplateArgumentKind(n)
+  }
+
   /** Gets the number of explicit template arguments for this call. */
   int getNumberOfExplicitTemplateArguments() {
     if numtemplatearguments(underlyingElement(this), _)
@@ -161,7 +173,10 @@ class FunctionCall extends Call, @funbindexpr {
   int getNumberOfTemplateArguments() { result = count(int i | exists(getTemplateArgument(i))) }
 
   /** Gets the nth template argument for this call (indexed from 0). */
-  Type getTemplateArgument(int n) { result = getTarget().getTemplateArgument(n) }
+  Locatable getTemplateArgument(int n) { result = getTarget().getTemplateArgument(n) }
+
+  /** Gets the nth template argument value for this call (indexed from 0). */
+  Locatable getTemplateArgumentKind(int n) { result = getTarget().getTemplateArgumentKind(n) }
 
   /** Holds if any template arguments for this call are implicit / deduced. */
   predicate hasImplicitTemplateArguments() {

cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll

@@ -2,6 +2,7 @@ import cpp
 import semmle.code.cpp.security.Security
 private import semmle.code.cpp.ir.dataflow.DataFlow
 private import semmle.code.cpp.ir.IR
+private import semmle.code.cpp.ir.dataflow.internal.DataFlowDispatch as Dispatch
 
 /**
  * A predictable instruction is one where an external user can predict
@@ -145,7 +146,8 @@ GlobalOrNamespaceVariable globalVarFromId(string id) {
 }
 
 Function resolveCall(Call call) {
-  // TODO: improve virtual dispatch. This will help in the test for
-  // `UncontrolledProcessOperation.ql`.
-  result = call.getTarget()
+  exists(CallInstruction callInstruction |
+    callInstruction.getAST() = call and
+    result = Dispatch::viableCallable(callInstruction)
+  )
 }

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

@@ -1,5 +1,6 @@
 private import cpp
 private import semmle.code.cpp.ir.IR
+private import semmle.code.cpp.ir.dataflow.DataFlow
 
 Function viableImpl(CallInstruction call) { result = viableCallable(call) }
 
@@ -20,6 +21,58 @@ Function viableCallable(CallInstruction call) {
     functionSignatureWithBody(qualifiedName, nparams, result) and
     strictcount(Function other | functionSignatureWithBody(qualifiedName, nparams, other)) = 1
   )
+  or
+  // Rudimentary virtual dispatch support. It's essentially local data flow
+  // where the source is a derived-to-base conversion and the target is the
+  // qualifier of a call.
+  exists(Class derived, DataFlow::Node thisArgument |
+    nodeMayHaveClass(derived, thisArgument) and
+    overrideMayAffectCall(derived, thisArgument, _, result, call)
+  )
+}
+
+/**
+ * Holds if `call` is a virtual function call with qualifier `thisArgument` in
+ * `enclosingFunction`, whose static target is overridden by
+ * `overridingFunction` in `overridingClass`.
+ */
+pragma[noinline]
+private predicate overrideMayAffectCall(
+  Class overridingClass, DataFlow::Node thisArgument, Function enclosingFunction,
+  MemberFunction overridingFunction, CallInstruction call
+) {
+  call.getEnclosingFunction() = enclosingFunction and
+  overridingFunction.getAnOverriddenFunction+() = call.getStaticCallTarget() and
+  overridingFunction.getDeclaringType() = overridingClass and
+  thisArgument = DataFlow::instructionNode(call.getThisArgument())
+}
+
+/**
+ * Holds if `node` may have dynamic class `derived`, where `derived` is a class
+ * that may affect virtual dispatch within the enclosing function.
+ *
+ * For the sake of performance, this recursion is written out manually to make
+ * it a relation on `Class x Node` rather than `Node x Node` or `MemberFunction
+ * x Node`, both of which would be larger. It's a forward search since there
+ * should usually be fewer classes than calls.
+ *
+ * If a value is cast several classes up in the hierarchy, that will be modeled
+ * as a chain of `ConvertToBaseInstruction`s and will cause the search to start
+ * from each of them and pass through subsequent ones. There might be
+ * performance to gain by stopping before a second upcast and reconstructing
+ * the full chain in a "big-step" recursion after this one.
+ */
+private predicate nodeMayHaveClass(Class derived, DataFlow::Node node) {
+  exists(ConvertToBaseInstruction toBase |
+    derived = toBase.getDerivedClass() and
+    overrideMayAffectCall(derived, _, toBase.getEnclosingFunction(), _, _) and
+    node.asInstruction() = toBase
+  )
+  or
+  exists(DataFlow::Node prev |
+    nodeMayHaveClass(derived, prev) and
+    DataFlow::localFlowStep(prev, node)
+  )
 }
 
 /**

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

@@ -205,7 +205,8 @@ private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction
   iTo.(CopyInstruction).getSourceValue() = iFrom or
   iTo.(PhiInstruction).getAnOperand().getDef() = iFrom or
   // Treat all conversions as flow, even conversions between different numeric types.
-  iTo.(ConvertInstruction).getUnary() = iFrom
+  iTo.(ConvertInstruction).getUnary() = iFrom or
+  iTo.(InheritanceConversionInstruction).getUnary() = iFrom
 }
 
 /**

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

@@ -4,6 +4,7 @@
 
 private import internal.IRTypeInternal
 
+cached
 private newtype TIRType =
   TIRVoidType() or
   TIRUnknownType() or
@@ -42,6 +43,10 @@ class IRType extends TIRType {
    *
    * This will hold for all `IRType` objects except `IRUnknownType`.
    */
+  // This predicate is overridden with `pragma[noinline]` in every leaf subclass.
+  // This allows callers to ask for things like _the_ floating-point type of
+  // size 4 without getting a join that first finds all types of size 4 and
+  // _then_ restricts them to floating-point types.
   int getByteSize() { none() }
 
   /**
@@ -104,8 +109,6 @@ private class IRSizedType extends IRType {
     this = TIRFunctionAddressType(byteSize) or
     this = TIROpaqueType(_, byteSize)
   }
-
-  final override int getByteSize() { result = byteSize }
 }
 
 /**
@@ -117,6 +120,9 @@ class IRBooleanType extends IRSizedType, TIRBooleanType {
   final override Language::LanguageType getCanonicalLanguageType() {
     result = Language::getCanonicalBooleanType(byteSize)
   }
+
+  pragma[noinline]
+  final override int getByteSize() { result = byteSize }
 }
 
 /**
@@ -141,6 +147,9 @@ class IRSignedIntegerType extends IRNumericType, TIRSignedIntegerType {
   final override Language::LanguageType getCanonicalLanguageType() {
     result = Language::getCanonicalSignedIntegerType(byteSize)
   }
+
+  pragma[noinline]
+  final override int getByteSize() { result = byteSize }
 }
 
 /**
@@ -153,6 +162,9 @@ class IRUnsignedIntegerType extends IRNumericType, TIRUnsignedIntegerType {
   final override Language::LanguageType getCanonicalLanguageType() {
     result = Language::getCanonicalUnsignedIntegerType(byteSize)
   }
+
+  pragma[noinline]
+  final override int getByteSize() { result = byteSize }
 }
 
 /**
@@ -164,6 +176,9 @@ class IRFloatingPointType extends IRNumericType, TIRFloatingPointType {
   final override Language::LanguageType getCanonicalLanguageType() {
     result = Language::getCanonicalFloatingPointType(byteSize)
   }
+
+  pragma[noinline]
+  final override int getByteSize() { result = byteSize }
 }
 
 /**
@@ -178,6 +193,9 @@ class IRAddressType extends IRSizedType, TIRAddressType {
   final override Language::LanguageType getCanonicalLanguageType() {
     result = Language::getCanonicalAddressType(byteSize)
   }
+
+  pragma[noinline]
+  final override int getByteSize() { result = byteSize }
 }
 
 /**
@@ -190,6 +208,9 @@ class IRFunctionAddressType extends IRSizedType, TIRFunctionAddressType {
   final override Language::LanguageType getCanonicalLanguageType() {
     result = Language::getCanonicalFunctionAddressType(byteSize)
   }
+
+  pragma[noinline]
+  final override int getByteSize() { result = byteSize }
 }
 
 /**
@@ -218,6 +239,9 @@ class IROpaqueType extends IRSizedType, TIROpaqueType {
    * same size.
    */
   final Language::OpaqueTypeTag getTag() { result = tag }
+
+  pragma[noinline]
+  final override int getByteSize() { result = byteSize }
 }
 
 module IRTypeSanity {

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll

@@ -51,20 +51,23 @@ private module Cached {
   Expr getInstructionConvertedResultExpression(Instruction instruction) {
     exists(TranslatedExpr translatedExpr |
       translatedExpr = getTranslatedExpr(result) and
-      instruction = translatedExpr.getResult()
+      instruction = translatedExpr.getResult() and
+      // Only associate `instruction` with this expression if the translated
+      // expression actually produced the instruction; not if it merely
+      // forwarded the result of another translated expression.
+      instruction = translatedExpr.getInstruction(_)
     )
   }
 
   cached
   Expr getInstructionUnconvertedResultExpression(Instruction instruction) {
-    exists(Expr converted, TranslatedExpr translatedExpr |
+    exists(Expr converted |
       result = converted.(Conversion).getExpr+()
       or
       result = converted
     |
       not result instanceof Conversion and
-      translatedExpr = getTranslatedExpr(converted) and
-      instruction = translatedExpr.getResult()
+      converted = getInstructionConvertedResultExpression(instruction)
     )
   }
 

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/InstructionTag.qll

@@ -45,6 +45,7 @@ newtype TInstructionTag =
   ConditionValueResultLoadTag() or
   BoolConversionConstantTag() or
   BoolConversionCompareTag() or
+  ResultCopyTag() or
   LoadTag() or // Implicit load due to lvalue-to-rvalue conversion
   CatchTag() or
   ThrowTag() or

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll

@@ -9,6 +9,7 @@ private import InstructionTag
 private import TranslatedCondition
 private import TranslatedFunction
 private import TranslatedStmt
+private import TranslatedExpr
 private import IRConstruction
 private import semmle.code.cpp.models.interfaces.SideEffect
 
@@ -235,6 +236,15 @@ newtype TTranslatedElement =
     expr.hasLValueToRValueConversion() and
     not ignoreLoad(expr)
   } or
+  TTranslatedResultCopy(Expr expr) {
+    not ignoreExpr(expr) and
+    exprNeedsCopyIfNotLoaded(expr) and
+    // Doesn't have a TTranslatedLoad
+    not (
+      expr.hasLValueToRValueConversion() and
+      not ignoreLoad(expr)
+    )
+  } or
   // An expression most naturally translated as control flow.
   TTranslatedNativeCondition(Expr expr) {
     not ignoreExpr(expr) and

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll

@@ -62,12 +62,11 @@ abstract class TranslatedExpr extends TranslatedElement {
   /**
    * Holds if the result of this `TranslatedExpr` is a glvalue.
    */
-  private predicate isResultGLValue() {
+  predicate isResultGLValue() {
+    // This implementation is overridden in `TranslatedCoreExpr` to mark them
+    // as glvalues if they have loads on them. It's not overridden in
+    // `TranslatedResultCopy` since result copies never have loads.
     expr.isGLValueCategory()
-    or
-    // If this TranslatedExpr doesn't produce the result, then it must represent
-    // a glvalue that is then loaded by a TranslatedLoad.
-    not producesExprResult()
   }
 
   final override Locatable getAST() { result = expr }
@@ -96,14 +95,28 @@ abstract class TranslatedExpr extends TranslatedElement {
 abstract class TranslatedCoreExpr extends TranslatedExpr {
   final override string toString() { result = expr.toString() }
 
+  /**
+   * Holds if the result of this `TranslatedExpr` is a glvalue.
+   */
+  override predicate isResultGLValue() {
+    super.isResultGLValue()
+    or
+    // If this TranslatedExpr doesn't produce the result, then it must represent
+    // a glvalue that is then loaded by a TranslatedLoad.
+    hasLoad()
+  }
+
+  final predicate hasLoad() {
+    expr.hasLValueToRValueConversion() and
+    not ignoreLoad(expr)
+  }
+
   final override predicate producesExprResult() {
     // If there's no load, then this is the only TranslatedExpr for this
     // expression.
-    not expr.hasLValueToRValueConversion()
-    or
-    // If we're supposed to ignore the load on this expression, then this
-    // is the only TranslatedExpr.
-    ignoreLoad(expr)
+    not hasLoad() and
+    // If there's a result copy, then this expression's result is the copy.
+    not exprNeedsCopyIfNotLoaded(expr)
   }
 }
 
@@ -288,6 +301,48 @@ class TranslatedLoad extends TranslatedExpr, TTranslatedLoad {
   private TranslatedCoreExpr getOperand() { result.getExpr() = expr }
 }
 
+/**
+ * IR translation of an implicit lvalue-to-rvalue conversion on the result of
+ * an expression.
+ */
+class TranslatedResultCopy extends TranslatedExpr, TTranslatedResultCopy {
+  TranslatedResultCopy() { this = TTranslatedResultCopy(expr) }
+
+  override string toString() { result = "Result of " + expr.toString() }
+
+  override Instruction getFirstInstruction() { result = getOperand().getFirstInstruction() }
+
+  override TranslatedElement getChild(int id) { id = 0 and result = getOperand() }
+
+  override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
+    tag = ResultCopyTag() and
+    opcode instanceof Opcode::CopyValue and
+    resultType = getOperand().getResultType()
+  }
+
+  override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
+    tag = ResultCopyTag() and
+    result = getParent().getChildSuccessor(this) and
+    kind instanceof GotoEdge
+  }
+
+  override Instruction getChildSuccessor(TranslatedElement child) {
+    child = getOperand() and result = getInstruction(ResultCopyTag())
+  }
+
+  override Instruction getResult() { result = getInstruction(ResultCopyTag()) }
+
+  override Instruction getInstructionOperand(InstructionTag tag, OperandTag operandTag) {
+    tag = ResultCopyTag() and
+    operandTag instanceof UnaryOperandTag and
+    result = getOperand().getResult()
+  }
+
+  final override predicate producesExprResult() { any() }
+
+  private TranslatedCoreExpr getOperand() { result.getExpr() = expr }
+}
+
 class TranslatedCommaExpr extends TranslatedNonConstantExpr {
   override CommaExpr expr;
 
@@ -2403,6 +2458,58 @@ class TranslatedErrorExpr extends TranslatedSingleInstructionExpr {
   final override Opcode getOpcode() { result instanceof Opcode::Error }
 }
 
+/**
+ * Holds if the translation of `expr` will not directly generate any
+ * `Instruction` for use as result. For such instructions we can synthesize a
+ * `CopyValue` instruction to ensure that there is a 1-to-1 mapping between
+ * expressions and result-bearing instructions.
+ */
+// This should ideally be a dispatch predicate on TranslatedNonConstantExpr,
+// but it doesn't look monotonic to QL.
+predicate exprNeedsCopyIfNotLoaded(Expr expr) {
+  (
+    expr instanceof AssignExpr
+    or
+    expr instanceof AssignOperation and
+    not expr.isPRValueCategory() // is C++
+    or
+    expr instanceof PrefixCrementOperation and
+    not expr.isPRValueCategory() // is C++
+    or
+    expr instanceof PointerDereferenceExpr
+    or
+    expr instanceof AddressOfExpr
+    or
+    expr instanceof BuiltInOperationBuiltInAddressOf
+    or
+    // No case for ParenthesisExpr to avoid getting too many instructions
+    expr instanceof ReferenceDereferenceExpr
+    or
+    expr instanceof ReferenceToExpr
+    or
+    expr instanceof CommaExpr
+    or
+    expr instanceof ConditionDeclExpr
+    // TODO: simplify TranslatedStmtExpr too
+  ) and
+  not exprImmediatelyDiscarded(expr)
+}
+
+/**
+ * Holds if `expr` is immediately discarded. Such expressions do not need a
+ * `CopyValue` because it's unlikely that anyone is interested in their value.
+ */
+private predicate exprImmediatelyDiscarded(Expr expr) {
+  exists(ExprStmt s |
+    s = expr.getParent() and
+    not exists(StmtExpr se | s = se.getStmt().(Block).getLastStmt())
+  )
+  or
+  exists(CommaExpr c | c.getLeftOperand() = expr)
+  or
+  exists(ForStmt for | for.getUpdate() = expr)
+}
+
 /**
  * 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

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

@@ -175,6 +175,7 @@ private IRType getIRTypeForPRValue(Type type) {
   )
 }
 
+cached
 private newtype TCppType =
   TPRValueType(Type type) { exists(getIRTypeForPRValue(type)) } or
   TFunctionGLValueType() or
@@ -203,6 +204,7 @@ class CppType extends TCppType {
    * Gets the `IRType` that represents this `CppType`. Many different `CppType`s can map to a single
    * `IRType`.
    */
+  cached
   IRType getIRType() { none() }
 
   /**

cpp/ql/src/semmlecode.cpp.dbscheme

@@ -731,6 +731,11 @@ class_template_argument(
     int index: int ref,
     int arg_type: @type ref
 );
+class_template_argument_value(
+    int type_id: @usertype ref,
+    int index: int ref,
+    int arg_value: @expr ref
+);
 
 is_proxy_class_for(
     unique int id: @usertype ref,
@@ -755,6 +760,11 @@ function_template_argument(
     int index: int ref,
     int arg_type: @type ref
 );
+function_template_argument_value(
+    int function_id: @function ref,
+    int index: int ref,
+    int arg_value: @expr ref
+);
 
 is_variable_template(unique int id: @variable ref);
 variable_instantiation(
@@ -766,6 +776,11 @@ variable_template_argument(
     int index: int ref,
     int arg_type: @type ref
 );
+variable_template_argument_value(
+    int variable_id: @variable ref,
+    int index: int ref,
+    int arg_value: @expr ref
+);
 
 /*
   Fixed point types