Skip to content

C++: Placeholder CFG for coroutines #16187

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 10 commits into from
Apr 19, 2024
Merged

C++: Placeholder CFG for coroutines #16187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4c4d241
C++: Mark 'CoAwaitExpr' as a unary expression in IR construction.
MathiasVP Apr 11, 2024
3e1359b
C++: Accept test changes.
MathiasVP Apr 11, 2024
d8bd18f
C++: IR translation of 'co_return' statements.
MathiasVP Apr 11, 2024
33364a8
C++: Accept test changes.
MathiasVP Apr 11, 2024
d030f0b
C++: Add a placeholder for 'getOpcode'.
MathiasVP Apr 11, 2024
2a91477
C++: Accept test changes.
MathiasVP Apr 11, 2024
65b69fe
C++: Mark 'CoYieldExpr' as a unary expression in IR construction.
MathiasVP Apr 11, 2024
8842b97
C++: Accept test changes.
MathiasVP Apr 11, 2024
846eac8
C++: Add a placeholder for 'getOpcode'.
MathiasVP Apr 11, 2024
477322d
C++: Accept test changes.
MathiasVP Apr 11, 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
10 changes: 9 additions & 1 deletion cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1235,7 +1235,9 @@ class TranslatedUnaryExpr extends TranslatedSingleInstructionExpr {
expr instanceof NotExpr or
expr instanceof ComplementExpr or
expr instanceof UnaryPlusExpr or
expr instanceof UnaryMinusExpr
expr instanceof UnaryMinusExpr or
expr instanceof CoAwaitExpr or
expr instanceof CoYieldExpr
}

final override Instruction getFirstInstruction(EdgeKind kind) {
Expand Down Expand Up @@ -1275,6 +1277,12 @@ class TranslatedUnaryExpr extends TranslatedSingleInstructionExpr {
expr instanceof UnaryPlusExpr and result instanceof Opcode::CopyValue
or
expr instanceof UnaryMinusExpr and result instanceof Opcode::Negate
or
// TODO: Use a new opcode to represent "awaiting the value"
expr instanceof CoAwaitExpr and result instanceof Opcode::CopyValue
or
// TODO: Use a new opcode to represent "awaiting the value"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be:

Suggested change
// TODO: Use a new opcode to represent "awaiting the value"
// TODO: Use a new opcode to represent "yielding a value"

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually no. co_yield is just syntactic sugar for:

co_await promise.yield_value(expr)

(see here) and the "yielding" is done by the call to yield_value.

So whatever instruction we come up with to use the above TODO should also be used here since a co_yield ultimately does something equivalent to a special kind of co_await.

expr instanceof CoYieldExpr and result instanceof Opcode::CopyValue
}

private TranslatedExpr getOperand() {
Expand Down
38 changes: 38 additions & 0 deletions cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedStmt.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1501,3 +1501,41 @@ class TranslatedVlaDeclarationStmt extends TranslatedStmt {

override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) { none() }
}

class TranslatedCoReturnStmt extends TranslatedStmt {
override CoReturnStmt stmt;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not know you can do this (override the type of a member variable in QL).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's extremely useful! We do it a lot in both C/C++ IR construction and in the Swift CFG construction code


private TranslatedExpr getTranslatedOperand() {
result = getTranslatedExpr(stmt.getOperand().getFullyConverted())
}

override TranslatedExpr getChildInternal(int id) {
id = 0 and
result = this.getTranslatedOperand()
}

override Instruction getFirstInstruction(EdgeKind kind) {
result = this.getTranslatedOperand().getFirstInstruction(kind)
}

override Instruction getALastInstructionInternal() {
result = this.getInstruction(OnlyInstructionTag())
}

override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
tag = OnlyInstructionTag() and
opcode instanceof Opcode::NoOp and
resultType = getVoidType()
}

override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) {
tag = OnlyInstructionTag() and
result = this.getParent().getChildSuccessor(this, kind)
}

