Skip to content

Commit

Permalink
Remove excessive debug statements and format comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@wunused
wunused committed Nov 15, 2023
1 parent 8b29208 commit 335a14c
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 193 deletions.
205 changes: 23 additions & 182 deletions joern-cli/frontends/php2cpg/src/main/scala/io/joern/php2cpg/passes/PhpTypeRecovery.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,6 @@ private class PhpTypeRecovery(cpg: Cpg, state: XTypeRecoveryState) extends XType

private class RecoverForPhpFile(cpg: Cpg, cu: NamespaceBlock, builder: DiffGraphBuilder, state: XTypeRecoveryState)
extends RecoverForXCompilationUnit[NamespaceBlock](cpg, cu, builder, state) {

override protected def prepopulateSymbolTable(): Unit = {
logger.debug(s"prepopulating symbol table")
super.prepopulateSymbolTable()
logger.debug(s"symbol table: ${symbolTable.view.mkString(" ")}")
}

override protected def prepopulateSymbolTableEntry(x: AstNode): Unit = x match {
case x: Call =>
x.methodFullName match {
Expand Down Expand Up @@ -70,64 +63,21 @@ private class RecoverForPhpFile(cpg: Cpg, cu: NamespaceBlock, builder: DiffGraph
.filter(_.dispatchType == DispatchTypes.DYNAMIC_DISPATCH)
.filter(_.methodFullName.startsWith(Defines.UnresolvedNamespace))

override def compute(): Boolean = {
logger.debug(s"compute() file: ${cu.file.name.l.mkString(" ")}")
// Note that we override postSetTypeInformation to iterate over and resolve
// the unresolvedDynamicCalls.
super.compute()
}

override def visitAssignments(a: OpNodes.Assignment): Set[String] = {
logger.debug(s"""visiting assigment: ${a.name}
|- arguments: ${a.argumentOut.l.mkString(" ")}\n- iteration: ${state.currentIteration}""".stripMargin)
super.visitAssignments(a)
}

override protected def visitIdentifierAssignedToBlock(i: Identifier, b: Block): Set[String] = {
logger.debug(s"visiting identifier ${i.name} assigned to block ${b.id}")
super.visitIdentifierAssignedToBlock(i, b)
}

override protected def visitCallAssignedToBlock(c: Call, b: Block): Set[String] = {
logger.debug(s"visiting call ${c.name} assigned to block ${b.id}")
super.visitCallAssignedToBlock(c, b)
}

override protected def visitStatementsInBlock(b: Block, assignmentTarget: Option[Identifier] = None): Set[String] = {
logger.debug(s"visiting statements in block ${b.id}")
super.visitStatementsInBlock(b, assignmentTarget)
}

override protected def visitIdentifierAssignedToCall(i: Identifier, c: Call): Set[String] = {
logger.debug(s"visiting identifier ${i.name} assigned to call ${c.name}")
super.visitIdentifierAssignedToCall(i, c)
}

override protected def visitIdentifierAssignedToIdentifier(x: Identifier, y: Identifier): Set[String] = {
logger.debug(s"visiting identifier ${x.name} assigned to identifier ${y.name}")
super.visitIdentifierAssignedToIdentifier(x, y)
}

override protected def visitIdentifierAssignedToOperator(i: Identifier, c: Call, operation: String): Set[String] = {
logger.debug(s"visiting identifier ${i.name} assigned to operator ${operation}")
super.visitIdentifierAssignedToOperator(i, c, operation)
/* Register post-processing pass that executes in the super class */
override protected def postSetTypeInformation(): Unit = {
unresolvedDynamicCalls.foreach(visitUnresolvedDynamicCall)
}

override protected def visitIdentifierAssignedToConstructor(i: Identifier, c: Call): Set[String] = {
logger.debug(s"visiting identifier ${i.name} assigned to constructor ${c.name}")
val constructorPaths = symbolTable.get(c).map(_.stripSuffix(s"${pathSep}<init>"))
associateTypes(i, constructorPaths)
}

override protected def visitIdentifierAssignedToCallRetVal(i: Identifier, c: Call): Set[String] = {
logger.debug(s"visiting identifier ${i.name} assigned to CallRetVal ${c.name}")

if (symbolTable.contains(c)) {
logger.debug(s"- symbol table contains call")
val callReturns = methodReturnValues(symbolTable.get(c).toSeq)
associateTypes(i, callReturns)
} else if (c.argument.exists(_.argumentIndex == 0)) {
logger.debug(s"- argument index exists")
val callFullNames = (c.argument(0) match {
case i: Identifier if symbolTable.contains(LocalVar(i.name)) => symbolTable.get(LocalVar(i.name))
case i: Identifier if symbolTable.contains(CallAlias(i.name)) => symbolTable.get(CallAlias(i.name))
Expand All @@ -136,66 +86,16 @@ private class RecoverForPhpFile(cpg: Cpg, cu: NamespaceBlock, builder: DiffGraph
val callReturns = methodReturnValues(callFullNames)
associateTypes(i, callReturns)
} else {
// Check if the CPG already contains type info for this method, otherwise
// use dummy return value.
logger.debug(s"- checking CPG")
/* CPG may already contain type info for this method (globally, outside of compilation)
* unit. If not, use dummy return value.
*/
val rs = methodReturnValues(Seq(c.methodFullName))
if (rs.isEmpty) associateTypes(i, Set(s"${c.name}$pathSep${XTypeRecovery.DummyReturnType}"))
else associateTypes(i, rs)
}
}

override protected def visitIdentifierAssignedToLiteral(i: Identifier, l: Literal): Set[String] = {
logger.debug(s"visiting identifier ${i.name} assigned to literal ${l.code}")
super.visitIdentifierAssignedToLiteral(i, l)
}

override protected def visitIdentifierAssignedToMethodRef(
i: Identifier,
m: MethodRef,
rec: Option[String] = None
): Set[String] = {
logger.debug(s"visiting identifier ${i.name} assigned to MethodRef ${m.code}")
super.visitIdentifierAssignedToMethodRef(i, m, rec)
}

override protected def visitIdentifierAssignedToTypeRef(
i: Identifier,
t: TypeRef,
rec: Option[String] = None
): Set[String] = {
logger.debug(s"visiting identifier ${i.name} assigned to TypeRef ${t.code}")
super.visitIdentifierAssignedToTypeRef(i, t, rec)
}

override protected def visitCallAssignedToIdentifier(c: Call, i: Identifier): Set[String] = {
logger.debug(s"visiting call ${c.name} assigned to identifier ${i.name}")
super.visitCallAssignedToIdentifier(c, i)
}

override protected def visitCallAssignedToCall(x: Call, y: Call): Set[String] = {
logger.debug(s"""visiting call ${x.name} assigned to call ${y.name}
|- ${x.name}: [${getTypesFromCall(y).mkString(", ")}]""".stripMargin)
super.visitCallAssignedToCall(x, y)
}

override protected def visitCallAssignedToLiteral(c: Call, l: Literal): Set[String] = {
logger.debug(s"visiting call ${c.name} assigned to literal ${l.code}")
super.visitCallAssignedToLiteral(c, l)
}

override protected def visitCallAssignedToMethodRef(c: Call, m: MethodRef): Set[String] = {
logger.debug(s"visiting call ${c.name} assigned to MethodRef ${m.code}")
super.visitCallAssignedToMethodRef(c, m)
}

override protected def visitIdentifierAssignedToFieldLoad(i: Identifier, fa: FieldAccess): Set[String] = {
logger.debug(s"visiting field identifier: ${getFieldName(fa)}")
super.visitIdentifierAssignedToFieldLoad(i, fa)
}

override protected def visitReturns(ret: Return): Unit = {
logger.debug(s"visiting return: ${ret.method.name}:(ID${ret.id})")
/* A bug in XTypeRecovery mishandles functions that have multiple return
* statements. We add a new "symbol table" (methodTypesTable) for method
* return types as they get collected across the multiple return statements
Expand All @@ -207,9 +107,7 @@ private class RecoverForPhpFile(cpg: Cpg, cu: NamespaceBlock, builder: DiffGraph
.filterNot(_ == "ANY")
.filterNot(_.startsWith(Defines.UnresolvedNamespace))
)
logger.debug(s"- existing types: ${existingTypes.mkString(", ")}")
existingTypes.addAll(methodTypesTable.getOrElse(m, mutable.HashSet()))
logger.debug(s"- existing types + methodTypesTable: ${existingTypes.mkString(", ")}")

@tailrec
def extractTypes(xs: List[CfgNode]): Set[String] = xs match {
Expand Down Expand Up @@ -245,22 +143,13 @@ private class RecoverForPhpFile(cpg: Cpg, cu: NamespaceBlock, builder: DiffGraph
case _ => Set.empty
}
val returnTypes = extractTypes(ret.argumentOut.l)
logger.debug(s"- extracted types: ${returnTypes.mkString(", ")}")
existingTypes.addAll(returnTypes)
logger.debug(s"- new existing types: ${existingTypes.mkString(", ")}")

// Look up whether the function return is already known (either in the CPG
// or in the methodTypesTable) and if it is known and in our set, then
// remove it from the saveTypes set.
/* Check whether method return is already known, and if so, remove dummy value */
val saveTypes = existingTypes.filterNot(typeName => {
if (typeName.startsWith(Defines.UnresolvedNamespace))
true
else if (typeName.endsWith(s"${XTypeRecovery.DummyReturnType}"))
// 1. Get methodFullName from typeName
// 2. Check if types for methodFullName are known
// - if they are, check if they are in the existingTypes.
// - if they are, remove this method dummy from the set by returning true
// - else, keep method dummy in the set by returning false.
typeName.split(pathSep).headOption match {
case Some(methodName) => {
val methodReturns = methodReturnValues(Seq(methodName))
Expand All @@ -273,15 +162,12 @@ private class RecoverForPhpFile(cpg: Cpg, cu: NamespaceBlock, builder: DiffGraph
false
})
methodTypesTable.update(m, saveTypes)
logger.debug(s"- saving types: ${saveTypes.mkString(", ")}")
builder.setNodeProperty(ret.method.methodReturn, PropertyNames.DYNAMIC_TYPE_HINT_FULL_NAME, saveTypes)
}

/** Necessary to change the filter regex from (this|self) to (\\$this|this), in order to account for $this PHP
* convention.
*
* (But need to leave the regular "this"? uncertain)
*/
/* Necessary to change the filter regex from (this|self) to (\\$this|this), in order to account for $this PHP
* convention.
*/
override protected def associateTypes(symbol: LocalVar, fa: FieldAccess, types: Set[String]): Set[String] = {
fa.astChildren.filterNot(_.code.matches("(\\$this|this|self)")).headOption.collect {
case fi: FieldIdentifier =>
Expand All @@ -292,8 +178,7 @@ private class RecoverForPhpFile(cpg: Cpg, cu: NamespaceBlock, builder: DiffGraph
symbolTable.append(symbol, types)
}

/** Reference the PythonTypeRecovery implementation. The XTypeRecovery one seems incorrect.
*/
/* Reference the PythonTypeRecovery implementation. The XTypeRecovery one seems incorrect. */
override protected def getFieldParents(fa: FieldAccess): Set[String] = {
if (fa.method.name == "<module>") {
Set(fa.method.fullName)
Expand All @@ -306,25 +191,11 @@ private class RecoverForPhpFile(cpg: Cpg, cu: NamespaceBlock, builder: DiffGraph
}
}

override protected def persistMemberWithTypeDecl(
typeFullName: String,
memberName: String,
types: Set[String]
): Unit = {
logger.debug(
s"persisting member with TypeDecl: typeFullName: ${typeFullName}, memberName: ${memberName}, types: [${types.mkString("; ")}]"
)
super.persistMemberWithTypeDecl(typeFullName, memberName, types)
}

override protected def getTypesFromCall(c: Call): Set[String] = c.name match {
case Operators.fieldAccess => symbolTable.get(LocalVar(getFieldName(new FieldAccess(c))))
case _ if symbolTable.contains(c) => symbolTable.get(c)
case Operators.indexAccess => getIndexAccessTypes(c)
case n => {
logger.debug(s"Unknown RHS call type '$n' @ ${c.name}\n- looking up in CPG")
methodReturnValues(Seq(c.methodFullName))
}
case n => methodReturnValues(Seq(c.methodFullName))
}

override protected def indexAccessToCollectionVar(c: Call): Option[CollectionVar] = {
Expand All @@ -337,8 +208,6 @@ private class RecoverForPhpFile(cpg: Cpg, cu: NamespaceBlock, builder: DiffGraph
.getOrElse(XTypeRecovery.DummyIndexAccess)
else x.name

logger.debug(s"Index access to collection var\n- call name: ${c.name}")

val collectionVar = Option(c.argumentOut.l match {
case List(i: Identifier, idx: Literal) => CollectionVar(i.name, idx.code)
case List(i: Identifier, idx: Identifier) => CollectionVar(i.name, idx.code)
Expand All @@ -349,46 +218,25 @@ private class RecoverForPhpFile(cpg: Cpg, cu: NamespaceBlock, builder: DiffGraph
logger.debug(s"Unhandled index access ${xs.map(x => (x.label, x.code)).mkString(",")} @ ${c.name}")
null
})
logger.debug(s"- collection var: ${collectionVar.mkString}")

collectionVar
}

override protected def storeCallTypeInfo(c: Call, types: Seq[String]): Unit = {
logger.debug(s"storing call type info: ${c.methodFullName}: [${types.mkString(", ")}]")
super.storeCallTypeInfo(c, types)
}

override protected def assignTypesToCall(x: Call, types: Set[String]): Set[String] = {
logger.debug(s"assigning types to call: ${x.name}: [${types.mkString(", ")}]")
if (types.nonEmpty) {
getSymbolFromCall(x) match {
case (lhs, globalKeys) if globalKeys.nonEmpty => {
logger.debug(s"- globalKeys non-empty: lhs: ${lhs.toString()}")
globalKeys.foreach { (fieldVar: FieldPath) =>
logger.debug(
s"- persisting member with type decl: compUnitFullName: ${fieldVar.compUnitFullName}; identifier: ${fieldVar.identifier}; types: ${types
.mkString(", ")}"
)
persistMemberWithTypeDecl(fieldVar.compUnitFullName, fieldVar.identifier, types)
}
symbolTable.append(lhs, types)
}
case (lhs, _) => {
logger.debug(s"- globalKeys empty: lhs: ${lhs.toString()}")
symbolTable.append(lhs, types)
}
case (lhs, _) => symbolTable.append(lhs, types)
}
} else Set.empty
}

override protected def persistType(x: StoredNode, types: Set[String]): Unit = {
logger.debug(s"persisting type: ${x.label}: [${types.mkString(",")}]")
super.persistType(x, types)
}

override protected def methodReturnValues(methodFullNames: Seq[String]): Set[String] = {
// Look up methods in existing CPG
/* Look up methods in existing CPG */
val rs = cpg.method
.fullNameExact(methodFullNames: _*)
.methodReturn
Expand All @@ -397,34 +245,31 @@ private class RecoverForPhpFile(cpg: Cpg, cu: NamespaceBlock, builder: DiffGraph
.filterNot(_.endsWith("alloc.<init>"))
.filterNot(_.endsWith(s"${XTypeRecovery.DummyReturnType}"))
.toSet
logger.debug(s"method return values: ${rs.mkString(",")}")
if (rs.isEmpty)
// Return dummy return type if not found.
/* Return dummy return type if not found */
methodFullNames
.flatMap(m => Set(m.concat(s"$pathSep${XTypeRecovery.DummyReturnType}")))
.toSet
else rs
}

/* If we know the type of the method's first parameter, use that to determine the method scope.
*
* TODO: Are there methods / instances where this doesn't work? Static methods?
* TODO: What if the first parameter could take multiple types?
* TODO: Test on nested dynamic calls, e.g. foo->bar->baz()
*/
protected def visitUnresolvedDynamicCall(c: Call): Unit = {
// If we know the type of the method's first parameter, use that to
// determine the method scope.
// TODO: Are there methods / instances where this doesn't work? Static methods?
// TODO: What if the first parameter could take multiple types?
// TODO: Test on nested dynamic calls, e.g. foo->bar->baz()

if (c.argument.exists(_.argumentIndex == 0)) {
val p = c.argument(0)
p match {
c.argument(0) match {
case p: Identifier => {
val ts = (p.typeFullName +: p.dynamicTypeHintFullName)
.filterNot(_.equals("ANY"))
.distinct
ts match {
case Seq() =>
case Seq(t) => {
// Need to update the call node method full name and dynamic type hint full name
// Or maybe just the dynamic type hint full name?
val newFullName = t + "->" + c.name
builder.setNodeProperty(c, PropertyNames.METHOD_FULL_NAME, newFullName)
builder.setNodeProperty(
Expand All @@ -434,15 +279,11 @@ private class RecoverForPhpFile(cpg: Cpg, cu: NamespaceBlock, builder: DiffGraph
)
builder.setNodeProperty(c, PropertyNames.DYNAMIC_TYPE_HINT_FULL_NAME, Seq.empty)
}
case _ => { /* TODO */ }
case _ => { /* TODO: case where multiple possible types are identified */ }
}
}
case _ =>
}
}
}

override protected def postSetTypeInformation(): Unit = {
unresolvedDynamicCalls.foreach(visitUnresolvedDynamicCall)
}
}
18 changes: 7 additions & 11 deletions ...i/frontends/php2cpg/src/test/scala/io/joern/php2cpg/passes/PhpTypeRecoveryPassTests.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ import io.shiftleft.semanticcpg.language._

class PhpTypeRecoveryPassTests extends PhpCode2CpgFixture() {

/* TODO: Future tests to specify correct type recovery behaviors:
* - Method call inherited from a super class should be recovered
* - A type hint on a parameter should be sufficient to resolve method full names at calls
* - Parameter types on builtins with variadic parameters
*/

"literals declared from built-in types" should {
lazy val cpg = code("""
|<?php
Expand Down Expand Up @@ -361,9 +367,6 @@ class PhpTypeRecoveryPassTests extends PhpCode2CpgFixture() {
|}
""".stripMargin).cpg

/* This seems like it's outside of the type recovery pass? More appropriate
* in the AST generation?
*/
"be properly resolved when called with $this" in {
val List(fooCall) = cpg.method("bar").ast.isCall.take(1).l
fooCall.methodFullName shouldBe "ClassA->foo"
Expand Down Expand Up @@ -419,11 +422,4 @@ class PhpTypeRecoveryPassTests extends PhpCode2CpgFixture() {
barMethod.methodReturn.dynamicTypeHintFullName shouldBe Seq("int")
}
}

/* TODO: remaining tests:
* - Method call inherited from a super class should be recovered
* - A type hint on a parameter should be sufficient to resolve method full names at calls
* - Parameter types on builtins with variadic parameters
*/

}
}

0 comments on commit 335a14c

Please sign in to comment.