Skip to content

Conversation

@jezhiggins
Copy link
Contributor

@jezhiggins jezhiggins commented Apr 12, 2021

Two related strands of work in this PR.

  • Extending abstract_environmentt::assume to cover all value representations. Up until now, assume was only correct for constants. It may have evaluated a given expression correctly for intervals and value-sets, but not reliably. These changes all assume expressions to be properly evaluated for all value representation, and in the face of different representations within the same expression.

  • Filling out the implementation of abstract_objectt::meet for value representations. Where merge is the union of two values, meet is the intersection. meet is used when evaluating an assumed equality expression.

+2,331/-384 looks alarming, but the major bulk of that is test cases exercising various combinations of constants, intervals, value-sets, and operators in assume and in meet.

@jezhiggins jezhiggins force-pushed the vsd-widening-implementation branch 2 times, most recently from eeaaee8 to 1614d74 Compare April 12, 2021 15:51
@jezhiggins
Copy link
Contributor Author

MacOS build failures look to be a meta-problem getting the tool dependencies

==> Downloading https://homebrew.bintray.com/bottles/ninja-1.10.2.catalina.bottle.tar.gz
curl: (22) The requested URL returned error: 403 Forbidden
Error: Failed to download resource "ninja"
Download failed: https://homebrew.bintray.com/bottles/ninja-1.10.2.catalina.bottle.tar.gz
Error: Process completed with exit code 1.

@martin-cs
Copy link
Collaborator

Paging @NlightNFotis and @thomasspriggs for the Mac OS CI issues.

@NlightNFotis
Copy link
Contributor

I took a quick look at it - in the beginning I thought that they upgraded the version so it's hitting the wrong URL because of outdated database issues (kind of like needing an apt-get update, if you want).

But it looks like it's still the same version of the formula (https://formulae.brew.sh/formula/ninja), and it's 403 if you hit the URL, which seems to be sitting on their servers, so I'm assuming this must be a temporary issue they are having.

Could you try it again (just restart the jobs from the github interface) later, and let me know if it doesn't work? I'll take a deeper look at it then.

@codecov
Copy link

codecov bot commented Apr 12, 2021

Codecov Report

Merging #6023 (b23e00d) into develop (0002950) will increase coverage by 8.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #6023      +/-   ##
===========================================
+ Coverage    67.40%   75.42%   +8.01%     
===========================================
  Files         1157     1454     +297     
  Lines        95236   159739   +64503     
===========================================
+ Hits         64197   120477   +56280     
- Misses       31039    39262    +8223     
Flag Coverage Δ
cproversmt2 ?
regression ?
unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/util/string_container.cpp 52.94% <0.00%> (-47.06%) ⬇️
src/ansi-c/gcc_types.cpp 41.86% <0.00%> (-46.38%) ⬇️
src/solvers/prop/prop.cpp 42.85% <0.00%> (-41.76%) ⬇️
src/solvers/flattening/boolbv_member.cpp 53.65% <0.00%> (-38.65%) ⬇️
src/cpp/cpp_storage_spec.cpp 65.00% <0.00%> (-35.00%) ⬇️
src/util/cmdline.h 66.66% <0.00%> (-33.34%) ⬇️
src/solvers/strings/array_pool.h 66.66% <0.00%> (-33.34%) ⬇️
src/solvers/strings/string_refinement.h 66.66% <0.00%> (-33.34%) ⬇️
...rs/strings/string_concatenation_builtin_function.h 0.00% <0.00%> (-33.34%) ⬇️
src/ansi-c/c_typecheck_base.cpp 50.54% <0.00%> (-30.35%) ⬇️
... and 1433 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b84b37d...b23e00d. Read the comment docs.

@jezhiggins jezhiggins force-pushed the vsd-widening-implementation branch from 1614d74 to 918f68d Compare April 13, 2021 08:16
@tautschnig
Copy link
Collaborator

MacOS build failures look to be a meta-problem getting the tool dependencies

==> Downloading https://homebrew.bintray.com/bottles/ninja-1.10.2.catalina.bottle.tar.gz
curl: (22) The requested URL returned error: 403 Forbidden
Error: Failed to download resource "ninja"
Download failed: https://homebrew.bintray.com/bottles/ninja-1.10.2.catalina.bottle.tar.gz
Error: Process completed with exit code 1.

For the record: actions/runner-images#3165 - presumably the current run will have updated runners already.

@jezhiggins jezhiggins force-pushed the vsd-widening-implementation branch 6 times, most recently from 93848ac to 83414be Compare April 22, 2021 14:39
@jezhiggins jezhiggins marked this pull request as ready for review April 22, 2021 15:14
@jezhiggins jezhiggins force-pushed the vsd-widening-implementation branch 4 times, most recently from 2492245 to ba7d4b9 Compare May 8, 2021 10:04
@jezhiggins jezhiggins force-pushed the vsd-widening-implementation branch from ba7d4b9 to da1e973 Compare May 11, 2021 08:43
@jezhiggins jezhiggins force-pushed the vsd-widening-implementation branch from da1e973 to f191b3c Compare May 18, 2021 12:01
Copy link
Collaborator

@martin-cs martin-cs left a comment

Choose a reason for hiding this comment

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

In principle happy but there are a few details to check.

@jezhiggins jezhiggins force-pushed the vsd-widening-implementation branch 2 times, most recently from 53134d6 to b427be5 Compare May 22, 2021 09:20
Seperates the assume evaluation from the action arising.
This is the first step on splitting out assume so we can given sensible
answers for all value representations, not just constants.
Consider cases when one or other operand is not a value, or is unknown.

Implement meet for write_location_context.
For intervals and value-sets it's possible for, say, (a > b) and !(a >
b) to both be true. When evaluationing !(expr) we can't simply flip the
result. Instead, where we can, we need to rewrite !(a > b) as (a <= b)
and evaluate that instead.
We can bail out early if an operand evals to false.
Otherwise we must evaluate al lthe operands, so that any pruning is
applied. That done, we return true iff all operands are true, otherwise
we return nil if anywhere nil.
Implemented for intervals and value-sets which are not top.
These operations are symmetric, so we can rewrite x > y as y < x, and
x >= y as y <= x.  As a consequence of rewriting, greater than
assumptions now also prune.
Remove duplication in data_dependency & write_location
merge and meet. Correct data_dependency_contextt::
has_been_modified to verify child object is also unchanged
Found and fixed a problem in interval_abstract_value::to_constant(),
for case when interval is a valid single value, but the whole
thing is also top. This could arise in a merge of, say, [0, 0] and TOP.
@jezhiggins jezhiggins force-pushed the vsd-widening-implementation branch from 371936f to b23e00d Compare June 15, 2021 08:01
@martin-cs martin-cs merged commit 1ff5344 into diffblue:develop Jun 15, 2021
@jezhiggins jezhiggins deleted the vsd-widening-implementation branch June 16, 2021 11:48
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