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

Add generic dataflow framework + example + stack value analysis #118

Draft
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

montyly
Copy link
Member

@montyly montyly commented Dec 14, 2022

  • Add AbstractDataflow, which is a generic dataflow analysis, following an abstract interpretation approach.
  • Add CollectInstruction, which is just an example. The analysis collects all the instructions executed
  • Add StackValue, which track the values push/pop on the stack, with a focus on int/global field/transaction field

Build on top of 116

This is a draft. Todo list:

  • Implement missing opcode handlers for important operations
  • Add more tests (ex: to stress the merging operations)

Once this is done, the next step will be to increase the abstract value to also track light equation (ex: Eq(1, GroupSize)), and propagating them while collecting logical operations (or, and)

- Add AbstractDataflow, which is a generic dataflow analysis, following an abstract interpretation approach.
- Add CollectInstruction, which is just an example. The analysis collects all the instructions executed
- Add StackValue, which track the values push/pop on the stack, with a focus on int/global field/transaction field

while values and values[0] == StackValue(TOP()):
values.pop()

Copy link
Contributor

Choose a reason for hiding this comment

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

is this supposed to be

while values and values[0] == StackValue(Top()):
    values.pop(0)

Or

while values and values[-1] == StackValue(Top()):
    values.pop()

I'm guessing it's the first one (?)

poped_values = list(self.values)
return Stack([]), [StackValue(TOP()) for _ in range(diff)] + poped_values

poped_values = self.values[:-number]
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this line be poped_values = self.values[-number:] (?)

v2 = stack.values

min_length = min(len(v1), len(v2))
v1 = v1[:-min_length]
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this lines be

v1 = v1[-min_length:]
v2 = v2[-min_length:]

self.ins_out: Dict[Instruction, Stack] = defaultdict(lambda: Stack([]))

def _merge_predecessor(self, bb: BasicBlock) -> Stack:
s = Stack([])
Copy link
Contributor

Choose a reason for hiding this comment

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

Stack union depends on the length of inputs.

    def union(self, stack: "Stack") -> "Stack":

        v1 = self.values
        v2 = stack.values

        min_length = min(len(v1), len(v2))
        v1 = v1[:-min_length]
        v2 = v2[:-min_length]

        v3 = []
        for i in range(min_length):
            v3.append(v1[i].union(v2[i]))

        return Stack(v3)

Because initial s = Stack([]) has length 0, output of union operation will be empty stack. _merge_predecessor will always result in empty stack.
Correct approach would be:

if not bb.prev: # zero predecessors
    return Stack([])

s = self.bb_out[bb.prev[0]]
for bb_prev in bb.prev[1:]:
    s = s.union(self.bb_out[bb_prev])

return s


def __hash__(self) -> int:
return hash(str(self))

Copy link
Contributor

Choose a reason for hiding this comment

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

^^ Won't this approach result in errors when transaction fields from gtxn instruction are tracked alongside txn instruction?

  1. txn RekeyTo
  2. gtxn 3 RekeyTo

RekeyTo from 1 and 2 would have same str(self) and type. They will be considered equal during stack value analysis. txn RekeyTo and gtxn 3 RekeyTo does not necessarily be equal and most of the times will not be equal.

I think we need a better way to compare and hash TransactionField objects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants