Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Constant variable substitution, loop unroll pass + dpor code cleanup #202

Merged
merged 9 commits into from
Sep 10, 2023
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this change necessary? How come this used to be so complicated, and now an ID equivalence is enough?

Copy link
Contributor Author

@csanadtelbisz csanadtelbisz Sep 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I have no idea why this was so complicated beforehand. I can see no reason for the ID equivalence check being an incorrect way of determining the equivalence of ArgNodes as long as IDs are unique (if we compare ArgNodes from different ARGs, it may be wrong; though I do not see such a use case). On the other hand, the former complicated way resulted in a depth-first ARG search from the given ArgNode (cf. line 284 in the old version comparing outEdges and line 33 comparing the targets of ArgEdges).
(We had a conversation about this issue on the 15th of June; also with @AdamZsofi).

Copy link
Contributor Author

@csanadtelbisz csanadtelbisz Sep 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on git blame, you are the author of the original equivalence check, so you may have the reasons for implementing equivalence check this way.
e9defab (fixed link)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, I've made these changes in e9defab. The reason for them was that even if you fully re-build an ARG from a model a second time, due to mismatched IDs, the two ARG's comparison would fail. I think that with this simplified version, the problem would reappear, and we'd have some problems with ARG comparisons, e.g., in the progress check of @AdamZsofi. Of course we can mitigate it some other way, I don't think it's acceptable that a DFS search should run at every ArgNode comparison.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this topic to our next in-person meeting. I think we will need to come up with a solution that neither breaks existing functionality nor does it place such a burden on performance.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(also, @AdamZsofi: some tests would be nice, so that similar changes are reflected in the CT pipeline's success!)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I omit this change from this PR but I have created the issue #203 about ArgNode comparison.

Original file line number Diff line number Diff line change
Expand Up @@ -278,11 +278,7 @@ public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
ArgNode<?, ?> argNode = (ArgNode<?, ?>) o;
return depth == argNode.depth &&
state.equals(argNode.state) &&
coveringNode.equals(argNode.coveringNode) &&
Set.copyOf(outEdges).equals(Set.copyOf(argNode.outEdges)) &&
target == argNode.target;
return id == argNode.id;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
package hu.bme.mit.theta.analysis.expl;

import hu.bme.mit.theta.analysis.stmtoptimizer.StmtOptimizer;
import hu.bme.mit.theta.analysis.stmtoptimizer.StmtSimplifier;
import hu.bme.mit.theta.core.utils.StmtSimplifier;
import hu.bme.mit.theta.core.stmt.Stmt;

public class ExplStmtOptimizer implements StmtOptimizer<ExplState> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
package hu.bme.mit.theta.analysis.pred;

import hu.bme.mit.theta.analysis.stmtoptimizer.StmtOptimizer;
import hu.bme.mit.theta.analysis.stmtoptimizer.StmtSimplifier;
import hu.bme.mit.theta.core.utils.StmtSimplifier;
import hu.bme.mit.theta.core.model.ImmutableValuation;
import hu.bme.mit.theta.core.stmt.Stmt;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

package hu.bme.mit.theta.analysis.stmtoptimizer;
package hu.bme.mit.theta.core.utils;

import com.google.common.collect.ImmutableList;
import hu.bme.mit.theta.core.decl.Decl;
Expand All @@ -38,7 +38,6 @@
import hu.bme.mit.theta.core.type.booltype.BoolLitExpr;
import hu.bme.mit.theta.core.type.booltype.BoolType;
import hu.bme.mit.theta.core.type.inttype.IntLitExpr;
import hu.bme.mit.theta.core.utils.ExprUtils;

import java.math.BigInteger;
import java.util.ArrayList;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package hu.bme.mit.theta.analysis.utils;
package hu.bme.mit.theta.core.utils;

import static com.google.common.collect.ImmutableSet.of;
import static hu.bme.mit.theta.core.decl.Decls.Var;
Expand All @@ -31,7 +31,7 @@
import java.util.Set;

