@@ -18,35 +18,64 @@
 import cpp
 import codingstandards.c.cert
 import codingstandards.cpp.Macro
-import semmle.code.cpp.dataflow.DataFlow
+import semmle.code.cpp.dataflow.new.DataFlow
+import semmle.code.cpp.ir.IR as IR
 
-abstract class VaAccess extends Expr { }
+abstract class VaAccess extends VariableAccess {
+  abstract DataFlow::Node getDfn();
+}
 
 /**
  * The argument of a call to `va_arg`
  */
 class VaArgArg extends VaAccess {
+  IR::NextVarArgInstruction nva;
+
   VaArgArg() { this = any(MacroInvocation m | m.getMacroName() = ["va_arg"]).getExpr().getChild(0) }
+
+  override DataFlow::Node getDfn() {
+    // Simply using `DataFlow::exprNode(this)` will not correctly find the IR nodes for this
+    // `va_arg` usage, so we have to dig into the IR here ourselves to properly wire things up.
+    //
+    // The IR for a `va_arg(p)` looks as follows:
+    //
+    // rx_n = VariableAddress[p]
+    // ...
+    // ry_0 = Load[p] : &:rx_n
+    // ry_1 = Load[?] : &:ry_n
+    // ry_2 = NextVarArg : ry_1
+    //
+    // The last occurrence of `va_list` that we have dataflow to is `ry_0` via an `OperandNode`,
+    // and the simplest attachment between the AST and the IR is through `ry_2` and our parent AST
+    // node, the `__builtin_vararg(...)` call.
+    exists(IR::Operand ry0, IR::Instruction ry1, IR::NextVarArgInstruction ry2 |
+      ry2.getAnOperand().getDef() = ry1 and
+      ry2.getAst() = this.getParent() and
+      ry1.getAnOperand() = ry0 and
+      result.(DataFlow::OperandNode).getOperand() = ry0
+    )
+  }
 }
 
 /**
  * The argument of a call to `va_end`
  */
 class VaEndArg extends VaAccess {
   VaEndArg() { this = any(MacroInvocation m | m.getMacroName() = ["va_end"]).getExpr().getChild(0) }
+
+  override DataFlow::Node getDfn() { result.asExpr() = this }
 }
 
 /**
- * Dataflow configuration for flow from a library function
- * to a call of function `asctime`
+ * Dataflow configuration for flow from between `va_list` usages.
  */
 module VaArgConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node src) {
     src.asUninitialized() =
       any(VariableDeclarationEntry m | m.getType().hasName("va_list")).getVariable()
   }
 
-  predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof VaAccess }
+  predicate isSink(DataFlow::Node sink) { exists(VaAccess va_acc | sink = va_acc.getDfn()) }
 }
 
 module VaArgFlow = DataFlow::Global<VaArgConfig>;
@@ -64,15 +93,15 @@ ControlFlowNode preceedsFC(VaAccess va_arg) {
     not result =
       any(MacroInvocation m |
         m.getMacroName() = ["va_start"] and
-        m.getExpr().getChild(0).(VariableAccess).getTarget() = va_arg.(VariableAccess).getTarget()
+        m.getExpr().getChild(0).(VariableAccess).getTarget() = va_arg.getTarget()
       ).getExpr()
   )
 }
 
 predicate sameSource(VaAccess e1, VaAccess e2) {
   exists(DataFlow::Node source |
-    VaArgFlow::flow(source, DataFlow::exprNode(e1)) and
-    VaArgFlow::flow(source, DataFlow::exprNode(e2))
+    VaArgFlow::flow(source, e1.getDfn()) and
+    VaArgFlow::flow(source, e2.getDfn())
   )
 }
 
 
@@ -0,0 +1,2 @@
+ - `MSC39-C` - `DoNotCallVaArgOnAVaListThatHasAnIndeterminateValue.ql`:
+   - Updated to use the new ir-based `DataFlow` module for tracking `va_list` usage. The new data flow will provide a different set of false positives and negatives compared to the previous implementation, but generally has higher precision.
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	+ - `MSC39-C` - `DoNotCallVaArgOnAVaListThatHasAnIndeterminateValue.ql`:
	`2`	+ - Updated to use the new ir-based `DataFlow` module for tracking `va_list` usage. The new data flow will provide a different set of false positives and negatives compared to the previous implementation, but generally has higher precision.