Implement a throw or raise statement #1733

maximiliankaul · 2024-09-26T14:41:23Z

This PR implements a new ThrowStatement

Check the new node. Does it work for all supported languages?
Implement EOG
Implement DFG
Implement all other relevant passes (are there any?)
Implement fluent tests

The language specific implementations will be handled in separate PRs:

C++ currently uses a UnaryOperator
Implement for Python Python: raise #1741
Implement for other languages

sonarcloud · 2024-09-27T09:24:43Z

Quality Gate failed

Failed conditions
29.6% Coverage on New Code (required ≥ 75%)

See analysis details on SonarCloud

cpg-core/src/test/kotlin/de/fraunhofer/aisec/cpg/graph/edges/flows/DataflowTest.kt

KuechA · 2024-10-02T18:38:31Z

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/EvaluationOrderGraphPass.kt

@@ -1028,6 +1029,10 @@ open class EvaluationOrderGraphPass(ctx: TranslationContext) : TranslationUnitPa
        pushToEOG(stmt)
    }

+    protected fun handleThrowStatement(statement: ThrowStatement) {
+        TODO("Needs implementing")


Suggested change

TODO("Needs implementing")

createEOG(statement.exception)

createEOG(statement.cause)

pushToEOG(statement)

Is this the right order of cause and exception?

However, I'm not sure how to connect the statement.nextEOG edge to either a CatchStatement (if it's surrounded by a try-catch with a matching catch) the FunctionDeclaration or if this will be subject of this PR at all. Might also be a separate PR to not block other developments.

In theory we might actually have all the necessary information because the EOG runs after the type resolver and we can the get the type information out of the CatchStatement and then match it. If there is no match, we should probably connect the EOG to the function declaration

