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

crucible-jvm: Add permission bit for writability to each instance field and array. #735

Merged
merged 5 commits into from
May 14, 2021

Conversation

brianhuffman
Copy link
Contributor

The field permission bit is asserted on every putfield instruction. The array permission bit is asserted on every astore instruction.

This PR builds on top of #733. It will be used to address GaloisInc/saw-script#900.

@brianhuffman brianhuffman requested a review from robdockins May 12, 2021 16:40
brianhuffman pushed a commit to GaloisInc/saw-script that referenced this pull request May 12, 2021
brianhuffman pushed a commit to GaloisInc/saw-script that referenced this pull request May 12, 2021
Copy link
Contributor

@robdockins robdockins left a comment

Choose a reason for hiding this comment

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

Looks OK to me. Two questions though:

  1. Why did you decide to use a separate StringMap for object field permissions instead of adding a boolean flag to the field value map?

  2. It seems like objects always default to writable, which makes sense for crux-style simulation. How does SAW twiddle those bits?

@brianhuffman
Copy link
Contributor Author

  1. Why did you decide to use a separate StringMap for object field permissions instead of adding a boolean flag to the field value map?

That choice is mainly because the object fields are often updated during simulation while the permission bits never are. By keeping them separate, we can be sure that the values in the permission map will stay concrete, even in the presence of conditional updates to the value fields. I mentioned some of this in response to some comments on #733.

  1. It seems like objects always default to writable, which makes sense for crux-style simulation. How does SAW twiddle those bits?

SAW sets those bits when an object is created (while executing the pre-state section of a method spec during verification) and then never changes them afterward. Fields are only set to writable if there is a jvm_modifies_field or jvm_field_is declaration in the post-state section; otherwise we interpret the spec as asserting that the field does not change, and so we set the field to read-only.

@brianhuffman brianhuffman force-pushed the jvm-instance-field-writable branch from 00ba356 to d6ab057 Compare May 14, 2021 19:41
Brian Huffman added 5 commits May 14, 2021 14:13
The permission bit is asserted at every `putfield` instruction.
The `astore` instruction now checks the write permission bit.

Add writability parameter to `doAllocateArray`.
@brianhuffman brianhuffman force-pushed the jvm-instance-field-writable branch from d6ab057 to 6dd6c06 Compare May 14, 2021 23:37
@brianhuffman brianhuffman changed the title crucible-jvm: Add permission bit for writability to each static field and array. crucible-jvm: Add permission bit for writability to each instance field and array. May 14, 2021
@brianhuffman brianhuffman merged commit 31e3976 into master May 14, 2021
@brianhuffman brianhuffman deleted the jvm-instance-field-writable branch May 14, 2021 23:44
brianhuffman pushed a commit to GaloisInc/saw-script that referenced this pull request May 14, 2021
brianhuffman pushed a commit to GaloisInc/saw-script that referenced this pull request May 17, 2021
brianhuffman pushed a commit to GaloisInc/saw-script that referenced this pull request May 19, 2021
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