Skip to content

Commit

Permalink
[c#] Try-Catch-Finally Order Fix & Using Statement Handling (#4366)
Browse files Browse the repository at this point in the history
* Fixed the ordering property of try-catch statements by using the `tryCatchAst` function
* Handling `Using` statements and lowering them to `try-finally` blocks with a call to `Dispose` on the declared variable.

Resolves #3989
  • Loading branch information
DavidBakerEffendi authored Mar 19, 2024
1 parent 37d0a68 commit 9bcbfd0
Show file tree
Hide file tree
Showing 6 changed files with 153 additions and 120 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ class AstCreator(
with AstForPrimitivesCreator
with AstForExpressionsCreator
with AstForStatementsCreator
with AstForControlStructuresCreator
with AstSummaryVisitor
with AstGenNodeBuilder[AstCreator] {

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ trait AstForExpressionsCreator(implicit withSchemaValidation: ValidationMode) {
}

def astForExpression(expr: DotNetNodeInfo): Seq[Ast] = {
expr.node match
expr.node match {
case _: UnaryExpr => astForUnaryExpression(expr)
case _: BinaryExpr => astForBinaryExpression(expr)
case _: LiteralExpr => astForLiteralExpression(expr)
Expand All @@ -39,6 +39,7 @@ trait AstForExpressionsCreator(implicit withSchemaValidation: ValidationMode) {
case SuppressNullableWarningExpression => astForSuppressNullableWarningExpression(expr)
case _: BaseLambdaExpression => astForSimpleLambdaExpression(expr)
case _ => notHandledYet(expr)
}
}

private def astForAwaitExpression(awaitExpr: DotNetNodeInfo): Seq[Ast] = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,39 +1,14 @@
package io.joern.csharpsrc2cpg.astcreation

import io.circe.Json
import io.joern.csharpsrc2cpg.parser.DotNetJsonAst.{
BinaryExpr,
Block,
BreakStatement,
CasePatternSwitchLabel,
CaseSwitchLabel,
ContinueStatement,
DefaultSwitchLabel,
DoStatement,
ExpressionStatement,
ForEachStatement,
ForStatement,
GenericName,
GlobalStatement,
GotoStatement,
IfStatement,
JumpStatement,
LiteralExpr,
ReturnStatement,
SwitchStatement,
ThrowStatement,
TryStatement,
UnaryExpr,
WhileStatement
}
import io.joern.csharpsrc2cpg.CSharpOperators
import io.joern.csharpsrc2cpg.parser.DotNetJsonAst.*
import io.joern.csharpsrc2cpg.parser.{DotNetNodeInfo, ParserKeys}
import io.joern.x2cpg.{Ast, ValidationMode}
import io.shiftleft.codepropertygraph.generated.ControlStructureTypes
import io.shiftleft.codepropertygraph.generated.nodes.ControlStructure
import io.shiftleft.codepropertygraph.generated.{ControlStructureTypes, DispatchTypes}
import io.shiftleft.codepropertygraph.generated.nodes.{NewBlock, NewControlStructure, NewIdentifier}

import scala.::
import scala.collection.mutable.ArrayBuffer
import scala.util.{Failure, Success, Try}
import scala.util.{Success, Try}

trait AstForStatementsCreator(implicit withSchemaValidation: ValidationMode) { this: AstCreator =>

Expand All @@ -57,7 +32,7 @@ trait AstForStatementsCreator(implicit withSchemaValidation: ValidationMode) { t
}

protected def astForStatement(nodeInfo: DotNetNodeInfo): Seq[Ast] = {
nodeInfo.node match
nodeInfo.node match {
case ExpressionStatement => astForExpressionStatement(nodeInfo)
case GlobalStatement => astForGlobalStatement(nodeInfo)
case IfStatement => astForIfStatement(nodeInfo)
Expand All @@ -68,8 +43,10 @@ trait AstForStatementsCreator(implicit withSchemaValidation: ValidationMode) { t
case DoStatement => astForDoStatement(nodeInfo)
case WhileStatement => astForWhileStatement(nodeInfo)
case SwitchStatement => astForSwitchStatement(nodeInfo)
case UsingStatement => astForUsingStatement(nodeInfo)
case _: JumpStatement => astForJumpStatement(nodeInfo)
case _ => notHandledYet(nodeInfo)
}
}

private def astForSwitchLabel(labelNode: DotNetNodeInfo): Seq[Ast] = {
Expand Down Expand Up @@ -222,4 +199,95 @@ trait AstForStatementsCreator(implicit withSchemaValidation: ValidationMode) { t
val _returnNode = returnNode(returnStmt, returnStmt.code)
Seq(returnAst(_returnNode, identifierAst))
}

protected def astForThrowStatement(throwStmt: DotNetNodeInfo): Seq[Ast] = {
val expr = createDotNetNodeInfo(throwStmt.json(ParserKeys.Expression))
val args = astForNode(expr)
val throwCall = createCallNodeForOperator(
throwStmt,
CSharpOperators.throws,
typeFullName = Option(getTypeFullNameFromAstNode(args))
)
Seq(callAst(throwCall, args))
}

protected def astForTryStatement(tryStmt: DotNetNodeInfo): Seq[Ast] = {
val tryNode = NewControlStructure()
.controlStructureType(ControlStructureTypes.TRY)
.code("try")
.lineNumber(line(tryStmt))
.columnNumber(column(tryStmt))
val tryAst = astForBlock(createDotNetNodeInfo(tryStmt.json(ParserKeys.Block)), Option("try"))

val catchAsts = Try(tryStmt.json(ParserKeys.Catches)).map(_.arr.flatMap(astForNode).toSeq).getOrElse(Seq.empty)

val finallyBlock = Try(createDotNetNodeInfo(tryStmt.json(ParserKeys.Finally))).map(astForFinallyClause) match
case Success(finallyAst :: Nil) =>
finallyAst.root.collect { case x: NewBlock => x.code("finally") }
finallyAst
case _ => Ast()

val controlStructureAst = tryCatchAst(tryNode, tryAst, catchAsts, Option(finallyBlock))
Seq(controlStructureAst)
}

protected def astForFinallyClause(finallyClause: DotNetNodeInfo): Seq[Ast] = {
Seq(astForBlock(createDotNetNodeInfo(finallyClause.json(ParserKeys.Block)), code = Option(code(finallyClause))))
}

/** Variables using the <a
* href="https://learn.microsoft.com/en-us/dotnet/api/system.idisposable?view=net-8.0">IDisposable</a> interface may
* be used in `using`, where a call to `Dispose` is guaranteed.
*
* Thus, this is lowered as a try-finally, with finally making a call to `Dispose` on the declared variable.
*/
private def astForUsingStatement(usingStmt: DotNetNodeInfo): Seq[Ast] = {
val tryNode = NewControlStructure()
.controlStructureType(ControlStructureTypes.TRY)
.code("try")
.lineNumber(line(usingStmt))
.columnNumber(column(usingStmt))
val tryAst = astForBlock(createDotNetNodeInfo(usingStmt.json(ParserKeys.Statement)), Option("try"))
val declNode = createDotNetNodeInfo(usingStmt.json(ParserKeys.Declaration))
val declAst = astForNode(declNode)

val finallyAst = declAst.flatMap(_.nodes).collectFirst { case x: NewIdentifier => x.copy }.map { id =>
val callCode = s"${id.name}.Dispose()"
id.code(callCode)
val disposeCall = callNode(
usingStmt,
callCode,
"Dispose",
"System.Disposable.Dispose:System.Void()",
DispatchTypes.DYNAMIC_DISPATCH,
Option("System.Void()"),
Option("System.Void")
)
val disposeAst = callAst(disposeCall, receiver = Option(Ast(id)))
Ast(blockNode(usingStmt).code("finally"))
.withChild(disposeAst)
}

declAst :+ tryCatchAst(tryNode, tryAst, Seq.empty, finallyAst)
}

protected def astForCatchClause(catchClause: DotNetNodeInfo): Seq[Ast] = {
val declAst = astForNode(catchClause.json(ParserKeys.Declaration)).toList
val blockAst = astForBlock(
createDotNetNodeInfo(catchClause.json(ParserKeys.Block)),
code = Option(code(catchClause)),
prefixAsts = declAst
)
Seq(blockAst)
}

protected def astForCatchDeclaration(catchDeclaration: DotNetNodeInfo): Seq[Ast] = {
val name = nameFromNode(catchDeclaration)
val typeFullName = nodeTypeFullName(catchDeclaration)
val _localNode = localNode(catchDeclaration, name, name, typeFullName)
val localNodeAst = Ast(_localNode)
scope.addToScope(name, _localNode)
Seq(localNodeAst)
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@ object DotNetJsonAst {

object SwitchSection extends BaseExpr

object UsingStatement extends BaseStmt

object RelationalPattern extends BasePattern

object ConstantPattern extends BasePattern
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class ControlStructureTests extends CSharpCode2CpgFixture {
inside(tryBlocks) {
case t :: c :: f :: Nil =>
t.code shouldBe "try"
c.code shouldBe "catch"
c.code shouldBe "catch (Exception e)"
f.code shouldBe "finally"
case _ => fail("Invalid number of children under the `try` control structure!")
}
Expand All @@ -77,16 +77,12 @@ class ControlStructureTests extends CSharpCode2CpgFixture {

"generate a catch-block with a child block containing the correct console call" in {
inside(Option(tryBlocks(1))) {
case Some(ctl) =>
inside(ctl.astChildren.l) {
case (catchBlock: Block) :: Nil =>
inside(catchBlock.astChildren.l) {
case (excepDecl: Local) :: (consoleCall: Call) :: Nil =>
excepDecl.name shouldBe "e"
consoleCall.code shouldBe "Console.WriteLine(\"Uh, oh!\")"
case _ => fail("Invalid `catch` block children!")
}
case _ => fail("No `catch` handler block found!")
case Some(catchBlock: Block) =>
inside(catchBlock.astChildren.l) {
case (excepDecl: Local) :: (consoleCall: Call) :: Nil =>
excepDecl.name shouldBe "e"
consoleCall.code shouldBe "Console.WriteLine(\"Uh, oh!\")"
case _ => fail("Invalid `catch` block children!")
}
case None => fail("No `catch` block found!")
}
Expand Down Expand Up @@ -183,4 +179,47 @@ class ControlStructureTests extends CSharpCode2CpgFixture {

}

"a using statement" should {
val cpg = code(basicBoilerplate("""
|var numbers = new List<int>();
|using (StreamReader reader = File.OpenText("numbers.txt"))
|{
| string line;
| while ((line = reader.ReadLine()) is not null)
| {
| if (int.TryParse(line, out int number))
| {
| numbers.Add(number);
| }
| }
|}
|""".stripMargin))

val tryBlocks = cpg.controlStructure.controlStructureTypeExact(ControlStructureTypes.TRY).astChildren.isBlock.l

"generate a try control structure with two children blocks" in {
inside(tryBlocks) {
case t :: f :: Nil =>
t.code shouldBe "try"
f.code shouldBe "finally"
case _ => fail("Invalid number of children under the `try` control structure!")
}
}

"generate a finally-block with the correct dispose call" in {
inside(tryBlocks.lastOption) {
case Some(ctl) =>
inside(ctl.astChildren.isCall.l) {
case disposeCall :: Nil =>
disposeCall.code shouldBe "reader.Dispose()"
disposeCall.name shouldBe "Dispose"
disposeCall.methodFullName shouldBe "System.Disposable.Dispose:System.Void()"
case _ => fail("No call node found!")
}
case None => fail("No `finally` block found!")
}
}

}

}

0 comments on commit 9bcbfd0

Please sign in to comment.