Skip to content

UnionDomain.meet: Throw Uncomparable exception for abstract unions with different active fields#1109

Merged
jerhard merged 6 commits intomasterfrom
union_meet
Jul 21, 2023
Merged

UnionDomain.meet: Throw Uncomparable exception for abstract unions with different active fields#1109
jerhard merged 6 commits intomasterfrom
union_meet

Conversation

@jerhard
Copy link
Copy Markdown
Member

@jerhard jerhard commented Jul 11, 2023

Edit: This PR changes the meet of the UnionDomain such that it throws a Uncomparable exception, as suggested below: #1109 (comment). The BaseInvariant that uses the VD.meet catches the Uncomparable exception and does not do a refinement in case it is thrown.

On test case 27/18 both branches of an if comparing two unions are no longer considered dead, and test 27/19 does no longer crashes with the IncompatbileIkindException.

Fixes #1105.

Previous Approach

This adds a function ValueDomain.try_meet, a version of the meet that may fail for unions with different active fields and use it in BaseInvariant. @michael-schwarz and I discussed this today.

The refinement by BaseInvariant to handle branching used VD.meet, which caused issue #1105, due to the behavior of UnionDomain.meet: The meet of unions with different active fields can result in a bottom value.
This led to e.g. spuriously dead code.

This adds a function try_meet to the VvalueDomain, the union domain, and those domains that may contain a union domain element: structs, arrays, blobs. The function try_meet in the union domain throws an exception if the fields of the unions for which a meet was tried do not coincide.

jerhard added 3 commits July 11, 2023 15:43
…t active fields,

use it in BaseInvariant.

The refinement by BaseInvariant to handle branching used VD.meet, which
caused issue #1105, due to the behavior of UnionDomain.meet:
The meet of unions with different active fields can result in a bottom value.
This led to e.g. spuriously dead code.

This commit adds a function try_meet to the valueDomain, the union domain,
and those domains that may contain a union domain element: structs, arrays, blobs.
The function try_meet in the union domain throws an exception in the
case the fields of the unions for which a meet was tried did not coincide.
In case this exception occurs, the BaseInvariant does not perform
refinement.
@jerhard
Copy link
Copy Markdown
Member Author

jerhard commented Jul 11, 2023

The exact behavior on 27/19 may be investigated further in the future, as currently, Goblint claims that for the following if always evaluates to true:

union u {
int x;
char c;
};
int main(){
union u a;
union u b;
a.x = 12;
b.c = 12;
int i = 0;
if(a.x == b.x){
i++;
}

Whether this actually holds may e.g. depend on the endianness of the machine. But this is a separate issue.

@sim642 sim642 self-requested a review July 11, 2023 20:44
jerhard added 2 commits July 12, 2023 16:33
…not match.

Raise an Lattice.Uncomparable exception when the active fields of the unions that the meet is applied to, do not match. Catch Uncomparable exception in
BaseInvarian, and do not do refinement in case the exception is thrown during the VD.meet.
@jerhard jerhard changed the title Add VD.try_meet function and use it instead of VD.meet in BaseInvariant UnionDomain.meet: Throw Uncomparable exception for abstract unions with different active fields Jul 12, 2023
@sim642 sim642 self-requested a review July 12, 2023 18:25
@sim642 sim642 added this to the v2.2.0 milestone Jul 18, 2023
The definition of the meet via Lattice.Prod (Field) (Values) is equivalent and thus does not need to be overwritten.
@jerhard jerhard merged commit 0fc9654 into master Jul 21, 2023
@jerhard jerhard deleted the union_meet branch July 21, 2023 08:27
@jerhard
Copy link
Copy Markdown
Member Author

jerhard commented Jul 21, 2023

I had to readd the explicit meet on master since otherwise it would not be ensured that the meet is first performed on the Field, leading to IncompatibleIkind exceptions, where Lattice.Uncomparable exceptions should be thrown.

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.

Use of UnionDomain.meet in refinement causes unsoundness and crashes

2 participants