Skip to content

Generate function bodies with nondet return values #3457

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

Merged
merged 2 commits into from
Jan 23, 2019

Conversation

danpoe
Copy link
Contributor

@danpoe danpoe commented Nov 24, 2018

  • Each commit message has a non-empty body, explaining why the change was made.
  • Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • n/a My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • White-space or formatting changes outside the feature-related changed lines are in commits of their own.

Copy link
Contributor

@smowton smowton left a comment

Choose a reason for hiding this comment

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

Cursory review because this is big. For a more thorough look over I recommend introducing this in stages

@@ -70,8 +70,7 @@ class languaget:public messaget
/// complete. Functions introduced here are visible to lazy loading and
/// can influence its decisions (e.g. picking the types of input parameters
/// and globals), whereas anything introduced during `final` cannot.
virtual bool generate_support_functions(
symbol_tablet &symbol_table)=0;
virtual bool generate_support_functions(symbol_tablet &symbol_table) = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated restyle in commit

@@ -10,6 +10,8 @@ Author: Daniel Kroening, [email protected]

#include <fstream>

#include <util/object_factory_parameters.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding an include but nothing that uses it (in this commit)?

/// \param loc: The location to assign to the symbol
/// \param type: The type of symbol to create
/// \param static_lifetime: Whether the symbol should have a static lifetime
/// \param prefix: The prefix to use for the symbol's basename
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 this in this PR?

assert(p->prev != &dummy);

assert(p->next != NULL);
assert(p->next != &dummy);
Copy link
Contributor

Choose a reason for hiding this comment

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

Also assert p->next != p->prev?

@@ -181,7 +224,6 @@ void symbol_factoryt::gen_nondet_init(
/// \param base_name: The name to use for the symbol created
/// \param type: The type for the symbol created
/// \param loc: The location to assign to generated code
/// \param allow_null: Whether to allow a null value when type is a pointer
Copy link
Contributor

Choose a reason for hiding this comment

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

does removing this unused parameter belong in a different commit?

exprt allocate_objectst::allocate_object(
code_blockt &assignments,
const exprt &target_expr,
const typet &allocate_type,
Copy link
Contributor

Choose a reason for hiding this comment

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

allocate_type and alloc_type are very similar names

GLOBAL,
/// Allocate local stacked objects
LOCAL,
/// Allocate dynamic objects (using MALLOC)
Copy link
Contributor

Choose a reason for hiding this comment

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

now using ALLOCATE

@danpoe
Copy link
Contributor Author

danpoe commented Nov 26, 2018

@smowton Thanks for the review! The PR is based on another open PR (#3455) and in this one only the last two commits are new (see PR description).

@danpoe danpoe force-pushed the feature/nondet-return-values branch from 3dbd888 to 9a2abb1 Compare November 27, 2018 15:59
@kroening
Copy link
Member

Should this live in goto-instrument? It's an instrumentation.

Copy link
Member

@kroening kroening left a comment

Choose a reason for hiding this comment

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

Should be in goto-instrument!

@danpoe danpoe changed the title Generate function bodies with nondet return values Generate function bodies with nondet return values [depends-on: #3455] Nov 29, 2018
@danpoe
Copy link
Contributor Author

danpoe commented Nov 29, 2018

@kroening One thing this PR does is move generate_function_bodies.h/cpp from goto-programs to goto-instrument. However I left the C object factory in ansi-c (just like the Java object factory is in java_bytecode). Are you suggesting to move the C object factory to goto-instrument as well?

@danpoe danpoe force-pushed the feature/nondet-return-values branch from 9a2abb1 to f2506f9 Compare November 29, 2018 17:52
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: f2506f9).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/93055935

@danpoe danpoe force-pushed the feature/nondet-return-values branch from f2506f9 to 923a9eb Compare December 19, 2018 11:49
danpoe added a commit that referenced this pull request Jan 22, 2019
…-targets

Generate function bodies that nondet havoc the function arguments [blocks: #3457]
@danpoe danpoe force-pushed the feature/nondet-return-values branch from 923a9eb to 27d3a72 Compare January 22, 2019 14:45
@danpoe danpoe changed the title Generate function bodies with nondet return values [depends-on: #3455] Generate function bodies with nondet return values Jan 22, 2019
@danpoe danpoe removed their assignment Jan 22, 2019
@danpoe danpoe force-pushed the feature/nondet-return-values branch from 27d3a72 to cc4baa5 Compare January 22, 2019 15:00
@danpoe
Copy link
Contributor Author

danpoe commented Jan 22, 2019

@kroening This is now done in goto-instrument.

Copy link
Contributor

@chrisr-diffblue chrisr-diffblue left a comment

Choose a reason for hiding this comment

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

LGTM

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: cc4baa5).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/98196114
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).

The incompatibility may have been introduced by an earlier PR. In that case merging this
PR should be avoided unless it fixes the current incompatibility.

@kroening kroening assigned danpoe and unassigned kroening Jan 22, 2019
@danpoe danpoe force-pushed the feature/nondet-return-values branch from cc4baa5 to 5fa9452 Compare January 23, 2019 10:39
Extends the generate function bodies feature to create nondet return values for
functions, using the c nondet symbol factory.
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: 5fa9452).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/98301475

@danpoe danpoe merged commit 9f1f3f5 into diffblue:develop Jan 23, 2019
@danpoe danpoe deleted the feature/nondet-return-values branch June 2, 2020 17:20
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.

5 participants