import com.google.common.collect.ImmutableList;
import hu.bme.mit.theta.analysis.stmtoptimizer.StmtSimplifier;
import hu.bme.mit.theta.core.utils.StmtSimplifier;
import hu.bme.mit.theta.core.stmt.NonDetStmt;
import hu.bme.mit.theta.core.stmt.SequenceStmt;
import org.junit.Test;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ open class XcfaDporLts(private val xcfa: XCFA) : LTS<S, A> {

companion object {

var random: Random = Random.Default // use Random(seed) with a seed or Random.Default without seed
var random: Random =
Random.Default // use Random(seed) with a seed or Random.Default without seed

/**
* Simple LTS that returns the enabled actions in a state.
Expand All @@ -92,8 +93,9 @@ open class XcfaDporLts(private val xcfa: XCFA) : LTS<S, A> {
* Partial order of states considering sleep sets (unexplored behavior).
*/
fun <E : ExprState> getPartialOrder(partialOrd: PartialOrd<E>) = PartialOrd<E> { s1, s2 ->
partialOrd.isLeq(s1, s2) && s2.reExplored == true && s1.sleep.containsAll(
s2.sleep - s2.explored)
partialOrd.isLeq(
s1, s2
) && s2.reExplored == true && s1.sleep.containsAll(s2.sleep - s2.explored)
}
}

Expand Down Expand Up @@ -162,7 +164,8 @@ open class XcfaDporLts(private val xcfa: XCFA) : LTS<S, A> {
// lazy pruning: goes to the root when the stack is empty
while (stack.isEmpty() && node.parent.isPresent) node = node.parent.get()

node.state.reExplored = true // lazy pruning: indicates that the state is explored in the current iteration
node.state.reExplored =
true // lazy pruning: indicates that the state is explored in the current iteration
push(node, stack.size)
}

Expand All @@ -188,15 +191,16 @@ open class XcfaDporLts(private val xcfa: XCFA) : LTS<S, A> {
// when lazy pruning is used the explored parts from previous iterations are reexplored to detect possible races
exploreLazily()
}
while (stack.isNotEmpty() &&
(last.node.isSubsumed || (last.node.isExpanded && last.backtrack.subtract(
last.sleep).isEmpty()))
while (stack.isNotEmpty() && (last.node.isSubsumed || (last.node.isExpanded && last.backtrack.subtract(
last.sleep
).isEmpty()))
) { // until we need to pop (the last is covered or not feasible, or it has no more actions that need to be explored
if (stack.size >= 2) {
val lastButOne = stack[stack.size - 2]
val mutexNeverReleased = last.mutexLocks.containsKey("") &&
(last.state.mutexes.keys subtract lastButOne.state.mutexes.keys).contains(
"")
val mutexNeverReleased =
last.mutexLocks.containsKey("") && (last.state.mutexes.keys subtract lastButOne.state.mutexes.keys).contains(
""
)
if (last.node.explored.isEmpty() || mutexNeverReleased) {
// if a mutex is never released another action (practically all the others) have to be explored
lastButOne.backtrack = lastButOne.state.enabled.toMutableSet()
Expand Down Expand Up @@ -238,12 +242,12 @@ open class XcfaDporLts(private val xcfa: XCFA) : LTS<S, A> {
val newaction = item.inEdge.get().action
val process = newaction.pid

val newProcessLastAction = last.processLastAction.toMutableMap()
.apply { this[process] = stack.size }
var newLastDependents = (last.lastDependents[process]?.toMutableMap()
?: mutableMapOf()).apply {
this[process] = stack.size
}
val newProcessLastAction =
last.processLastAction.toMutableMap().apply { this[process] = stack.size }
var newLastDependents =
(last.lastDependents[process]?.toMutableMap() ?: mutableMapOf()).apply {
this[process] = stack.size
}
val relevantProcesses = (newProcessLastAction.keys - setOf(process)).toMutableSet()

// Race detection
Expand All @@ -253,8 +257,7 @@ open class XcfaDporLts(private val xcfa: XCFA) : LTS<S, A> {

val action = node.inEdge.get().action
if (relevantProcesses.contains(action.pid)) {
if (newLastDependents.containsKey(
action.pid) && index <= checkNotNull(newLastDependents[action.pid])) {
if (newLastDependents.containsKey(action.pid) && index <= newLastDependents[action.pid]!!) {
// there is an action a' such that action -> a' -> newaction (->: happens-before)
relevantProcesses.remove(action.pid)
} else if (dependent(newaction, action)) {
Expand All @@ -272,8 +275,8 @@ open class XcfaDporLts(private val xcfa: XCFA) : LTS<S, A> {
}

newLastDependents[action.pid] = index
newLastDependents = max(newLastDependents,
checkNotNull(stack[index].lastDependents[action.pid]))
newLastDependents =
max(newLastDependents, stack[index].lastDependents[action.pid]!!)
relevantProcesses.remove(action.pid)
}
}
Expand All @@ -289,13 +292,14 @@ open class XcfaDporLts(private val xcfa: XCFA) : LTS<S, A> {
if (!item.state.isBottom) {
releasedMutexes.forEach { m ->
last.mutexLocks[m]?.let {
stack[it].mutexLocks.remove(m)
stack[it].mutexLocks.remove(
m
)
}
}
}

val isCoveringNode = item.parent.get() != last.node
val isVirtualExploration = virtualLimit < stack.size || isCoveringNode
val isVirtualExploration = virtualLimit < stack.size || item.parent.get() != last.node
val newSleep = if (isVirtualExploration) {
item.state.sleep
} else {
Expand Down Expand Up @@ -327,13 +331,11 @@ open class XcfaDporLts(private val xcfa: XCFA) : LTS<S, A> {
stack.push(
StackItem(
node = item,
processLastAction = if (!isCoveringNode) newProcessLastAction else last.processLastAction.toMutableMap(),
processLastAction = newProcessLastAction,
lastDependents = last.lastDependents.toMutableMap().apply {
if (!isCoveringNode) {
this[process] = newLastDependents
newProcesses.forEach {
this[it] = max(this[it] ?: mutableMapOf(), newLastDependents)
}
this[process] = newLastDependents
newProcesses.forEach {
this[it] = max(this[it] ?: mutableMapOf(), newLastDependents)
}
},
mutexLocks = last.mutexLocks.toMutableMap().apply {
Expand All @@ -360,7 +362,9 @@ open class XcfaDporLts(private val xcfa: XCFA) : LTS<S, A> {

if (node != visiting) {
if (!push(visiting, startStackSize) || noInfluenceOnRealExploration(
realStackSize)) continue
realStackSize
)
) continue
}

// visiting is not on the stack: no cycle && further virtual exploration can influence real exploration
Expand Down Expand Up @@ -398,21 +402,21 @@ open class XcfaDporLts(private val xcfa: XCFA) : LTS<S, A> {
*/
private fun max(map1: Map<Int, Int>, map2: Map<Int, Int>) =
(map1.keys union map2.keys).associateWith { key ->
max(map1[key] ?: -1, map2[key] ?: -1)
max(
map1[key] ?: -1, map2[key] ?: -1
)
}.toMutableMap()

/**
* See the article for the definition of notdep.
*/
private fun notdep(start: Int, action: A): List<A> {
val e = stack[start].action
return stack.slice(start + 1 until stack.size)
.filterIndexed { index, item ->
item.node.parent.get() == stack[start + 1 + index - 1].node && !dependent(e,
item.action)
}
.map { it.action }
.toMutableList().apply { add(action) }
return stack.slice(start + 1 until stack.size).filterIndexed { index, item ->
item.node.parent.get() == stack[start + 1 + index - 1].node && !dependent(
e, item.action
)
}.map { it.action }.toMutableList().apply { add(action) }
}

/**
Expand All @@ -435,13 +439,12 @@ open class XcfaDporLts(private val xcfa: XCFA) : LTS<S, A> {
/**
* Returns true when the virtual exploration cannot detect any more races relevant in the "real" exploration (the part of the search stack before the first covering node).
*/
private fun noInfluenceOnRealExploration(
realStackSize: Int) = last.processLastAction.keys.all { process ->
last.lastDependents.containsKey(
process) && checkNotNull(last.lastDependents[process]).all { (otherProcess, index) ->
index >= realStackSize || index >= checkNotNull(last.processLastAction[otherProcess])
private fun noInfluenceOnRealExploration(realStackSize: Int) =
last.processLastAction.keys.all { process ->
last.lastDependents.containsKey(process) && last.lastDependents[process]!!.all { (_, index) ->
index >= realStackSize
}
}
}
}

/**
Expand All @@ -453,8 +456,7 @@ open class XcfaDporLts(private val xcfa: XCFA) : LTS<S, A> {
val aGlobalVars = a.edge.getGlobalVars(xcfa)
val bGlobalVars = b.edge.getGlobalVars(xcfa)
// dependent if they access the same variable (at least one write)
return (aGlobalVars.keys intersect bGlobalVars.keys)
.any { aGlobalVars[it].isWritten || bGlobalVars[it].isWritten }
return (aGlobalVars.keys intersect bGlobalVars.keys).any { aGlobalVars[it].isWritten || bGlobalVars[it].isWritten }
}
}

Expand All @@ -471,8 +473,7 @@ class XcfaAadporLts(private val xcfa: XCFA) : XcfaDporLts(xcfa) {
/**
* Returns actions to be explored from the given state considering the given precision.
*/
override fun <P : Prec> getEnabledActionsFor(state: S, exploredActions: Collection<A>,
prec: P): Set<A> {
override fun <P : Prec> getEnabledActionsFor(state: S, exploredActions: Collection<A>, prec: P): Set<A> {
this.prec = prec
return getEnabledActionsFor(state)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import hu.bme.mit.theta.xcfa.cli.witnesses.XcfaTraceConcretizer
import hu.bme.mit.theta.xcfa.model.XCFA
import hu.bme.mit.theta.xcfa.model.toDot
import hu.bme.mit.theta.xcfa.passes.LbePass
import hu.bme.mit.theta.xcfa.passes.LoopUnrollPass
import org.antlr.v4.runtime.CharStreams
import java.io.File
import java.io.FileInputStream
Expand Down Expand Up @@ -76,6 +77,9 @@ class XcfaCli(private val args: Array<String>) {
description = "Level of LBE (NO_LBE, LBE_LOCAL, LBE_SEQ, LBE_FULL)")
var lbeLevel: LbePass.LbeLevel = LbePass.LbeLevel.LBE_SEQ

@Parameter(names = ["--unroll"], description = "Max number of loop iterations to unroll")
var loopUnroll: Int = 50

@Parameter(names = ["--input-type"], description = "Format of the input")
var inputType: InputType = InputType.C

Expand Down Expand Up @@ -164,13 +168,14 @@ class XcfaCli(private val args: Array<String>) {
val gsonForOutput = getGson()
val logger = ConsoleLogger(logLevel)
val explicitProperty: ErrorDetection = getExplicitProperty()
registerAllSolverManagers(solverHome, logger)

// propagating input variables
LbePass.level = lbeLevel
if (randomSeed >= 0) XcfaDporLts.random = Random(randomSeed)
LoopUnrollPass.UNROLL_LIMIT = loopUnroll
WebDebuggerLogger.getInstance().setTitle(input?.name)


logger.write(Logger.Level.INFO, "Parsing the input $input as $inputType")
val parseContext = ParseContext()
val xcfa = getXcfa(logger, explicitProperty, parseContext)
Expand All @@ -187,7 +192,6 @@ class XcfaCli(private val args: Array<String>) {
// verification
stopwatch.reset().start()
logger.write(Logger.Level.INFO, "Starting verification of ${xcfa.name} using $backend")
registerAllSolverManagers(solverHome, logger)
val config = parseConfigFromCli()
if (strategy != Strategy.PORTFOLIO && printConfigFile != null) {
printConfigFile!!.writeText(gsonForOutput.toJson(config))
Expand Down
2 changes: 2 additions & 0 deletions subprojects/xcfa/xcfa/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,6 @@ dependencies {
implementation(project(":theta-core"))
implementation(project(":theta-grammar"))
implementation(project(":theta-c-frontend"))
implementation(project(":theta-analysis"))
implementation(project(":theta-solver"))
}
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ private fun List<VarAccessMap>.mergeAndCollect(): VarAccessMap = this.fold(mapOf
* Returns the list of accessed variables by the label.
* The variable is associated with true if the variable is written and false otherwise.
*/
private fun XcfaLabel.collectVarsWithAccessType(): VarAccessMap = when (this) {
internal fun XcfaLabel.collectVarsWithAccessType(): VarAccessMap = when (this) {
is StmtLabel -> {
when (stmt) {
is HavocStmt<*> -> mapOf(stmt.varDecl to WRITE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class XcfaBuilder @JvmOverloads constructor(
}

fun addEntryPoint(toAdd: XcfaProcedureBuilder, params: List<Expr<*>>) {
procedures.add(toAdd)
addProcedure(toAdd)
initProcedures.add(Pair(toAdd, params))
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
package hu.bme.mit.theta.xcfa.passes

import com.google.common.base.Preconditions
import hu.bme.mit.theta.core.stmt.AssumeStmt
import hu.bme.mit.theta.core.type.booltype.FalseExpr
import hu.bme.mit.theta.frontend.ParseContext
import hu.bme.mit.theta.xcfa.collectVars
import hu.bme.mit.theta.xcfa.getAtomicBlockInnerLocations
Expand Down Expand Up @@ -303,8 +305,8 @@ class LbePass(val parseContext: ParseContext) : ProcedurePass {
*/
private fun isNotLocal(edge: XcfaEdge): Boolean {
return !edge.getFlatLabels().all { label ->
!(label is StartLabel || label is JoinLabel) && label.collectVars()
.all(builder.getVars()::contains)
!(label is StartLabel || label is JoinLabel) && label.collectVars().all(builder.getVars()::contains) &&
!(label is StmtLabel && label.stmt is AssumeStmt && label.stmt.cond is FalseExpr)
}
}
}
Loading
Loading