Skip to content

Constant propagator: add callback to filter tracked values #2520

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

Conversation

smowton
Copy link
Contributor

@smowton smowton commented Jul 4, 2018

A user may supply a predicate, in which case the constant propagator will only track symbols that pass the predicate. The security product currently uses this to do const prop only over global variables.

This incidentally also regularises the constructors for constant_propagator_ait, so there is a side-effecting and a non-side-effecting version for both whole-model and single-function cases.

Copy link
Contributor

@allredj allredj left a comment

Choose a reason for hiding this comment

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

Passed Diffblue compatibility checks (cbmc commit: b96dd06).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/78102658

@smowton smowton force-pushed the smowton/feature/constant-propagator-selectivity branch from b96dd06 to 0930b0d Compare July 5, 2018 15:36
@smowton smowton requested a review from a team as a code owner July 5, 2018 15:36
Copy link
Contributor

@NlightNFotis NlightNFotis left a comment

Choose a reason for hiding this comment

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

One question, very minor, LGTM


#include <util/message.h>

#include <iostream>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is iostream imported here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because I'm silly, is why

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wait no I'm not, this is used as the output for goto_convert

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't. Unit tests have become very hard see failures in as they generate a ton of noise. Tests must be completely silent by default. If there is an error, then it should be reported, but not in any other case. As such: please report to a std::ostringstream and check that that output is empty.

Copy link
Member

@peterschrammel peterschrammel left a comment

Choose a reason for hiding this comment

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

LGTM as soon as CI passes...

A user may supply a predicate, in which case the constant propagator will only
track symbols that pass the predicate. For example, one out-of-tree user uses this
to restrict propagation to globals.
@smowton smowton force-pushed the smowton/feature/constant-propagator-selectivity branch from 0930b0d to 56a487e Compare July 5, 2018 16:54
Copy link
Contributor

@allredj allredj left a comment

Choose a reason for hiding this comment

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

This PR failed Diffblue compatibility checks (cbmc commit: 0930b0d).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/78210227
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.

Common spurious failures:

  • the cbmc commit has disappeared in the mean time (e.g. in a force-push)
  • the author is not in the list of contributors (e.g. first-time contributors).

Copy link
Contributor

@allredj allredj left a comment

Choose a reason for hiding this comment

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

Passed Diffblue compatibility checks (cbmc commit: 56a487e).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/78212385

@smowton smowton merged commit 6e9e655 into diffblue:develop Jul 6, 2018
NathanJPhillips pushed a commit to NathanJPhillips/cbmc that referenced this pull request Aug 22, 2018
6409eae Merge pull request diffblue#2449 from tautschnig/c++-template-cleanup
1134455 Merge pull request diffblue#2443 from tautschnig/vs-rdecl
6e9e655 Merge pull request diffblue#2520 from smowton/smowton/feature/constant-propagator-selectivity
9013779 Merge pull request diffblue#2383 from tautschnig/no-sed
56a487e Constant propagator: add callback to filter tracked values
a354513 Merge pull request diffblue#2510 from polgreen/hex_trace
3545be4 Merge pull request diffblue#2523 from peterschrammel/do-not-ignore-full-slice
819c683 Merge pull request diffblue#2493 from jeannielynnmoulton/jeannie/InnerClasses
a018652 allow unsigned long instead of unsigned in regression test for hex_trace
d5bbdd8 represent numerical values in CBMC trace in hex
19bddce Do not ignore --full-slice
5350133 Refactor inner class parsing.
6e554d9 Merge pull request diffblue#2500 from diffblue/git-version-speedup
9ba55e2 Marks anonymous classes as inner classes
b96c7ba move build commands for version.h from common to util/
6ce7b13 Clarifies language in documentation.
c0c1029 Fixes parsing for anonymous classes
1930aef Refactors parsing of inner classes attribute.
b28562b Adds unit test for parsing inner classes.
c336455 Stores inner class data in java class types.
457bac9 Parses InnerClasses attribute of java bytecode.
34bd58f Clean up unused template instantiation symbols
fea1f47 Remove unused parameter from rDeclarator
e00c6ee Set BUILD_ENV via make variable instead of patching via sed

git-subtree-dir: cbmc
git-subtree-split: 6409eae
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants