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

Replace direct stdout printing with logging #1117

Merged
merged 43 commits into from
Feb 13, 2024
Merged

Replace direct stdout printing with logging #1117

merged 43 commits into from
Feb 13, 2024

Conversation

sim642
Copy link
Member

@sim642 sim642 commented Jul 25, 2023

Apparently this is something I started but never opened a PR for. The problem is that there's lots of direct printing to stdout throughout Goblint, which makes output uncontrollable and unpredictable. For example, server mode cannot use stdout because some random printing might get mixed in and break the whole JSON stream.

The idea is to add a Logs library, which is a bit like Messages. It has various logging levels that can be toggled globally (unlike direct printing). The difference is that Messages is for things related to analysis results and shown in IDE, while logging is about Goblint itself, e.g. those TD3 incremental changes lines.
Messages are unordered and incrementally preserved, while Logs just come out in the order they happen and aren't preserved like that.

TODO

  • Fix conflicts with master. (latest: 2024-02-13)
  • Replace all new direct printing on master with Logs. (latest: 2024-02-13)
  • Add log level option.
  • Maingoblint.check_arguments.
  • Remove dbg.verbose.
  • What to do with GobView printing hacks?
  • Document Logs in developer guide.
  • Adapt to Logs module in Goblint gobview#40.

@sim642 sim642 added the cleanup Refactoring, clean-up label Jul 25, 2023
@sim642 sim642 self-assigned this Jul 25, 2023
src/analyses/extractPthread.ml Fixed Show fixed Hide fixed
src/maingoblint.ml Fixed Show fixed Hide fixed
src/spec/specUtil.ml Fixed Show fixed Hide fixed
src/util/backtrace/goblint_backtrace.ml Fixed Show fixed Hide fixed
src/util/options.ml Fixed Show fixed Hide fixed
src/util/options.ml Fixed Show fixed Hide fixed
src/maingoblint.ml Fixed Show fixed Hide fixed
src/maingoblint.ml Fixed Show fixed Hide fixed
src/maingoblint.ml Fixed Show fixed Hide fixed
src/maingoblint.ml Fixed Show fixed Hide fixed
src/maingoblint.ml Fixed Show fixed Hide fixed
src/maingoblint.ml Fixed Show fixed Hide fixed
src/maingoblint.ml Fixed Show fixed Hide fixed
src/maingoblint.ml Fixed Show fixed Hide fixed
src/maingoblint.ml Fixed Show fixed Hide fixed
@sim642 sim642 added this to the v2.4.0 milestone Nov 28, 2023
src/analyses/extractPthread.ml Dismissed Show dismissed Hide dismissed
src/config/options.ml Dismissed Show dismissed Hide dismissed
src/config/options.ml Dismissed Show dismissed Hide dismissed
src/maingoblint.ml Fixed Show fixed Hide fixed
src/maingoblint.ml Fixed Show fixed Hide fixed
src/maingoblint.ml Fixed Show fixed Hide fixed
src/maingoblint.ml Fixed Show fixed Hide fixed
src/maingoblint.ml Fixed Show fixed Hide fixed
src/maingoblint.ml Dismissed Show dismissed Hide dismissed
src/util/backtrace/goblint_backtrace.ml Dismissed Show dismissed Hide dismissed
@sim642
Copy link
Member Author

sim642 commented Jan 11, 2024

There's now a "result" log level that by default goes to stdout for the handful of places that were pointed out. In server mode, it is also directed to stderr to allow clean JSON-RPC communication.

Copy link
Member

@michael-schwarz michael-schwarz left a comment

Choose a reason for hiding this comment

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

Looks good! I have a few remaining questions in some places 😄

src/analyses/extractPthread.ml Show resolved Hide resolved
src/analyses/threadId.ml Show resolved Hide resolved
src/framework/constraints.ml Show resolved Hide resolved
src/framework/control.ml Show resolved Hide resolved
src/witness/witness.ml Show resolved Hide resolved
src/util/tracing/goblint_tracing.ml Outdated Show resolved Hide resolved
@michael-schwarz
Copy link
Member

michael-schwarz commented Feb 7, 2024

It seems like there's only two open comments left. I'd suggest to merge as soon as these are addressed to avoid it going out-of-sync and requiring a lot of work again. Wdyt?

@sim642
Copy link
Member Author

sim642 commented Feb 7, 2024

@sim642
Copy link
Member Author

sim642 commented Feb 13, 2024

On top of GobView fixes, I confirmed that it still works.

@sim642 sim642 merged commit 60c1030 into master Feb 13, 2024
17 checks passed
@sim642 sim642 deleted the logs branch February 13, 2024 13:51
sim642 added a commit to sim642/opam-repository that referenced this pull request Aug 2, 2024
CHANGES:

* Remove unmaintained analyses: spec, file (goblint/analyzer#1281).
* Add linear two-variable equalities analysis (goblint/analyzer#1297, goblint/analyzer#1412, goblint/analyzer#1466).
* Add callstring, loopfree callstring and context gas analyses (goblint/analyzer#1038, goblint/analyzer#1340, goblint/analyzer#1379, goblint/analyzer#1427, goblint/analyzer#1439).
* Add non-relational thread-modular value analyses with thread IDs (goblint/analyzer#1366, goblint/analyzer#1398, goblint/analyzer#1399).
* Add NULL byte array domain (goblint/analyzer#1076).
* Fix spurious overflow warnings from internal evaluations (goblint/analyzer#1406, goblint/analyzer#1411, goblint/analyzer#1511).
* Refactor non-definite mutex handling to fix unsoundness (goblint/analyzer#1430, goblint/analyzer#1500, goblint/analyzer#1503, goblint/analyzer#1409).
* Fix non-relational thread-modular value analysis unsoundness with ambiguous points-to sets (goblint/analyzer#1457, goblint/analyzer#1458).
* Fix mutex type analysis unsoundness and enable it by default (goblint/analyzer#1414, goblint/analyzer#1416, goblint/analyzer#1510).
* Add points-to set refinement on mutex path splitting (goblint/analyzer#1287, goblint/analyzer#1343, goblint/analyzer#1374, goblint/analyzer#1396, goblint/analyzer#1407).
* Improve narrowing operators (goblint/analyzer#1502, goblint/analyzer#1540, goblint/analyzer#1543).
* Extract automatic configuration tuning for soundness (goblint/analyzer#1369).
* Fix many locations in witnesses (goblint/analyzer#1355, goblint/analyzer#1372, goblint/analyzer#1400, goblint/analyzer#1403).
* Improve output readability (goblint/analyzer#1294, goblint/analyzer#1312, goblint/analyzer#1405, goblint/analyzer#1497).
* Refactor logging (goblint/analyzer#1117).
* Modernize all library function specifications (goblint/analyzer#1029, goblint/analyzer#688, goblint/analyzer#1174, goblint/analyzer#1289, goblint/analyzer#1447, goblint/analyzer#1487).
* Remove OCaml 4.10, 4.11, 4.12 and 4.13 support (goblint/analyzer#1448).
avsm pushed a commit to avsm/opam-repository that referenced this pull request Sep 5, 2024
CHANGES:

* Remove unmaintained analyses: spec, file (goblint/analyzer#1281).
* Add linear two-variable equalities analysis (goblint/analyzer#1297, goblint/analyzer#1412, goblint/analyzer#1466).
* Add callstring, loopfree callstring and context gas analyses (goblint/analyzer#1038, goblint/analyzer#1340, goblint/analyzer#1379, goblint/analyzer#1427, goblint/analyzer#1439).
* Add non-relational thread-modular value analyses with thread IDs (goblint/analyzer#1366, goblint/analyzer#1398, goblint/analyzer#1399).
* Add NULL byte array domain (goblint/analyzer#1076).
* Fix spurious overflow warnings from internal evaluations (goblint/analyzer#1406, goblint/analyzer#1411, goblint/analyzer#1511).
* Refactor non-definite mutex handling to fix unsoundness (goblint/analyzer#1430, goblint/analyzer#1500, goblint/analyzer#1503, goblint/analyzer#1409).
* Fix non-relational thread-modular value analysis unsoundness with ambiguous points-to sets (goblint/analyzer#1457, goblint/analyzer#1458).
* Fix mutex type analysis unsoundness and enable it by default (goblint/analyzer#1414, goblint/analyzer#1416, goblint/analyzer#1510).
* Add points-to set refinement on mutex path splitting (goblint/analyzer#1287, goblint/analyzer#1343, goblint/analyzer#1374, goblint/analyzer#1396, goblint/analyzer#1407).
* Improve narrowing operators (goblint/analyzer#1502, goblint/analyzer#1540, goblint/analyzer#1543).
* Extract automatic configuration tuning for soundness (goblint/analyzer#1369).
* Fix many locations in witnesses (goblint/analyzer#1355, goblint/analyzer#1372, goblint/analyzer#1400, goblint/analyzer#1403).
* Improve output readability (goblint/analyzer#1294, goblint/analyzer#1312, goblint/analyzer#1405, goblint/analyzer#1497).
* Refactor logging (goblint/analyzer#1117).
* Modernize all library function specifications (goblint/analyzer#1029, goblint/analyzer#688, goblint/analyzer#1174, goblint/analyzer#1289, goblint/analyzer#1447, goblint/analyzer#1487).
* Remove OCaml 4.10, 4.11, 4.12 and 4.13 support (goblint/analyzer#1448).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Refactoring, clean-up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants