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

Fixes for the incremental analysis with cfg comparison #841

Merged
merged 4 commits into from
Nov 25, 2022

Conversation

stilscher
Copy link
Member

@stilscher stilscher commented Sep 28, 2022

This PR arises from several incremental test cases that failed when enabling the cfg comparison (#835). With the grouping of globals (#836) and the bug fix in 6f2486a a couple of problems were fixed but resulted in more/different test cases failing.

This PR includes:

  • fix reluctant destabilization: return nodes of partially changed functions need to be collected for reluctant destabilization (not only when reluctant is turned off!)
  • extended update_suite.rb with the option -c to run incremental tests with cfg comparison
  • run update_suite.rb -c regularly with regression tests

It remains to do:

  • the incremental test 11/15 mutex-simple-wrap1 still fails with cfg comparison because an expected race is not found. It seems that the restarting differs from the run with the ast comparison, but it needs to be investigated further what the actual issue is.
    Update: The reason seems to be that fuel is lost when destabilizing through side_dep first before destabilizing through infl. I suggest to change the behaviour of the destabilize_with_sides in Avoid cyclic destabilize in destabilize_with_sides leading to less fuel than expected #862.

Closes #835

@stilscher stilscher added the bug label Sep 28, 2022
@stilscher stilscher self-assigned this Sep 28, 2022
@jerhard jerhard self-assigned this Sep 28, 2022
@stilscher stilscher marked this pull request as ready for review October 21, 2022 14:24
@stilscher stilscher requested a review from jerhard October 21, 2022 14:24
src/solvers/td3.ml Outdated Show resolved Hide resolved
@jerhard
Copy link
Member

jerhard commented Oct 25, 2022

Looks good!

@sim642
Copy link
Member

sim642 commented Oct 25, 2022

Is there a reason this is to be merged into #836 instead of master? I just merged master into this to confirm that #862 fixes the test, not realizing it's not against master. This made the diff really ugly here, so if there was a reason for this, I'll hard reset this branch back before my merge.

@stilscher
Copy link
Member Author

stilscher commented Oct 31, 2022

The only reason was that I needed #836 to see which tests are actually failing when grouping the globals correctly. I am not sure whether it's now best to wait for #836 to be completed (it should be already, but the cil fix for c99 inlines is still missing) or merge this into master directly.

@sim642 sim642 changed the base branch from fix-incr-global-comparison to master October 31, 2022 11:23
@sim642 sim642 force-pushed the fix-incr-tests-with-cfg-comp branch from 66e7b9e to d9cc7aa Compare October 31, 2022 11:34
@sim642 sim642 changed the base branch from master to fix-incr-global-comparison October 31, 2022 11:34
@sim642 sim642 added the pr-dependency Depends or builds on another PR, which should be merged before label Oct 31, 2022
@sim642
Copy link
Member

sim642 commented Oct 31, 2022

I tried to see how it looks like rebased for master, but it gets overly messy and the CFG testing added here due to #836. So I reset it back and lets wait for #836.

@stilscher stilscher linked an issue Nov 15, 2022 that may be closed by this pull request
3 tasks
Base automatically changed from fix-incr-global-comparison to master November 25, 2022 14:12
@sim642 sim642 added this to the v2.2.0 milestone Nov 25, 2022
@sim642 sim642 removed the pr-dependency Depends or builds on another PR, which should be merged before label Nov 25, 2022
@sim642 sim642 merged commit 76cf1d4 into master Nov 25, 2022
@sim642 sim642 deleted the fix-incr-tests-with-cfg-comp branch November 25, 2022 14:49
sim642 added a commit to sim642/opam-repository that referenced this pull request Sep 13, 2023
CHANGES:

* Add `setjmp`/`longjmp` analysis (goblint/analyzer#887, goblint/analyzer#970, goblint/analyzer#1015, goblint/analyzer#1019).
* Refactor race analysis to lazy distribution (goblint/analyzer#1084, goblint/analyzer#1089, goblint/analyzer#1136, goblint/analyzer#1016).
* Add thread-unsafe library function call analysis (goblint/analyzer#723, goblint/analyzer#1082).
* Add mutex type analysis and mutex API analysis (goblint/analyzer#800, goblint/analyzer#839, goblint/analyzer#1073).
* Add interval set domain and string literals domain (goblint/analyzer#901, goblint/analyzer#966, goblint/analyzer#994, goblint/analyzer#1048).
* Add affine equalities analysis (goblint/analyzer#592).
* Add use-after-free analysis (goblint/analyzer#1050, goblint/analyzer#1114).
* Add dead code elimination transformation (goblint/analyzer#850, goblint/analyzer#979).
* Add taint analysis for partial contexts (goblint/analyzer#553, goblint/analyzer#952).
* Add YAML witness validation via unassume (goblint/analyzer#796, goblint/analyzer#977, goblint/analyzer#1044, goblint/analyzer#1045, goblint/analyzer#1124).
* Add incremental analysis rename detection (goblint/analyzer#774, goblint/analyzer#777).
* Fix address sets unsoundness (goblint/analyzer#822, goblint/analyzer#967, goblint/analyzer#564, goblint/analyzer#1032, goblint/analyzer#998, goblint/analyzer#1031).
* Fix thread escape analysis unsoundness (goblint/analyzer#939, goblint/analyzer#984, goblint/analyzer#1074, goblint/analyzer#1078).
* Fix many incremental analysis issues (goblint/analyzer#627, goblint/analyzer#836, goblint/analyzer#835, goblint/analyzer#841, goblint/analyzer#932, goblint/analyzer#678, goblint/analyzer#942, goblint/analyzer#949, goblint/analyzer#950, goblint/analyzer#957, goblint/analyzer#955, goblint/analyzer#954, goblint/analyzer#960, goblint/analyzer#959, goblint/analyzer#1004, goblint/analyzer#558, goblint/analyzer#1010, goblint/analyzer#1091).
* Fix server mode for abstract debugging (goblint/analyzer#983, goblint/analyzer#990, goblint/analyzer#997, goblint/analyzer#1000, goblint/analyzer#1001, goblint/analyzer#1013, goblint/analyzer#1018, goblint/analyzer#1017, goblint/analyzer#1026, goblint/analyzer#1027).
* Add documentation for configuration JSON schema and OCaml API (goblint/analyzer#999, goblint/analyzer#1054, goblint/analyzer#1055, goblint/analyzer#1053).
* Add many library function specifications (goblint/analyzer#962, goblint/analyzer#996, goblint/analyzer#1028, goblint/analyzer#1079, goblint/analyzer#1121, goblint/analyzer#1135, goblint/analyzer#1138).
* Add OCaml 5.0 support (goblint/analyzer#1003, goblint/analyzer#945, goblint/analyzer#1162).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Three incremental tests failing with the cfg comparison
3 participants