Skip to content

Commit

Permalink
misc fixes in passes for build
Browse files Browse the repository at this point in the history
 * remove ConcurrentWriterCpgPass which we thought we would need
 * adjust ForkJoinParallelCpgPassNewTests timeout tests
 * remove all overflowdb usage
  • Loading branch information
tuxology committed Aug 7, 2024
1 parent a9ecde9 commit ddb766b
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 285 deletions.
4 changes: 2 additions & 2 deletions build.sbt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name := "codepropertygraph"

// parsed by project/Versions.scala, updated by updateDependencies.sh
val flatgraphVersion = "0.1.2"
val flatgraphVersion = "0.1.3"

inThisBuild(
List(
Expand Down Expand Up @@ -83,4 +83,4 @@ credentials +=
"maven.pkg.github.com",
"Privado-Inc",
sys.env.getOrElse("GITHUB_TOKEN", "N/A")
)
)
23 changes: 7 additions & 16 deletions codepropertygraph/src/main/scala/io/shiftleft/passes/CpgPass.scala
Original file line number Diff line number Diff line change
Expand Up @@ -51,23 +51,9 @@ abstract class CpgPass(cpg: Cpg, outName: String = "") extends ForkJoinParallelC
* methods. This may be better than using the constructor or GC, because e.g. SCPG chains of passes construct
* passes eagerly, and releases them only when the entire chain has run.
* */
abstract class ForkJoinParallelCpgPass[T <: AnyRef](cpg: Cpg, @nowarn outName: String = "") extends CpgPassBase {
type DiffGraphBuilder = io.shiftleft.codepropertygraph.generated.DiffGraphBuilder
// generate Array of parts that can be processed in parallel
def generateParts(): Array[? <: AnyRef]
// setup large data structures, acquire external resources
def init(): Unit = {}
// release large data structures and external resources
def finish(): Unit = {}
// main function: add desired changes to builder
def runOnPart(builder: DiffGraphBuilder, part: T): Unit
// Override this to disable parallelism of passes. Useful for debugging.
def isParallel: Boolean = true

abstract class ForkJoinParallelCpgPassWithTimeout[T <: AnyRef](
cpg: Cpg,
@nowarn outName: String = "",
keyPool: Option[KeyPool] = None,
timeout: Long = -1
) extends NewStyleCpgPassBaseWithTimeout[T](timeout) {

Expand Down Expand Up @@ -111,6 +97,11 @@ abstract class ForkJoinParallelCpgPassWithTimeout[T <: AnyRef](
}
}

@deprecated("Please use createAndApply")
override def createApplySerializeAndStore(serializedCpg: SerializedCpg, prefix: String = ""): Unit = {
createAndApply()
}

}

