Steensgaard implementation and adding ctx sensitivity at call sites #147
Conversation
src/main/scala/ir/Expr.scala
Outdated
|
|
||
| sealed trait Expr { | ||
| var ssa_id: mutable.Set[Int] = mutable.Set[Int]() |
There was a problem hiding this comment.
Doesn't this only need to be defined on variables? It should also be a val instead since the set itself is mutable.
There was a problem hiding this comment.
Expressions are immutable case classes, so for expressions in different statements to be distinguished this might need to be a real field in the register and local var classes. But there might be a better approach. Currently it seems like the variable "R0" gets the same ssa_id everywhere in the program.
There was a problem hiding this comment.
I will have to add this as a field for the Variable trait as Register and LocalVar are immutable case classes. Would this fix the issue?
| * - Each variable has a set of versions | ||
| * - New assignments create new versions and replaces any new versions | ||
| */ | ||
| object SSAForm { |
There was a problem hiding this comment.
Can we make this a class? Alternatively, far as I can tell all the members are temporary and can be private and cleared when calling applySSA?
| (block, v.name), | ||
| context.getOrElseUpdate((proc, v.name), mutable.Set(getMax(v.name))) | ||
| ) | ||
| }) |
There was a problem hiding this comment.
This seems like its doing a kind of use-def lookup which I don't think will do the right thing without assigning the SSA form indexes in the order of definitions?
There was a problem hiding this comment.
On second look, this is just creating new SSA IDs without caring about the specific definition? In that case it is ok?
There was a problem hiding this comment.
This is meant to create a new SSA ID without relying on any particular order if the variable (register) has not been defined before. If the variable has been defined before in that procedure, then we want to assume the last definition applies. The caller of transformVariables is responsible for controlling the order of processing statements.
|
|
||
| def applySSA(program: Program): Unit = { | ||
| for (proc <- program.procedures) { | ||
| for (block <- proc.blocks) { |
There was a problem hiding this comment.
As above, the order of this is not defined and it may not match cfg order. If we visit a block before its predecessors the ssa ids of the variables won't correspond to earlier assignments. Could maybe just use a stack and and Block.nextBlocks() to iterate the blocks instead.
| override def hashCode(): Int = regionIdentifier.hashCode() * start.hashCode() | ||
| override def equals(obj: Any): Boolean = obj match { | ||
| case s: StackRegion => s.start == start && s.regionIdentifier == regionIdentifier | ||
| case s: StackRegion => s.start == start && s.regionIdentifier.equals(regionIdentifier) |
There was a problem hiding this comment.
You can use == rather than equals, they mean the same thing in Scala. Reference equality is .eq().
src/main/scala/analysis/ANR.scala
Outdated
| m - indirectCall.target | ||
| case localAssign: LocalAssign => | ||
| m = m.diff(localAssign.rhs.variables) | ||
| if (ignoreRegions.contains(localAssign.lhs)) return m |
There was a problem hiding this comment.
return is deprecated so better to use if (_) then m else m + localassign.lhs
|
How much left is there to do here before we can look at merging this? |
|
I am currently writing tests and it will need to be moved to use the IR iterator at some point (working on it in another branch). The Points-to constraints may need to be refined as they are not precisely Steensgaard constraints because they were not meant for Binary-level pointer analysis. The current constraints are a highly modified version of the Steensgaard constraints and work is needed to verify soundness. |
|
Ah, what I'm really interested in everything being moved over to the new iterator, and I was wanting to start work on that, but it's difficult to do so when there's significant outstanding changes to the existing analyses The simplest thing would be to merge this branch into staging once the current merge happening there is finished, and then we can merge all of staging into main. It's not an issue if the points-to and Steengaard analyses aren't perfect yet, just as long as nothing else with the existing analyses has regressed. |
yousifpatti
force-pushed
the
yousif-steensgaard-ctx-sensetive
branch
from
2c7ed92 to
d133b75
Compare
|
Is this ready to merge into staging now? |
|
@l-kent points-to and analyses changes has been merged into staging |
- [ ] Heap SupportIn the coming iterationContext SensitivitySet based SSA