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

Relational: Use same invalidation strategy as base #1646

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

michael-schwarz
Copy link
Member

Closes #1535

@michael-schwarz michael-schwarz added bug precision relational Relational analyses (Apron, affeq, lin2var) labels Dec 17, 2024
@michael-schwarz
Copy link
Member Author

michael-schwarz commented Dec 17, 2024

I don't get what goes wrong in the CI here, it works locally.
The reason seems to be non-unique test names.

@sim642 sim642 self-requested a review December 18, 2024 12:05
src/analyses/apron/relationAnalysis.apron.ml Outdated Show resolved Hide resolved
Comment on lines +682 to +687
(* TODO: HACK: Simulate enter_multithreaded for first entering thread to publish global inits before analyzing thread.
Otherwise thread is analyzed with no global inits, reading globals gives bot, which turns into top, which might get published...
sync `Thread doesn't help us here, it's not specific to entering multithreaded mode.
EnterMultithreaded events only execute after threadenter and threadspawn. *)
if not (ThreadFlag.has_ever_been_multi (Analyses.ask_of_man man)) then
ignore (Priv.enter_multithreaded (Analyses.ask_of_man man) man.global man.sideg st);
Copy link
Member

Choose a reason for hiding this comment

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

Why is this moved out? Base only does it for non-unknown thread functions.

Copy link
Member Author

Choose a reason for hiding this comment

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

The logic of why it is needed is the same here as it is for the case where a known thread is created: One should not go into multi-threaded mode without notifying the privatization that this switch happened to ensure everything that should be published is published.

Maybe base has some reason this can be avoided, but I'm not really sure why that should be the case. We may want to make an issue to investigate this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also see the last few comments in #1551

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

The one test in this PR doesn't cover this, does it?

I tried the very simple thing of creating a thread from unknown function pointer and that seems to work with base. That's because threadflag analysis emits the EnterMultiThreaded event which is handled by base (and relational).
The comment for this hack is about the globals values that get used when analyzing the created thread. For an unknown function pointer, there's no thread body to analyze anyway, so it's not clear to me when it would make a difference.

Copy link
Member Author

Choose a reason for hiding this comment

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

The test covers it in some sense. We mean to invalidate e and output

[Info][Imprecise] Invalidating expressions: & e (1.c:11:3-11:35)

But this invalidation does not take and we still claim to be able to prove things about e.

Copy link
Member Author

Choose a reason for hiding this comment

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

Edit: That case is listed in the linked issue.

tests/regression/46-apron2/98-invalidate-more.c Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug precision relational Relational analyses (Apron, affeq, lin2var)
Projects
None yet
2 participants