override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
child = this.getTranslatedOperand() and
kind instanceof GotoEdge and
result = this.getInstruction(OnlyInstructionTag())
}
}
277 changes: 265 additions & 12 deletions cpp/ql/test/library-tests/ir/ir/aliased_ir.expected
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 0 additions & 6 deletions cpp/ql/test/library-tests/ir/ir/aliased_ssa_consistency.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,6 @@ missingOperandType
duplicateChiOperand
sideEffectWithoutPrimary
instructionWithoutSuccessor
| coroutines.cpp:87:20:87:20 | Uninitialized: declaration of (unnamed local variable) | Instruction 'Uninitialized: declaration of (unnamed local variable)' has no successors in function '$@'. | coroutines.cpp:87:20:87:33 | co_returnable_void co_return_void() | co_returnable_void co_return_void() |
| coroutines.cpp:91:21:91:21 | Uninitialized: declaration of (unnamed local variable) | Instruction 'Uninitialized: declaration of (unnamed local variable)' has no successors in function '$@'. | coroutines.cpp:91:21:91:33 | co_returnable_value co_return_int(int) | co_returnable_value co_return_int(int) |
| coroutines.cpp:95:20:95:20 | Uninitialized: declaration of (unnamed local variable) | Instruction 'Uninitialized: declaration of (unnamed local variable)' has no successors in function '$@'. | coroutines.cpp:95:20:95:38 | co_returnable_void co_yield_value_void(int) | co_returnable_void co_yield_value_void(int) |
| coroutines.cpp:99:21:99:21 | Uninitialized: declaration of (unnamed local variable) | Instruction 'Uninitialized: declaration of (unnamed local variable)' has no successors in function '$@'. | coroutines.cpp:99:21:99:40 | co_returnable_value co_yield_value_value(int) | co_returnable_value co_yield_value_value(int) |
| coroutines.cpp:103:20:103:20 | Uninitialized: declaration of (unnamed local variable) | Instruction 'Uninitialized: declaration of (unnamed local variable)' has no successors in function '$@'. | coroutines.cpp:103:20:103:43 | co_returnable_void co_yield_and_return_void(int) | co_returnable_void co_yield_and_return_void(int) |
| coroutines.cpp:108:21:108:21 | Uninitialized: declaration of (unnamed local variable) | Instruction 'Uninitialized: declaration of (unnamed local variable)' has no successors in function '$@'. | coroutines.cpp:108:21:108:45 | co_returnable_value co_yield_and_return_value(int) | co_returnable_value co_yield_and_return_value(int) |
ambiguousSuccessors
unexplainedLoop
unnecessaryPhiInstruction
Expand Down
6 changes: 0 additions & 6 deletions cpp/ql/test/library-tests/ir/ir/aliased_ssa_consistency_unsound.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,6 @@ missingOperandType
duplicateChiOperand
sideEffectWithoutPrimary
instructionWithoutSuccessor
| coroutines.cpp:87:20:87:20 | Uninitialized: declaration of (unnamed local variable) | Instruction 'Uninitialized: declaration of (unnamed local variable)' has no successors in function '$@'. | coroutines.cpp:87:20:87:33 | co_returnable_void co_return_void() | co_returnable_void co_return_void() |
| coroutines.cpp:91:21:91:21 | Uninitialized: declaration of (unnamed local variable) | Instruction 'Uninitialized: declaration of (unnamed local variable)' has no successors in function '$@'. | coroutines.cpp:91:21:91:33 | co_returnable_value co_return_int(int) | co_returnable_value co_return_int(int) |
| coroutines.cpp:95:20:95:20 | Uninitialized: declaration of (unnamed local variable) | Instruction 'Uninitialized: declaration of (unnamed local variable)' has no successors in function '$@'. | coroutines.cpp:95:20:95:38 | co_returnable_void co_yield_value_void(int) | co_returnable_void co_yield_value_void(int) |
| coroutines.cpp:99:21:99:21 | Uninitialized: declaration of (unnamed local variable) | Instruction 'Uninitialized: declaration of (unnamed local variable)' has no successors in function '$@'. | coroutines.cpp:99:21:99:40 | co_returnable_value co_yield_value_value(int) | co_returnable_value co_yield_value_value(int) |
| coroutines.cpp:103:20:103:20 | Uninitialized: declaration of (unnamed local variable) | Instruction 'Uninitialized: declaration of (unnamed local variable)' has no successors in function '$@'. | coroutines.cpp:103:20:103:43 | co_returnable_void co_yield_and_return_void(int) | co_returnable_void co_yield_and_return_void(int) |
| coroutines.cpp:108:21:108:21 | Uninitialized: declaration of (unnamed local variable) | Instruction 'Uninitialized: declaration of (unnamed local variable)' has no successors in function '$@'. | coroutines.cpp:108:21:108:45 | co_returnable_value co_yield_and_return_value(int) | co_returnable_value co_yield_and_return_value(int) |
ambiguousSuccessors
unexplainedLoop
unnecessaryPhiInstruction
Expand Down
Loading