From 4e18cca0f42b2aad9e982caec54a31cc7ce99b91 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 24 Jan 2024 14:14:38 +0000 Subject: [PATCH 1/2] C++: Add a way to test the behavior of 'asExpr' and 'toString' on dataflow nodes. --- .../dataflow/asExpr/test-indirect.expected | 2 + .../dataflow/asExpr/test-indirect.ql | 40 +++++++++++++++++++ .../library-tests/dataflow/asExpr/test.cpp | 23 +++++++++++ .../dataflow/asExpr/test.expected | 2 + .../library-tests/dataflow/asExpr/test.ql | 37 +++++++++++++++++ 5 files changed, 104 insertions(+) create mode 100644 cpp/ql/test/library-tests/dataflow/asExpr/test-indirect.expected create mode 100644 cpp/ql/test/library-tests/dataflow/asExpr/test-indirect.ql create mode 100644 cpp/ql/test/library-tests/dataflow/asExpr/test.cpp create mode 100644 cpp/ql/test/library-tests/dataflow/asExpr/test.expected create mode 100644 cpp/ql/test/library-tests/dataflow/asExpr/test.ql diff --git a/cpp/ql/test/library-tests/dataflow/asExpr/test-indirect.expected b/cpp/ql/test/library-tests/dataflow/asExpr/test-indirect.expected new file mode 100644 index 000000000000..8ec8033d086e --- /dev/null +++ b/cpp/ql/test/library-tests/dataflow/asExpr/test-indirect.expected @@ -0,0 +1,2 @@ +testFailures +failures diff --git a/cpp/ql/test/library-tests/dataflow/asExpr/test-indirect.ql b/cpp/ql/test/library-tests/dataflow/asExpr/test-indirect.ql new file mode 100644 index 000000000000..b7d6761f02ff --- /dev/null +++ b/cpp/ql/test/library-tests/dataflow/asExpr/test-indirect.ql @@ -0,0 +1,40 @@ +import cpp +import TestUtilities.InlineExpectationsTest +import semmle.code.cpp.dataflow.new.DataFlow::DataFlow + +bindingset[s] +string quote(string s) { if s.matches("% %") then result = "\"" + s + "\"" else result = s } + +string formatNumberOfNodesForIndirectExpr(Expr e) { + exists(int n | n = strictcount(Node node | node.asIndirectExpr() = e) | + n > 1 and result = ": " + n + ) +} + +module AsIndirectExprTest implements TestSig { + string getARelevantTag() { result = ["asIndirectExpr", "numberOfIndirectNodes"] } + + predicate hasActualResult(Location location, string element, string tag, string value) { + exists(Node n, Expr e, string exprString | + e = n.asIndirectExpr() and + location = e.getLocation() and + element = n.toString() and + exprString = e.toString() + | + tag = "asIndirectExpr" and + ( + // The toString on an indirect is often formatted like `***myExpr`. + // If the node's `toString` is of that form then we don't show it in + // the expected output. + if element.matches("%" + exprString) + then value = quote(exprString) + else value = quote(exprString + "(" + element + ")") + ) + or + tag = "numberOfIndirectNodes" and + value = quote(exprString + formatNumberOfNodesForIndirectExpr(e)) + ) + } +} + +import MakeTest diff --git a/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp b/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp new file mode 100644 index 000000000000..ce11111df68b --- /dev/null +++ b/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp @@ -0,0 +1,23 @@ +void take_const_ref_int(const int &); + +void test_materialize_temp_int() +{ + take_const_ref_int(42); // $ asExpr=42 numberOfNodes="42: 2" asIndirectExpr=42 numberOfIndirectNodes="42: 2" +} + +struct A {}; + +A get(); +void take_const_ref(const A &); + +void test1(){ + take_const_ref(get()); // $ asExpr="call to get" numberOfNodes="call to get: 2" asIndirectExpr="call to get" numberOfIndirectNodes="call to get: 2" +} + +void take_ref(A &); + +A& get_ref(); + +void test2() { + take_ref(get_ref()); // $ asExpr="call to get_ref" asIndirectExpr="call to get_ref" +} diff --git a/cpp/ql/test/library-tests/dataflow/asExpr/test.expected b/cpp/ql/test/library-tests/dataflow/asExpr/test.expected new file mode 100644 index 000000000000..8ec8033d086e --- /dev/null +++ b/cpp/ql/test/library-tests/dataflow/asExpr/test.expected @@ -0,0 +1,2 @@ +testFailures +failures diff --git a/cpp/ql/test/library-tests/dataflow/asExpr/test.ql b/cpp/ql/test/library-tests/dataflow/asExpr/test.ql new file mode 100644 index 000000000000..d686aad80613 --- /dev/null +++ b/cpp/ql/test/library-tests/dataflow/asExpr/test.ql @@ -0,0 +1,37 @@ +import cpp +import TestUtilities.InlineExpectationsTest +import semmle.code.cpp.dataflow.new.DataFlow::DataFlow + +bindingset[s] +string quote(string s) { if s.matches("% %") then result = "\"" + s + "\"" else result = s } + +string formatNumberOfNodesForExpr(Expr e) { + exists(int n | n = strictcount(Node node | node.asExpr() = e) | n > 1 and result = ": " + n) +} + +module AsExprTest implements TestSig { + string getARelevantTag() { result = ["asExpr", "numberOfNodes"] } + + predicate hasActualResult(Location location, string element, string tag, string value) { + exists(Node n, Expr e, string exprString | + e = n.asExpr() and + location = e.getLocation() and + element = n.toString() and + exprString = e.toString() + | + tag = "asExpr" and + ( + // If the `toString` on the node is identical to the `toString` of the + // expression then we don't show it in the expected output. + if exprString = element + then value = quote(exprString) + else value = quote(exprString + "(" + element + ")") + ) + or + tag = "numberOfNodes" and + value = quote(exprString + formatNumberOfNodesForExpr(e)) + ) + } +} + +import MakeTest From 7916616ee198932dd2b5bb2c64bef2d6971cf8f5 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 24 Jan 2024 16:07:52 +0000 Subject: [PATCH 2/2] C++: Fix duplication for indirect exprs similar to how we fixed it in #15410. --- .../semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll | 7 ++++++- cpp/ql/test/library-tests/dataflow/asExpr/test.cpp | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll index d9c4c6a187c2..3f1c61a3713c 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll @@ -1484,12 +1484,17 @@ private module IndirectNodeToIndirectExpr { indirectNodeHasIndirectExpr(node, e, n, indirectionIndex) and not exists(Expr conv, int adjustedIndirectionIndex | adjustForReference(e, indirectionIndex, conv, adjustedIndirectionIndex) and - indirectNodeHasIndirectExpr(_, conv, n + 1, adjustedIndirectionIndex) + indirectExprNodeShouldBe(conv, n + 1, adjustedIndirectionIndex) ) ) } } +private predicate indirectExprNodeShouldBe(Expr e, int n, int indirectionIndex) { + indirectExprNodeShouldBeIndirectOperand(_, e, n, indirectionIndex) or + indirectExprNodeShouldBeIndirectInstruction(_, e, n, indirectionIndex) +} + private module IndirectOperandIndirectExprNodeImpl implements IndirectNodeToIndirectExprSig { class IndirectNode = IndirectOperand; diff --git a/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp b/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp index ce11111df68b..74c630c1c39f 100644 --- a/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp +++ b/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp @@ -2,7 +2,7 @@ void take_const_ref_int(const int &); void test_materialize_temp_int() { - take_const_ref_int(42); // $ asExpr=42 numberOfNodes="42: 2" asIndirectExpr=42 numberOfIndirectNodes="42: 2" + take_const_ref_int(42); // $ asExpr=42 numberOfNodes="42: 2" asIndirectExpr=42 } struct A {}; @@ -11,7 +11,7 @@ A get(); void take_const_ref(const A &); void test1(){ - take_const_ref(get()); // $ asExpr="call to get" numberOfNodes="call to get: 2" asIndirectExpr="call to get" numberOfIndirectNodes="call to get: 2" + take_const_ref(get()); // $ asExpr="call to get" numberOfNodes="call to get: 2" asIndirectExpr="call to get" } void take_ref(A &);