Ah lol we already have a (working?) implementation in handleThrowOperator! This handles the C++ implementation with a throw unary operator. But we have a problem here... This relies on a TryScope to find a matching TryStatement. But in python we do not open a scope for try since it does not open a new (variable) scoping. This is now the problem that we are mixing scopes for variable visibility and other stuff :(

Additionally, the implementation is not yet handling nested try statements.

KuechA · 2024-10-02T18:43:47Z

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/DFGPass.kt

+        node.exception?.let { exc ->
+            when (exc) {
+                is CallExpression -> {
+                    handleCallExpression(exc, inferDfgForUnresolvedSymbols)


In fact, this should be much easier to handle ;) I guess what you want to do is to generate a DFG from the node.exception to node and maybe also from node.cause to node? This would be simply:

node.exception?.let{ node.prevDFGEdges += it } node.cause?.let{ node.prevDFGEdges += it }

Since the exception and cause are part of the AST, we will visit and handle them automatically.

KuechA · 2024-10-02T18:49:08Z

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/ThrowStatement.kt

+     * EOG or DFG connections, as it is only of informational purpose, but it doesn't change the
+     * program behavior.
+     */
+    @Relationship(value = "CAUSE") var causeEdge = astOptionalEdgeOf<Expression>()


Can this only be a Reference in all cases? Currently, it's any Expression. Adapt the type or it would make sense to include them in the EOG and DFG.

Anyway, I would recommend adding the cause to the DFG because e.g. if there's sensitive data inside the Exception in the "cause" we may also have this data in the exception raised here; this could violate some policies and the data flow would be lost without the DFG-edge.

For the EOG, it may be useful as well if we can do some more sophisticated things (and it shouldn't harm to have an edge here I guess).

The cause can be any expression, either a call / constructor to a new exception or a reference to an already created one. So, yes it would make sense to have a DFG from cause to throw.

Ah, I confused exception with cause... But same applies, in python this can be any expression.

Can we maybe rename this field to parentException, which is a bit more generic?

KuechA · 2024-10-02T18:52:00Z

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/ThrowStatement.kt

+    }
+
+    override fun hasArgument(expression: Expression): Boolean {
+        return when {


return exception == expression || cause == expression?

This is copy & paste from other nodes to be in line with the style.
Changed.

…lows/DataflowTest.kt Co-authored-by: KuechA <[email protected]>

* Rename `findSymbols` into `lookupSymbolByName` This PR renames `findSymbols` into `lookupSymbolByName` as a more appropriate name, because it lookups a symbol by its name. Fixes #1767 * Update cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/ScopeManager.kt Co-authored-by: KuechA <[email protected]> * Added documentation --------- Co-authored-by: KuechA <[email protected]>

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

Added language trait `HasImplicit Receiver` This is needed in preparation for #1777 to better handle access to fields/members of a class when an implicit receiver is used.

* Add new function `lookupUniqueTypeSymbolByName` This adds two new functions `ScopeManager.lookupUniqueTypeSymbolByName` and `Reference.doesReferToType`. This harmonizes a lot of boilerplate code in type resolver, cxx extra pass and resolve ambuigity pass, which were all trying to achieve the same thing. Fixes #1766 * Fixed issue with Go test, more robust handling of wrapped references * Addressed code review

… nodes (#1783) * Make sure to move `typeObservers` from old to new node when replacing nodes * Added doc for typeobservers

…1784) Otherwise, we override the code/location again.

Named expressions in Python use `:=` as operator. Therefore we need to include it in the language definition. Otherwise, the `access` value of a reference will not be set correctly.

maximiliankaul added 2 commits September 26, 2024 16:38

new node for RaiseStatement + NodeBuilder code

58c0528

don't add python stuff in this branch

4523b73

maximiliankaul self-assigned this Sep 26, 2024

maximiliankaul added 5 commits September 27, 2024 10:09

Merge branch 'main' into mk/raiseStmt

616c413

started work on EOG and DFG

9c8ac1b

doc

30c4908

raise: fluent and first test

3380050

fluent: patch @oxisto

40d7658

maximiliankaul added 3 commits September 27, 2024 11:26

Start work on DFG test for raise

ca5ac9d

rename raise -> throw

5054a4f

more renaming raise -> throw

735f0aa

maximiliankaul changed the title ~~Add a RaiseStatement~~ Implement a throw or raise statement Sep 27, 2024

maximiliankaul removed their assignment Sep 27, 2024

maximiliankaul added the graph-changing This change breaks the current graph structure, i.e. it changes semantic of properties and edges label Sep 27, 2024

Merge branch 'main' into mk/raiseStmt

9d5f5a5

KuechA reviewed Oct 2, 2024

View reviewed changes

KuechA mentioned this pull request Oct 2, 2024

Python: raise #1741

Draft

maximiliankaul and others added 12 commits October 4, 2024 16:02

copy & paste handleThrowOperator

db77867

Update cpg-core/src/test/kotlin/de/fraunhofer/aisec/cpg/graph/edges/f…

b0ed02b

…lows/DataflowTest.kt Co-authored-by: KuechA <[email protected]>

Update dependency rollup to v4.24.0 (#1774)

2c7dd71

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

Added language trait HasImplicitReceiver (#1778)

f542681

Added language trait `HasImplicit Receiver` This is needed in preparation for #1777 to better handle access to fields/members of a class when an implicit receiver is used.

Cleanup of SymbolResolver (#1777)

0187754

Fixed crash in getCodeOfSubregion (#1776)

9755f71

Make sure to move typeObservers from old to new node when replacing…

b358fa1

… nodes (#1783) * Make sure to move `typeObservers` from old to new node when replacing nodes * Added doc for typeobservers

implicit() only triggers code/location update now if its not empty (#…

5e83741

…1784) Otherwise, we override the code/location again.

Added := as simple operator in Python (#1785)

205a275

Named expressions in Python use `:=` as operator. Therefore we need to include it in the language definition. Otherwise, the `access` value of a reference will not be set correctly.

code review

89fd8ec

maximiliankaul added 2 commits October 8, 2024 11:27

code review

3d4c64b

code review

ebe1a27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement a throw or raise statement #1733

Implement a throw or raise statement #1733

maximiliankaul commented Sep 26, 2024 •

edited

Loading

sonarcloud bot commented Sep 27, 2024

KuechA Oct 2, 2024

KuechA Oct 2, 2024

oxisto Oct 3, 2024

oxisto Oct 3, 2024

KuechA Oct 2, 2024

KuechA Oct 2, 2024

oxisto Oct 3, 2024

oxisto Oct 3, 2024

oxisto Oct 3, 2024

KuechA Oct 2, 2024

maximiliankaul Oct 8, 2024

-        TODO("Needs implementing")
+        createEOG(statement.exception)
+        createEOG(statement.cause)
+        pushToEOG(statement)

Implement a throw or raise statement #1733

Are you sure you want to change the base?

Implement a throw or raise statement #1733

Conversation

maximiliankaul commented Sep 26, 2024 • edited Loading

sonarcloud bot commented Sep 27, 2024

Quality Gate failed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maximiliankaul commented Sep 26, 2024 •

edited

Loading