-
Notifications
You must be signed in to change notification settings - Fork 77
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
Small output readability improvements #1312
Conversation
…223372036854775807]) → ⊤
src/analyses/mHPAnalysis.ml
Outdated
let should_print {tid; created; must_joined} = | ||
not (ConcDomain.ThreadSet.is_empty created) | ||
|| not (ConcDomain.ThreadSet.is_empty must_joined) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this an improvement? All this information is needed to understand why accesses are considered racy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In mhp.ml
these exact conditions are already used to not show empty sets. This just means we don't print mhp:{}
without anything in it.
We also don't print empty locksets etc. Everything we analyze and print for accesses is what we have for why access are not racy, i.e. information that can rule something out. If we don't have any excluding information (e.g. no locks), then we don't print that we don't know it. If we printed everything we didn't know, we'd have to print complete states.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, for must-joined I guess this holds.
An empty created set is very strong information though (no information would be top).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the created set is empty, we're already single-threaded and that's already reflected by the thread flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the created set is empty, we're already single-threaded and that's already reflected by the thread flag.
I'm not sure that's true: The created set is relative to the things the current thread created: We may be in a multi-threaded setting and I may not be the main thread but unique. Then an empty created set means that none of my children can be created yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good by me. With dbg.full-output
all the old output is retained now.
The splitting up of mhp:{...}
in accesses I would leave for a separate PR because it's more than just show
/pretty
/printXml
changes. Unless we want it to be like thread:{tid=...; created={...}}
, which would be the case since the created thread set is also in the threadid
analysis domain. Allowing one analysis to nicely output two components into accesses probably requires some MCPAccess
changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now! Sorry I was unaware that this was still waiting for me review!
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).
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).
Resolves #1192