Skip to content

Add symbolic execution rules for readByteBF in sparse-bytes.md #73

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

Merged
merged 6 commits into from
Apr 18, 2025

Conversation

Stevengre
Copy link
Contributor

Enhance symbolic execution capabilities by introducing new rules to handle patterns of #bytes(B +Bytes _) _ and #bytes(B +Bytes BS) EF, allowing for more precise value retrieval in readByte.

This update improves the accuracy of symbolic execution in the context of byte reading operations.

@Stevengre Stevengre marked this pull request as ready for review April 15, 2025 12:06
@Stevengre Stevengre self-assigned this Apr 15, 2025
@Stevengre Stevengre requested a review from tothtamas28 April 15, 2025 12:06
@Stevengre Stevengre changed the base branch from add-debug-mode-option to debug-mode April 16, 2025 07:36
@Stevengre Stevengre changed the base branch from debug-mode to add-debug-mode-option April 16, 2025 07:38
@Stevengre Stevengre changed the base branch from add-debug-mode-option to master April 16, 2025 13:43
@Stevengre Stevengre force-pushed the support-symbolic-memory-read branch from 0c77db9 to da88a1f Compare April 16, 2025 13:45
- Enhance symbolic execution capabilities by introducing new rules to handle patterns of `#bytes(B +Bytes _) _` and `#bytes(B +Bytes BS) EF`, allowing for more precise value retrieval in `readByte`.
- Skip test for `lw-spec.k` in `test_specs` due to known issue with implies failure.
@Stevengre Stevengre force-pushed the support-symbolic-memory-read branch from da88a1f to 24cbaff Compare April 16, 2025 13:46
@@ -53,6 +53,9 @@ def test_specs(
temp_dir: Path,
spec_file: Path,
) -> None:
if spec_file.name == 'lw-spec.k':
pytest.skip('Skipping lw-spec.k due to the weird implies failed')
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the issue here specifically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The target node and the final node is syntatically the same. But the prover cannot return a correct result.

Copy link
Contributor Author

@Stevengre Stevengre Apr 16, 2025

Choose a reason for hiding this comment

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

Here is the response from @jberthold:

I ran the last implies request 038 from your bug report, and got the following back (consistent with the response saved in the bug report):

$ jq .result.condition.predicate implies-response.json | ../../.build/kore/bin/pretty definition.kore /dev/stdin 
Pretty-printing predicates:
Bool predicates:
  kseq(
      signExtend( VarY:SortBytes{} [ "3" ] <<Int "8" |Int VarY:SortBytes{} [ "2" ] <<Int "8" |Int VarY:SortBytes{} [ "1" ] <<Int "8" |Int VarY:SortBytes{} [ "0" ] , "32" ),
      dotk
  ) ==K kseq(
      signExtend( VarY:SortBytes{} [ "3" ] <<Int "8" |Int VarY:SortBytes{} [ "2" ] <<Int "8" |Int VarY:SortBytes{} [ "1" ] <<Int "8" |Int VarY:SortBytes{} [ "0" ] , "32" ),
      dotk
  )
Ceil predicates:
  \ceil{}(VarY:SortBytes{} [ "0" ])
  \ceil{}(VarY:SortBytes{} [ "1" ])
  \ceil{}(VarY:SortBytes{} [ "2" ])
  \ceil{}(VarY:SortBytes{} [ "3" ])
Substitution:

In the logs I can see that the request is handled completely by kore (the legacy backend), not by booster.
So the legacy backend says that the implication holds under the printed conditions.

  • The "Bool predicate" that signExtend(thing) == signExtend(thing), which is actually trivial, because thing is syntactically the same term. However, the legacy backend does not accept that to be automatically the same, because of
  • the four \ceil predicates, saying that indexing with 0, 1, 2 and 3 into the Bytes in Y is defined. (so the legacy backend is cautious not to even accept the syntactic identity).

The request does not have any condition about the lengthBytes of Y . Maybe worth a shot adding a condition 3 <=Int length(Y) ? Howeverin the definition.kore I could not find any \ceil rules about BYTES.get (which is the implementation of the indexing _ [ _ ] on bytes.

Another thing that you could try is to add an "assume-defined": true to the parameters, then booster will try to run the implication (booster does not consider ceil requirements). Of course that means you as the caller have to be certain that your Y is long enough (four bytes at least).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But I think that the problem is not caused by the definedness problem, because:

  • I still failed to pass this implies even I use "assume-defined": true
  • I still fiailed after introducing the following simplification rule:
rule #Ceil(@X:Bytes[@I:Int]) => {(lengthBytes(@X) >Int @I) #Equals true} #And #Ceil(@X) #And #Ceil(@I) [simplification]

However, since I don't quite understand how the definedness transforms from / between the terms, I don't know if there any other rules should I introduce.

Additionally, I don't quite understand the preserve-definedness attribute for the simplification rules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Becuase this is not trivial to solve this problem, I want to skip it now and may construct a small example to explore how to solve the problem.

BTW, I think the implies problem is not urgent. Because even we introduced these simplification rules to simplify the read process, it can still be some gap for verifying add here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I want to skip it now

Sure. Please open an issue for tracking purposes.

Copy link
Member

Choose a reason for hiding this comment

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

I want to skip it now

Sure. Please open an issue for tracking purposes.

Yes, please open an issue and I can attach the information that we only have in slack at the moment. We want to a) improve the booster implies endpoint to handle this case, and b) find out why your \ceil simplification does not fire.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for my late reply. Here is the issue to this problem: #80

@Stevengre Stevengre requested a review from tothtamas28 April 16, 2025 15:13
@Stevengre Stevengre merged commit 98eea27 into master Apr 18, 2025
3 checks passed
@Stevengre Stevengre deleted the support-symbolic-memory-read branch April 18, 2025 13:20
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.

4 participants