Skip to content

Commit

Permalink
[XTypeRecovery] Parallelism Fix, Serial Data Structures, CPG Pass For…
Browse files Browse the repository at this point in the history
…malisms (joernio#3886)

This is the final PR in my sequence of XTypeRecovery refactors. It removes all the now-unneeded concurrent datastructures. In includes the previous change-sets joernio#3875 and joernio#3874
---------

Co-authored-by: David Baker Effendi <[email protected]>
  • Loading branch information
bbrehm and DavidBakerEffendi authored Dec 6, 2023
1 parent 5199d29 commit 0b7376e
Show file tree
Hide file tree
Showing 19 changed files with 185 additions and 209 deletions.
2 changes: 1 addition & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name := "joern"
ThisBuild / organization := "io.joern"
ThisBuild / scalaVersion := "3.3.1"

val cpgVersion = "1.4.30"
val cpgVersion = "1.4.32"

lazy val joerncli = Projects.joerncli
lazy val querydb = Projects.querydb
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ case class PythonSrcCpgGenerator(config: FrontendConfig, rootPath: Path) extends
val typeRecoveryConfig = pyConfig match
case Some(config) => XTypeRecoveryConfig(config.typePropagationIterations, !config.disableDummyTypes)
case None => XTypeRecoveryConfig()
new PythonTypeRecoveryPass(cpg, typeRecoveryConfig).createAndApply()
new PythonTypeRecoveryPassGenerator(cpg, typeRecoveryConfig).generate().foreach(_.createAndApply())
new PythonTypeHintCallLinker(cpg).createAndApply()
new NaiveCallLinker(cpg).createAndApply()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import io.joern.javasrc2cpg.passes.{
AstCreationPass,
ConfigFileCreationPass,
JavaTypeHintCallLinker,
JavaTypeRecoveryPass,
JavaTypeRecoveryPassGenerator,
TypeInferencePass
}
import io.joern.x2cpg.X2Cpg.withNewEmptyCpg
Expand Down Expand Up @@ -56,10 +56,9 @@ object JavaSrc2Cpg {
def apply(): JavaSrc2Cpg = new JavaSrc2Cpg()

def typeRecoveryPasses(cpg: Cpg, config: Option[Config] = None): List[CpgPassBase] = {
List(
new JavaTypeRecoveryPass(cpg, XTypeRecoveryConfig(enabledDummyTypes = !config.exists(_.disableDummyTypes))),
new JavaTypeHintCallLinker(cpg)
)
new JavaTypeRecoveryPassGenerator(cpg, XTypeRecoveryConfig(enabledDummyTypes = !config.exists(_.disableDummyTypes)))
.generate() ++
List(new JavaTypeHintCallLinker(cpg))
}

def showEnv(): Unit = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,41 +8,39 @@ import io.shiftleft.codepropertygraph.generated.nodes._
import io.shiftleft.semanticcpg.language._
import overflowdb.BatchedUpdate.DiffGraphBuilder

class JavaTypeRecoveryPass(cpg: Cpg, config: XTypeRecoveryConfig = XTypeRecoveryConfig())
extends XTypeRecoveryPass[Method](cpg, config) {
override protected def generateRecoveryPass(state: XTypeRecoveryState): XTypeRecovery[Method] =
new JavaTypeRecovery(cpg, state)
class JavaTypeRecoveryPassGenerator(cpg: Cpg, config: XTypeRecoveryConfig = XTypeRecoveryConfig())
extends XTypeRecoveryPassGenerator[Method](cpg, config) {
override protected def generateRecoveryPass(state: XTypeRecoveryState, iteration: Int): XTypeRecovery[Method] =
new JavaTypeRecovery(cpg, state, iteration)
}

private class JavaTypeRecovery(cpg: Cpg, state: XTypeRecoveryState) extends XTypeRecovery[Method](cpg, state) {
private class JavaTypeRecovery(cpg: Cpg, state: XTypeRecoveryState, iteration: Int)
extends XTypeRecovery[Method](cpg, state, iteration: Int) {

override def compilationUnit: Iterator[Method] = cpg.method.isExternal(false).iterator
override def compilationUnits: Iterator[Method] = cpg.method.isExternal(false).iterator

override def generateRecoveryForCompilationUnitTask(
unit: Method,
builder: DiffGraphBuilder
): RecoverForXCompilationUnit[Method] = {
val newConfig = state.config.copy(enabledDummyTypes = state.isFinalIteration && state.config.enabledDummyTypes)
new RecoverForJavaFile(cpg, unit, builder, state.copy(config = newConfig))
new RecoverForJavaFile(cpg, unit, builder, state)
}
}

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

private def javaNodeToLocalKey(n: AstNode): Option[LocalKey] = n match {
override protected def fromNodeToLocalKey(n: AstNode): Option[LocalKey] = n match {
case i: Identifier if i.name == "this" && i.code == "super" => Option(LocalVar("super"))
case _ => SBKey.fromNodeToLocalKey(n)
}

override protected val symbolTable = new SymbolTable[LocalKey](javaNodeToLocalKey)

override protected def isConstructor(c: Call): Boolean = isConstructor(c.name)

override protected def isConstructor(name: String): Boolean = !name.isBlank && name.charAt(0).isUpper

override protected def postVisitImports(): Unit = {
symbolTable.view.foreach { case (k, ts) =>
for ((k, ts) <- symbolTable.itemsCopy) {
val tss = ts.filterNot(_.startsWith(Defines.UnresolvedNamespace))
if (tss.isEmpty)
symbolTable.remove(k)
Expand All @@ -59,7 +57,6 @@ private class RecoverForJavaFile(cpg: Cpg, cu: Method, builder: DiffGraphBuilder

override protected def storeCallTypeInfo(c: Call, types: Seq[String]): Unit =
if (types.nonEmpty) {
state.changesWereMade.compareAndSet(false, true)
val signedTypes = types.map {
case t if t.endsWith(c.signature) => t
case t => s"$t:${c.signature}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,10 @@ object JsSrc2Cpg {
val typeRecoveryConfig = config
.map(c => XTypeRecoveryConfig(c.typePropagationIterations, !c.disableDummyTypes))
.getOrElse(XTypeRecoveryConfig())
List(
new JavaScriptInheritanceNamePass(cpg),
new ConstClosurePass(cpg),
new JavaScriptImportResolverPass(cpg),
new JavaScriptTypeRecoveryPass(cpg, typeRecoveryConfig),
new JavaScriptTypeHintCallLinker(cpg),
new NaiveCallLinker(cpg)
)
List(new JavaScriptInheritanceNamePass(cpg), new ConstClosurePass(cpg), new JavaScriptImportResolverPass(cpg))
++
new JavaScriptTypeRecoveryPassGenerator(cpg, typeRecoveryConfig).generate() ++
List(new JavaScriptTypeHintCallLinker(cpg), new NaiveCallLinker(cpg))
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,22 @@ import io.shiftleft.semanticcpg.language.*
import io.shiftleft.semanticcpg.language.operatorextension.OpNodes.FieldAccess
import overflowdb.BatchedUpdate.DiffGraphBuilder

class JavaScriptTypeRecoveryPass(cpg: Cpg, config: XTypeRecoveryConfig = XTypeRecoveryConfig())
extends XTypeRecoveryPass[File](cpg, config) {
override protected def generateRecoveryPass(state: XTypeRecoveryState): XTypeRecovery[File] =
new JavaScriptTypeRecovery(cpg, state)
class JavaScriptTypeRecoveryPassGenerator(cpg: Cpg, config: XTypeRecoveryConfig = XTypeRecoveryConfig())
extends XTypeRecoveryPassGenerator[File](cpg, config) {
override protected def generateRecoveryPass(state: XTypeRecoveryState, iteration: Int): XTypeRecovery[File] =
new JavaScriptTypeRecovery(cpg, state, iteration)
}

private class JavaScriptTypeRecovery(cpg: Cpg, state: XTypeRecoveryState) extends XTypeRecovery[File](cpg, state) {
private class JavaScriptTypeRecovery(cpg: Cpg, state: XTypeRecoveryState, iteration: Int)
extends XTypeRecovery[File](cpg, state, iteration) {

override def compilationUnit: Iterator[File] = cpg.file.iterator
override def compilationUnits: Iterator[File] = cpg.file.iterator

override def generateRecoveryForCompilationUnitTask(
unit: File,
builder: DiffGraphBuilder
): RecoverForXCompilationUnit[File] = {
val newConfig = state.config.copy(enabledDummyTypes = state.isFinalIteration && state.config.enabledDummyTypes)
new RecoverForJavaScriptFile(cpg, unit, builder, state.copy(config = newConfig))
new RecoverForJavaScriptFile(cpg, unit, builder, state)
}

}
Expand Down Expand Up @@ -102,8 +102,8 @@ private class RecoverForJavaScriptFile(cpg: Cpg, cu: File, builder: DiffGraphBui
.name
.toSet

override protected def isField(i: Identifier): Boolean =
state.isFieldCache.getOrElseUpdate(i.id(), exportedIdentifiers.contains(i.name) || super.isField(i))
override protected def isFieldUncached(i: Identifier): Boolean =
exportedIdentifiers.contains(i.name) || super.isFieldUncached(i)

override protected def visitIdentifierAssignedToConstructor(i: Identifier, c: Call): Set[String] = {
val constructorPaths = if (c.methodFullName.endsWith(".alloc")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ package io.joern.jssrc2cpg.passes.ast

import io.joern.jssrc2cpg.passes.AbstractPassTest
import io.joern.x2cpg.Defines
import io.shiftleft.codepropertygraph.generated.Operators
import io.shiftleft.codepropertygraph.generated.nodes.Identifier
import io.shiftleft.codepropertygraph.generated.nodes.MethodRef
import io.shiftleft.codepropertygraph.generated.ModifierTypes
import io.shiftleft.semanticcpg.language._
import io.shiftleft.codepropertygraph.generated.{ModifierTypes, Operators}
import io.shiftleft.codepropertygraph.generated.nodes.{Identifier, MethodRef}
import io.shiftleft.semanticcpg.language.*

class JsClassesAstCreationPassTest extends AbstractPassTest {

Expand All @@ -28,9 +26,9 @@ class JsClassesAstCreationPassTest extends AbstractPassTest {
|function sink(par1) {}
|""".stripMargin) { cpg =>
val List(x1, x2) = cpg.local("x").l
x1._blockViaAstIn should not be empty
x1.parentBlock should not be empty
x1.referencingIdentifiers.name.l shouldBe List("x")
x2._blockViaAstIn should not be empty
x2.parentBlock should not be empty
x2.referencingIdentifiers.name.l shouldBe List("x")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import io.joern.kotlin2cpg.passes.{
ConfigPass,
DependenciesFromMavenCoordinatesPass,
KotlinTypeHintCallLinker,
KotlinTypeRecoveryPass
KotlinTypeRecoveryPassGenerator
}
import io.joern.kotlin2cpg.compiler.{CompilerAPI, ErrorLoggingMessageCollector}
import io.joern.kotlin2cpg.types.{ContentSourcesPicker, DefaultTypeInfoProvider}
Expand All @@ -37,7 +37,7 @@ object Kotlin2Cpg {
type InputProvider = () => InputPair

def postProcessingPass(cpg: Cpg): Unit = {
new KotlinTypeRecoveryPass(cpg).createAndApply()
new KotlinTypeRecoveryPassGenerator(cpg).generate().map { _.createAndApply() }
new KotlinTypeHintCallLinker(cpg).createAndApply()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,35 +9,33 @@ import io.shiftleft.codepropertygraph.generated.nodes.*
import io.shiftleft.semanticcpg.language.*
import overflowdb.BatchedUpdate.DiffGraphBuilder

class KotlinTypeRecoveryPass(cpg: Cpg, config: XTypeRecoveryConfig = XTypeRecoveryConfig())
extends XTypeRecoveryPass[File](cpg, config) {
override protected def generateRecoveryPass(state: XTypeRecoveryState): XTypeRecovery[File] =
new KotlinTypeRecovery(cpg, state)
class KotlinTypeRecoveryPassGenerator(cpg: Cpg, config: XTypeRecoveryConfig = XTypeRecoveryConfig())
extends XTypeRecoveryPassGenerator[File](cpg, config) {
override protected def generateRecoveryPass(state: XTypeRecoveryState, iteration: Int): XTypeRecovery[File] =
new KotlinTypeRecovery(cpg, state, iteration)
}

private class KotlinTypeRecovery(cpg: Cpg, state: XTypeRecoveryState) extends XTypeRecovery[File](cpg, state) {
private class KotlinTypeRecovery(cpg: Cpg, state: XTypeRecoveryState, iteration: Int)
extends XTypeRecovery[File](cpg, state, iteration) {

override def compilationUnit: Iterator[File] = cpg.file.iterator
override def compilationUnits: Iterator[File] = cpg.file.iterator

override def generateRecoveryForCompilationUnitTask(
unit: File,
builder: DiffGraphBuilder
): RecoverForXCompilationUnit[File] = {
val newConfig = state.config.copy(enabledDummyTypes = state.isFinalIteration && state.config.enabledDummyTypes)
new RecoverForKotlinFile(cpg, unit, builder, state.copy(config = newConfig))
new RecoverForKotlinFile(cpg, unit, builder, state)
}
}

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

private def kotlinNodeToLocalKey(n: AstNode): Option[LocalKey] = n match {
override protected def fromNodeToLocalKey(n: AstNode): Option[LocalKey] = n match {
case i: Identifier if i.name == "this" && i.code == "super" => Option(LocalVar("super"))
case _ => SBKey.fromNodeToLocalKey(n)
}

override protected val symbolTable = new SymbolTable[LocalKey](kotlinNodeToLocalKey)

override protected def importNodes: Iterator[Import] = cu.ast.isImport
override protected def visitImport(i: Import): Unit = {

Expand All @@ -54,7 +52,7 @@ private class RecoverForKotlinFile(cpg: Cpg, cu: File, builder: DiffGraphBuilder
override protected def isConstructor(name: String): Boolean = !name.isBlank && name.charAt(0).isUpper

override protected def postVisitImports(): Unit = {
symbolTable.view.foreach { case (k, ts) =>
for ((k, ts) <- symbolTable.itemsCopy) {
val tss = ts.filterNot(_.startsWith(Defines.UnresolvedNamespace))
if (tss.isEmpty)
symbolTable.remove(k)
Expand All @@ -71,7 +69,6 @@ private class RecoverForKotlinFile(cpg: Cpg, cu: File, builder: DiffGraphBuilder

override protected def storeCallTypeInfo(c: Call, types: Seq[String]): Unit =
if (types.nonEmpty) {
state.changesWereMade.compareAndSet(false, true)
val signedTypes = types.map {
case t if t.endsWith(c.signature) => t
case t => s"$t:${c.signature}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import io.joern.php2cpg.passes.{
ClosureRefPass,
LocalCreationPass,
PhpSetKnownTypesPass,
PhpTypeRecoveryPass
PhpTypeRecoveryPassGenerator
}
import io.joern.x2cpg.X2Cpg.withNewEmptyCpg
import io.joern.x2cpg.X2CpgFrontend
Expand Down Expand Up @@ -84,6 +84,6 @@ object Php2Cpg {
val typeRecoveryConfig = config
.map(c => XTypeRecoveryConfig(c.typePropagationIterations, !c.disableDummyTypes))
.getOrElse(XTypeRecoveryConfig(iterations = 3))
List(new PhpSetKnownTypesPass(cpg), new PhpTypeRecoveryPass(cpg, typeRecoveryConfig))
List(new PhpSetKnownTypesPass(cpg)) ++ new PhpTypeRecoveryPassGenerator(cpg, typeRecoveryConfig).generate()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,26 @@ import overflowdb.BatchedUpdate.DiffGraphBuilder
import scala.annotation.tailrec
import scala.collection.mutable

class PhpTypeRecoveryPass(cpg: Cpg, config: XTypeRecoveryConfig = XTypeRecoveryConfig(iterations = 3))
extends XTypeRecoveryPass[NamespaceBlock](cpg, config) {
class PhpTypeRecoveryPassGenerator(cpg: Cpg, config: XTypeRecoveryConfig = XTypeRecoveryConfig(iterations = 3))
extends XTypeRecoveryPassGenerator[NamespaceBlock](cpg, config) {

override protected def generateRecoveryPass(state: XTypeRecoveryState): XTypeRecovery[NamespaceBlock] =
new PhpTypeRecovery(cpg, state)
override protected def generateRecoveryPass(
state: XTypeRecoveryState,
iteration: Int
): XTypeRecovery[NamespaceBlock] =
new PhpTypeRecovery(cpg, state, iteration)
}

private class PhpTypeRecovery(cpg: Cpg, state: XTypeRecoveryState) extends XTypeRecovery[NamespaceBlock](cpg, state) {
private class PhpTypeRecovery(cpg: Cpg, state: XTypeRecoveryState, iteration: Int)
extends XTypeRecovery[NamespaceBlock](cpg, state, iteration) {

override def compilationUnit: Iterator[NamespaceBlock] = cpg.file.namespaceBlock.iterator
override def compilationUnits: Iterator[NamespaceBlock] = cpg.file.namespaceBlock.iterator

override def generateRecoveryForCompilationUnitTask(
unit: NamespaceBlock,
builder: DiffGraphBuilder
): RecoverForXCompilationUnit[NamespaceBlock] = {
val newConfig = state.config.copy(enabledDummyTypes = state.isFinalIteration && state.config.enabledDummyTypes)
new RecoverForPhpFile(cpg, unit, builder, state.copy(config = newConfig))
new RecoverForPhpFile(cpg, unit, builder, state)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,23 @@ import io.shiftleft.semanticcpg.language.operatorextension.OpNodes
import io.shiftleft.semanticcpg.language.operatorextension.OpNodes.FieldAccess
import overflowdb.BatchedUpdate.DiffGraphBuilder

class PythonTypeRecoveryPass(cpg: Cpg, config: XTypeRecoveryConfig = XTypeRecoveryConfig())
extends XTypeRecoveryPass[File](cpg, config) {
class PythonTypeRecoveryPassGenerator(cpg: Cpg, config: XTypeRecoveryConfig = XTypeRecoveryConfig())
extends XTypeRecoveryPassGenerator[File](cpg, config) {

override protected def generateRecoveryPass(state: XTypeRecoveryState): XTypeRecovery[File] =
new PythonTypeRecovery(cpg, state)
override protected def generateRecoveryPass(state: XTypeRecoveryState, iteration: Int): XTypeRecovery[File] =
new PythonTypeRecovery(cpg, state, iteration)
}

private class PythonTypeRecovery(cpg: Cpg, state: XTypeRecoveryState) extends XTypeRecovery[File](cpg, state) {
private class PythonTypeRecovery(cpg: Cpg, state: XTypeRecoveryState, iteration: Int)
extends XTypeRecovery[File](cpg, state, iteration) {

override def compilationUnit: Iterator[File] = cpg.file.iterator
override def compilationUnits: Iterator[File] = cpg.file.iterator

override def generateRecoveryForCompilationUnitTask(
unit: File,
builder: DiffGraphBuilder
): RecoverForXCompilationUnit[File] = {
val newConfig = state.config.copy(enabledDummyTypes = state.isFinalIteration && state.config.enabledDummyTypes)
new RecoverForPythonFile(cpg, unit, builder, state.copy(config = newConfig))
new RecoverForPythonFile(cpg, unit, builder, state)
}

}
Expand All @@ -39,14 +39,12 @@ private class RecoverForPythonFile(cpg: Cpg, cu: File, builder: DiffGraphBuilder
/** Replaces the `this` prefix with the Pythonic `self` prefix for instance methods of functions local to this
* compilation unit.
*/
private def fromNodeToLocalPythonKey(node: AstNode): Option[LocalKey] =
override protected def fromNodeToLocalKey(node: AstNode): Option[LocalKey] =
node match {
case n: Method => Option(CallAlias(n.name, Option("self")))
case _ => SBKey.fromNodeToLocalKey(node)
}

override val symbolTable: SymbolTable[LocalKey] = new SymbolTable[LocalKey](fromNodeToLocalPythonKey)

override def visitImport(i: Import): Unit = {
if (i.importedAs.isDefined && i.importedEntity.isDefined) {

Expand Down Expand Up @@ -97,8 +95,8 @@ private class RecoverForPythonFile(cpg: Cpg, cu: File, builder: DiffGraphBuilder

/** If the parent method is module then it can be used as a field.
*/
override def isField(i: Identifier): Boolean =
state.isFieldCache.getOrElseUpdate(i.id(), i.method.name.matches("(<module>|__init__)") || super.isField(i))
override def isFieldUncached(i: Identifier): Boolean =
i.method.name.matches("(<module>|__init__)") || super.isFieldUncached(i)

override def visitIdentifierAssignedToOperator(i: Identifier, c: Call, operation: String): Set[String] = {
operation match {
Expand Down Expand Up @@ -192,7 +190,6 @@ private class RecoverForPythonFile(cpg: Cpg, cu: File, builder: DiffGraphBuilder
val existingTypes = (identifierTypes ++ otherTypes).distinct
val resolvedTypes = identifierTypes.map(LocalVar.apply).flatMap(symbolTable.get)
if (existingTypes != resolvedTypes && resolvedTypes.nonEmpty) {
state.changesWereMade.compareAndExchange(false, true)
builder.setNodeProperty(t, PropertyNames.INHERITS_FROM_TYPE_FULL_NAME, resolvedTypes)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class PySrcTestCpg extends TestCpg with PythonFrontend {
new PythonImportResolverPass(this).createAndApply()
new PythonInheritanceNamePass(this).createAndApply()
new DynamicTypeHintFullNamePass(this).createAndApply()
new PythonTypeRecoveryPass(this).createAndApply()
new PythonTypeRecoveryPassGenerator(this).generate().foreach(_.createAndApply())
new PythonTypeHintCallLinker(this).createAndApply()
new NaiveCallLinker(this).createAndApply()

Expand Down
Loading

0 comments on commit 0b7376e

Please sign in to comment.