Skip to content

Ensure no Bytes2Int in proof's regs cells to terminate the proof #43

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 8 commits into from
Jun 20, 2025

Conversation

Stevengre
Copy link
Contributor

No description provided.

@Stevengre Stevengre requested a review from tothtamas28 June 19, 2025 09:10
@Stevengre Stevengre self-assigned this Jun 19, 2025
Comment on lines 188 to 189
if isinstance(kinner, KApply) and kinner.label == 'Int2Bytes(_,_,_)_BYTES-HOOKED_Bytes_Int_Int_Endianness':
no_int2bytes_flag = True
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if isinstance(kinner, KApply) and kinner.label == 'Int2Bytes(_,_,_)_BYTES-HOOKED_Bytes_Int_Int_Endianness':
no_int2bytes_flag = True
match kinner:
case KApply(KLabel('Int2Bytes(_,_,_)_BYTES-HOOKED_Bytes_Int_Int_Endianness')):
no_int2bytes_flag = True

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 remember you mentioned a similar suggestion before, but the Python pattern matching didn’t seem to work as intended at the time. I can’t find a suitable test case for this issue right now, so would it be okay to stick with the current implementation for now?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, but I'm pretty sure this will work as intended.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even there are arguments for Int2Bytes?

Copy link
Contributor Author

@Stevengre Stevengre Jun 19, 2025

Choose a reason for hiding this comment

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

Sorry, I'm not familiar with Python's pattern match. But if you are sure about this fact, I can refactor it and see how it works during proving.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just refactored the implementation with pattern matching. But I still want to know more about the Python pattern matching functionality.

…rn matching for `Int2Bytes` collection, enhancing readability and maintainability
return int2bytes_list


def check_proof(proof: APRProof, symtool: SymTools) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider a more expressive name:

Suggested change
def check_proof(proof: APRProof, symtool: SymTools) -> str:
def generate_int2bytes_report(proof: APRProof, symtool: SymTools) -> str:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we might not only check int2bytes, I want to use this name to handle all the checks for the proof.

Copy link
Contributor

Choose a reason for hiding this comment

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

Still, the name is somewhat misleading: it does not check anything (i.e., no assert or bool return value), just generates a textual report. How about generate_report then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, let me refactor it.

…rting of `Int2Bytes` in `regs` cells, improving clarity and performance
Stevengre and others added 2 commits June 20, 2025 19:56
…t`, consolidating proof reporting logic and enhancing clarity
@Stevengre Stevengre requested a review from tothtamas28 June 20, 2025 12:00
@automergerpr-permission-manager automergerpr-permission-manager bot merged commit ed6d043 into master Jun 20, 2025
3 of 4 checks passed
@automergerpr-permission-manager automergerpr-permission-manager bot deleted the jh/check_result branch June 20, 2025 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants