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

Fix overflow checking #3353

Merged
merged 14 commits into from
Jun 24, 2024
Merged

Fix overflow checking #3353

merged 14 commits into from
Jun 24, 2024

Conversation

WolframPfeifer
Copy link
Member

@WolframPfeifer WolframPfeifer commented Nov 15, 2023

The purpose of this PR is to repair the overflow checking of KeY and thus fix #3352.

TODO:

  •  fix creation of branches for overflow checking
  • fix the actual proof obligation of the overflow check (should use inInt(...), inRangeInt(...), taclets in inRules(Un)CheckedSemantics.key)
  • throw an error if a choice used in a taclet does not exist

Copy link

codecov bot commented Nov 15, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 37.86%. Comparing base (1b48b9e) to head (5b68afc).
Report is 73 commits behind head on main.

Current head 5b68afc differs from pull request most recent head fb3a5b1

Please upload reports for the commit fb3a5b1 to get more accurate results.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #3353      +/-   ##
============================================
- Coverage     38.03%   37.86%   -0.18%     
+ Complexity    17089    16904     -185     
============================================
  Files          2099     2055      -44     
  Lines        127274   125528    -1746     
  Branches      21386    21226     -160     
============================================
- Hits          48409    47525     -884     
+ Misses        72892    72152     -740     
+ Partials       5973     5851     -122     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Drodt Drodt added the HacKeYthon Candidate Issue for HacKeYthon '24 label Jan 25, 2024
@WolframPfeifer WolframPfeifer removed the HacKeYthon Candidate Issue for HacKeYthon '24 label Jan 26, 2024
@WolframPfeifer
Copy link
Member Author

While working on this PR, I stumbled upon some unclear points:

  • Does the code_bigint/safe/java_math annotation belong to the contract or to the method?
  • Is it allowed to have multiple contracts with different code math modes?

In addition, I am not sure what syntax is currently supported by KeY's parser ...

Does anyone know the status quo? How should it be implemented?

@WolframPfeifer
Copy link
Member Author

I am currently struggling with the assignmentToUpdate rules.

The original rule (i.e., status quo in KeY since #3027, with fix for the wrongly named taclet option) for e.g. subtraction looks like this:

    assignmentSubtractionInt {
        \find(\modality{#normalassign}{..
                    #loc = #seCharByteShortInt0 - #seCharByteShortInt1;
                ...}\endmodality (post))
        (intRules:arithmeticSemanticsCheckingOF) {
            "Overflow check":
                \replacewith(inInt(sub(#seCharByteShortInt0, #seCharByteShortInt1)))
        };
        \replacewith({#loc := javaSubInt(#seCharByteShortInt0, #seCharByteShortInt1)}
            \modality{#normalassign}{.. ...}\endmodality (post))
        \heuristics(executeIntegerAssignment)
        \displayname "subtraction"
    };

As @unp1 pointed out, if the modality is in the antecedent, you probably do not want to open a branch with the overflow "check" in the antecedent, as this leads to an unclosable branch usually.

If you would like to prevent that, you would have to duplicate all the rules (about 30 at the moment):

    assignmentSubtractionInt {
        \find(\modality{#normalassign}{..
                    #loc = #seCharByteShortInt0 - #seCharByteShortInt1;
                ...}\endmodality (post))
        \succedentPolarity
        (intRules:arithmeticSemanticsCheckingOF) {
            "Overflow check":
                \replacewith(inInt(sub(#seCharByteShortInt0, #seCharByteShortInt1)))
        };
        \replacewith({#loc := javaSubInt(#seCharByteShortInt0, #seCharByteShortInt1)}
            \modality{#normalassign}{.. ...}\endmodality (post))
        \heuristics(executeIntegerAssignment)
        \displayname "subtraction"
    };

    assignmentSubtractionInt2 {
        \find(\modality{#normalassign}{..
                    #loc = #seCharByteShortInt0 - #seCharByteShortInt1;
                ...}\endmodality (post))
        \antecedentPolarity
        \replacewith({#loc := javaSubInt(#seCharByteShortInt0, #seCharByteShortInt1)}
            \modality{#normalassign}{.. ...}\endmodality (post))
        \heuristics(executeIntegerAssignment)
        \displayname "subtraction"
    };

The open question is: Do we have to add another branch in the antecedent case to be able to assume that inInt(a - b)? Or do we have to check it? I really do not understand what would be correct here ...

Another option without the rule duplication would be to just limit application to the succedent. This has been done for example for the rule assignmentDivisionInt with the runtimeExceptions:ban semantics. However, this is also strange: There is just no rule to further symbolically execute a division (with this semantics selected) in a modality on the left-hand side.

@WolframPfeifer WolframPfeifer marked this pull request as ready for review June 21, 2024 13:29
@wadoon wadoon enabled auto-merge June 24, 2024 11:15
…Builder.java


More concrete comment where to declare taclet options
@wadoon wadoon added this pull request to the merge queue Jun 24, 2024
Merged via the queue into main with commit 64126d9 Jun 24, 2024
12 of 13 checks passed
@wadoon wadoon deleted the pfeifer/3352overflowCheckingFix branch June 24, 2024 14:34
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.

Taclet option for checking overflows (in Java code) does not work
3 participants