diff --git a/c/cert/src/rules/MSC39-C/DoNotCallVaArgOnAVaListThatHasAnIndeterminateValue.ql b/c/cert/src/rules/MSC39-C/DoNotCallVaArgOnAVaListThatHasAnIndeterminateValue.ql index a14aa75cfc..56613c1943 100644 --- a/c/cert/src/rules/MSC39-C/DoNotCallVaArgOnAVaListThatHasAnIndeterminateValue.ql +++ b/c/cert/src/rules/MSC39-C/DoNotCallVaArgOnAVaListThatHasAnIndeterminateValue.ql @@ -18,43 +18,15 @@ import cpp import codingstandards.c.cert import codingstandards.cpp.Macro -import semmle.code.cpp.dataflow.new.DataFlow -import semmle.code.cpp.ir.IR as IR +import semmle.code.cpp.dataflow.DataFlow -abstract class VaAccess extends VariableAccess { - abstract DataFlow::Node getDfn(); -} +abstract class VaAccess extends Expr { } /** * 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 - ) - } } /** @@ -62,12 +34,11 @@ class VaArgArg extends VaAccess { */ 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 between `va_list` usages. + * Dataflow configuration for flow from a library function + * to a call of function `asctime` */ module VaArgConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { @@ -75,7 +46,7 @@ module VaArgConfig implements DataFlow::ConfigSig { any(VariableDeclarationEntry m | m.getType().hasName("va_list")).getVariable() } - predicate isSink(DataFlow::Node sink) { exists(VaAccess va_acc | sink = va_acc.getDfn()) } + predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof VaAccess } } module VaArgFlow = DataFlow::Global; @@ -93,15 +64,15 @@ ControlFlowNode preceedsFC(VaAccess va_arg) { not result = any(MacroInvocation m | m.getMacroName() = ["va_start"] and - m.getExpr().getChild(0).(VariableAccess).getTarget() = va_arg.getTarget() + m.getExpr().getChild(0).(VariableAccess).getTarget() = va_arg.(VariableAccess).getTarget() ).getExpr() ) } predicate sameSource(VaAccess e1, VaAccess e2) { exists(DataFlow::Node source | - VaArgFlow::flow(source, e1.getDfn()) and - VaArgFlow::flow(source, e2.getDfn()) + VaArgFlow::flow(source, DataFlow::exprNode(e1)) and + VaArgFlow::flow(source, DataFlow::exprNode(e2)) ) } diff --git a/change_notes/2026-10-04-use-new-dataflow-in-msc39-c.md b/change_notes/2026-10-04-use-new-dataflow-in-msc39-c.md deleted file mode 100644 index b005511575..0000000000 --- a/change_notes/2026-10-04-use-new-dataflow-in-msc39-c.md +++ /dev/null @@ -1,2 +0,0 @@ - - `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. \ No newline at end of file