Skip to content

C++: Revert #15528 #15601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 13, 2024
Merged

C++: Revert #15528 #15601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cb7fe16
Revert "Merge pull request #15537 from MathiasVP/swap-also-clears-fir…
MathiasVP Feb 9, 2024
fb4bd53
Revert "Merge pull request #15528 from MathiasVP/flow-barrier-interface"
MathiasVP Feb 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions cpp/ql/lib/change-notes/2024-02-06-flow-out-barrier-function.md
View file
Open in desktop

This file was deleted.

25 changes: 1 addition & 24 deletions cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,8 @@ private import codeql.ssa.Ssa as SsaImplCommon
private import semmle.code.cpp.ir.IR
private import DataFlowUtil
private import DataFlowImplCommon as DataFlowImplCommon
private import semmle.code.cpp.ir.dataflow.internal.ModelUtil
private import semmle.code.cpp.models.interfaces.Allocation as Alloc
private import semmle.code.cpp.models.interfaces.DataFlow as DataFlow
private import semmle.code.cpp.models.interfaces.FlowOutBarrier as FOB
private import semmle.code.cpp.models.interfaces.FunctionInputsAndOutputs as FIO
private import semmle.code.cpp.ir.internal.IRCppLanguage
private import DataFlowPrivate
private import ssa0.SsaInternals as SsaInternals0
Expand Down Expand Up @@ -797,30 +794,10 @@ private Node getAPriorDefinition(SsaDefOrUse defOrUse) {
)
}

/**
* Holds if there should not be use-use flow out of `n` (or a conversion that
* flows to `n`).
*/
private predicate modeledFlowBarrier(Node n) {
exists(FIO::FunctionInput input, CallInstruction call |
call.getStaticCallTarget().(FOB::FlowOutBarrierFunction).isFlowOutBarrier(input) and
n = callInput(call, input)
)
or
exists(Operand operand, Instruction instr, Node n0, int indirectionIndex |
modeledFlowBarrier(n0) and
nodeHasInstruction(n0, instr, indirectionIndex) and
conversionFlow(operand, instr, false, _) and
nodeHasOperand(n, operand, indirectionIndex)
)
}