abstract class ForkJoinParallelCpgPass[T <: AnyRef](cpg: Cpg, @nowarn outName: String = "") extends CpgPassBase {
Expand Down Expand Up @@ -213,7 +204,7 @@ abstract class ForkJoinParallelCpgPass[T <: AnyRef](cpg: Cpg, @nowarn outName: S
}

abstract class NewStyleCpgPassBaseWithTimeout[T <: AnyRef](timeout: Long) extends CpgPassBase {
type DiffGraphBuilder = overflowdb.BatchedUpdate.DiffGraphBuilder
type DiffGraphBuilder = io.shiftleft.codepropertygraph.generated.DiffGraphBuilder

// generate Array of parts that can be processed in parallel
def generateParts(): Array[? <: AnyRef]
Expand All @@ -232,7 +223,7 @@ abstract class NewStyleCpgPassBaseWithTimeout[T <: AnyRef](timeout: Long) extend

override def createAndApply(): Unit = createApplySerializeAndStore(null)

override def runWithBuilder(externalBuilder: BatchedUpdate.DiffGraphBuilder): Int = {
override def runWithBuilder(externalBuilder: DiffGraphBuilder): Int = {
try {
init()
val parts = generateParts()
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,115 +4,20 @@ import better.files.File
import io.shiftleft.SerializedCpg
import io.shiftleft.codepropertygraph.generated.Cpg
import io.shiftleft.codepropertygraph.generated.Properties
import io.shiftleft.codepropertygraph.generated.language.*
import io.shiftleft.codepropertygraph.generated.nodes.{NewCall, NewFile}
import org.scalatest.matchers.should.Matchers
import org.scalatest.wordspec.AnyWordSpec
import overflowdb.traversal._

import java.nio.file.Files
import scala.jdk.CollectionConverters._

class ParallelCpgPassNewTests extends AnyWordSpec with Matchers {

private object Fixture {
def apply(keyPools: Option[Iterator[KeyPool]] = None)(f: (Cpg, CpgPassBase) => Unit): Unit = {
val cpg = Cpg.empty
val pool = keyPools.flatMap(_.nextOption())
class MyPass(cpg: Cpg) extends ConcurrentWriterCpgPass[String](cpg, "MyPass", pool) {
override def generateParts(): Array[String] = Array("foo", "bar")

override def runOnPart(diffGraph: DiffGraphBuilder, part: String): Unit = {
diffGraph.addNode(NewFile().name(part))
}
}
val pass = new MyPass(cpg)
f(cpg, pass)
}
}

"ConcurrentWriterCpgPass" should {
"allow creating and applying result of pass" in Fixture() { (cpg, pass) =>
pass.createAndApply()
cpg.graph.nodes.map(_.property(Properties.Name)).toSetMutable shouldBe Set("foo", "bar")
}

"produce a serialized CPG file" in Fixture() { (_, pass) =>
File.usingTemporaryFile("pass", ".zip") { file =>
file.delete()
val filename = file.path.toString
val serializedCpg = new SerializedCpg(filename)
pass.createApplySerializeAndStore(serializedCpg)
serializedCpg.close()
file.exists shouldBe true
Files.size(file.path) should not be 0
}
}

val keyPools = Iterator(new IntervalKeyPool(10, 20), new IntervalKeyPool(30, 40))

"use only the first KeyPool for createAndApply" in Fixture(Some(keyPools)) { (cpg, pass) =>
pass.createAndApply()
cpg.graph.V.asScala.map(_.id()).toSet shouldBe Set(10, 11)
}

"fail for schema violations" in {
val cpg = Cpg.empty
val pass = new ConcurrentWriterCpgPass[String](cpg, "pass2") {
override def generateParts() = Array("a", "b")
override def runOnPart(diffGraph: DiffGraphBuilder, part: String): Unit =
part match {
case "a" =>
// this is fine
diffGraph.addNode(NewFile().name(part))
case "b" =>
// schema violation
val file1 = NewFile().name("foo")
val file2 = NewFile().name("bar")
diffGraph
.addNode(file1)
.addNode(file2)
.addEdge(file1, file2, "illegal_edge_label")

}
}

// the above DiffGraph (part "b") is not schema conform, applying it must throw an exception
intercept[Exception] {
pass.createAndApply()
}
}

"add NewNodes that are referenced in different parts only once" in {
val cpg = Cpg.empty
val pass = new ConcurrentWriterCpgPass[String](cpg, "pass2") {
val call1 = NewCall().name("call1")
val call2 = NewCall().name("call2")
val call3 = NewCall().name("call3")

override def generateParts() = Array("a", "b")
override def runOnPart(diffGraph: DiffGraphBuilder, part: String): Unit =
part match {
case "a" =>
diffGraph.addEdge(call1, call2, "AST")
case "b" =>
diffGraph.addEdge(call2, call3, "AST")
}
}
pass.createAndApply()
cpg.graph.nodeCount() shouldBe 3
}
}

}

class ForkJoinParallelCpgPassNewTests extends AnyWordSpec with Matchers {

private object Fixture {
def apply(keyPools: Option[Iterator[KeyPool]] = None, timeout: Long = -1)(f: (Cpg, CpgPassBase) => Unit): Unit = {
val cpg = Cpg.empty
val pool = keyPools.flatMap(_.nextOption())
class MyPass(cpg: Cpg)
extends ForkJoinParallelCpgPassWithTimeout[String](cpg, "MyPass", pool, timeout = timeout) {
def apply(timeout: Long = -1)(f: (Cpg, CpgPassBase) => Unit): Unit = {
val cpg = Cpg.empty
class MyPass(cpg: Cpg) extends ForkJoinParallelCpgPassWithTimeout[String](cpg, "MyPass", timeout = timeout) {
override def generateParts(): Array[String] = Range(1, 101).map(_.toString).toArray

override def runOnPart(diffGraph: DiffGraphBuilder, part: String): Unit = {
Expand All @@ -128,12 +33,12 @@ class ForkJoinParallelCpgPassNewTests extends AnyWordSpec with Matchers {
"ForkJoinParallelPassWithTimeout" should {
"generate partial result in case of timeout" in Fixture(timeout = 2) { (cpg, pass) =>
pass.createAndApply()
assert(cpg.graph.nodes.map(_.property(Properties.Name)).toList.size != 100)
assert(cpg.all.map(_.property(Properties.Name)).toList.size != 100)
}

"generate complete result without timeout" in Fixture() { (cpg, pass) =>
pass.createAndApply()
assert(cpg.graph.nodes.map(_.property(Properties.Name)).toList.size == 100)
assert(cpg.all.map(_.property(Properties.Name)).toList.size == 100)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ object GraphSchema extends flatgraph.Schema {
"UNKNOWN"
)
val nodeKindByLabel = nodeLabels.zipWithIndex.toMap
val edgeLabels: Array[String] = Array(
val edgeLabels = Array(
"ALIAS_OF",
"ARGUMENT",
"AST",
Expand Down Expand Up @@ -236,7 +236,7 @@ object GraphSchema extends flatgraph.Schema {
size => new Array[flatgraph.GNode](size),
size => new Array[flatgraph.GNode](size)
)
val normalNodePropertyNames: Array[String] = Array(
val normalNodePropertyNames = Array(
"ALIAS_TYPE_FULL_NAME",
"ARGUMENT_INDEX",
"ARGUMENT_NAME",
Expand Down

0 comments on commit ddb766b

Please sign in to comment.