/** Holds if there is def-use or use-use flow from `nodeFrom` to `nodeTo`. */
predicate ssaFlow(Node nodeFrom, Node nodeTo) {
exists(Node nFrom, boolean uncertain, SsaDefOrUse defOrUse |
ssaFlowImpl(defOrUse, nFrom, nodeTo, uncertain) and
not modeledFlowBarrier(nFrom) and
nodeFrom != nodeTo
ssaFlowImpl(defOrUse, nFrom, nodeTo, uncertain) and nodeFrom != nodeTo
|
if uncertain = true then nodeFrom = [nFrom, getAPriorDefinition(defOrUse)] else nodeFrom = nFrom
)
Expand Down
13 changes: 2 additions & 11 deletions cpp/ql/lib/semmle/code/cpp/models/implementations/Swap.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
import semmle.code.cpp.models.interfaces.DataFlow
import semmle.code.cpp.models.interfaces.Taint
import semmle.code.cpp.models.interfaces.Alias
import semmle.code.cpp.models.interfaces.FlowOutBarrier

/**
* The standard function `swap`. A use of `swap` looks like this:
* ```
* std::swap(obj1, obj2)
* ```
*/
private class Swap extends DataFlowFunction, FlowOutBarrierFunction {
private class Swap extends DataFlowFunction {
Swap() { this.hasQualifiedName(["std", "bsl"], "swap") }

override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
Expand All @@ -19,8 +18,6 @@ private class Swap extends DataFlowFunction, FlowOutBarrierFunction {
input.isParameterDeref(1) and
output.isParameterDeref(0)
}

override predicate isFlowOutBarrier(FunctionInput input) { input.isParameterDeref([0, 1]) }
}

/**
Expand All @@ -29,9 +26,7 @@ private class Swap extends DataFlowFunction, FlowOutBarrierFunction {
* obj1.swap(obj2)
* ```
*/
private class MemberSwap extends TaintFunction, MemberFunction, AliasFunction,
FlowOutBarrierFunction
{
private class MemberSwap extends TaintFunction, MemberFunction, AliasFunction {
MemberSwap() {
this.hasName("swap") and
this.getNumberOfParameters() = 1 and
Expand All @@ -52,8 +47,4 @@ private class MemberSwap extends TaintFunction, MemberFunction, AliasFunction,
override predicate parameterEscapesOnlyViaReturn(int index) { index = 0 }

override predicate parameterIsAlwaysReturned(int index) { index = 0 }

override predicate isFlowOutBarrier(FunctionInput input) {
input.isQualifierObject() or input.isParameterDeref(0)
}
}
26 changes: 0 additions & 26 deletions cpp/ql/lib/semmle/code/cpp/models/interfaces/FlowOutBarrier.qll
View file
Open in desktop

This file was deleted.

16 changes: 8 additions & 8 deletions cpp/ql/test/library-tests/dataflow/taint-tests/map.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,11 @@ void test_pair()
sink(i.second); // $ MISSING: ast,ir
sink(i); // $ ast,ir
sink(j.first);
sink(j.second); // $ SPURIOUS: ast
sink(j); // $ SPURIOUS: ast
sink(j.second); // $ SPURIOUS: ast,ir
sink(j); // $ SPURIOUS: ast,ir
sink(k.first);
sink(k.second); // $ SPURIOUS: ast
sink(k); // $ SPURIOUS: ast
sink(k.second); // $ SPURIOUS: ast,ir
sink(k); // $ SPURIOUS: ast,ir
sink(l.first);
sink(l.second); // $ MISSING: ast,ir
sink(l); // $ ast,ir
Expand Down Expand Up @@ -196,10 +196,10 @@ void test_map()
sink(m18); // $ ast,ir
m15.swap(m16);
m17.swap(m18);
sink(m15); // $ SPURIOUS: ast
sink(m15); // $ SPURIOUS: ast,ir
sink(m16); // $ ast,ir
sink(m17); // $ ast,ir
sink(m18); // $ SPURIOUS: ast
sink(m18); // $ SPURIOUS: ast,ir

// merge
std::map<char *, char *> m19, m20, m21, m22;
Expand Down Expand Up @@ -345,10 +345,10 @@ void test_unordered_map()
sink(m18); // $ ast,ir
m15.swap(m16);
m17.swap(m18);
sink(m15); // $ SPURIOUS: ast
sink(m15); // $ SPURIOUS: ast,ir
sink(m16); // $ ast,ir
sink(m17); // $ ast,ir
sink(m18); // $ SPURIOUS: ast
sink(m18); // $ SPURIOUS: ast,ir

// merge
std::unordered_map<char *, char *> m19, m20, m21, m22;
Expand Down
8 changes: 4 additions & 4 deletions cpp/ql/test/library-tests/dataflow/taint-tests/set.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,10 @@ void test_set()
sink(s15); // $ ast,ir
s12.swap(s13);
s14.swap(s15);
sink(s12); // $ SPURIOUS: ast
sink(s12); // $ SPURIOUS: ast,ir
sink(s13); // $ ast,ir
sink(s14); // $ ast,ir
sink(s15); // $ SPURIOUS: ast
sink(s15); // $ SPURIOUS: ast,ir

// merge
std::set<char *> s16, s17, s18, s19;
Expand Down Expand Up @@ -193,10 +193,10 @@ void test_unordered_set()
sink(s15); // $ ast,ir
s12.swap(s13);
s14.swap(s15);
sink(s12); // $ SPURIOUS: ast
sink(s12); // $ SPURIOUS: ast,ir
sink(s13); // $ ast,ir
sink(s14); // $ ast,ir
sink(s15); // $ SPURIOUS: ast
sink(s15); // $ SPURIOUS: ast,ir

// merge
std::unordered_set<char *> s16, s17, s18, s19;
Expand Down
4 changes: 2 additions & 2 deletions cpp/ql/test/library-tests/dataflow/taint-tests/string.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,9 +280,9 @@ void test_string_swap() {
s4.swap(s3);

sink(s1); // $ ast,ir
sink(s2); // $ SPURIOUS: ast
sink(s2); // $ SPURIOUS: ast,ir
sink(s3); // $ ast,ir
sink(s4); // $ SPURIOUS: ast
sink(s4); // $ SPURIOUS: ast,ir
}

void test_string_clear() {
Expand Down
4 changes: 2 additions & 2 deletions cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,9 @@ void test_stringstream_swap()
ss4.swap(ss3);

sink(ss1); // $ ast,ir
sink(ss2); // $ SPURIOUS: ast
sink(ss2); // $ SPURIOUS: ast,ir
sink(ss3); // $ ast,ir
sink(ss4); // $ SPURIOUS: ast
sink(ss4); // $ SPURIOUS: ast,ir
}

void test_stringstream_in()
Expand Down
2 changes: 1 addition & 1 deletion cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ void test_swap() {

std::swap(x, y);

sink(x); // $ SPURIOUS: ast
sink(x); // $ SPURIOUS: ast,ir
sink(y); // $ ast,ir
}

Expand Down
4 changes: 2 additions & 2 deletions cpp/ql/test/library-tests/dataflow/taint-tests/vector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,10 @@ void test_vector_swap() {
v1.swap(v2);
v3.swap(v4);

sink(v1); // $ SPURIOUS: ast
sink(v1); // $ SPURIOUS: ast,ir
sink(v2); // $ ast,ir
sink(v3); // $ ast,ir
sink(v4); // $ SPURIOUS: ast
sink(v4); // $ SPURIOUS: ast,ir
}

void test_vector_clear() {
Expand Down