Simplifier and symex rewriting fixes/extensions #731

tautschnig · 2017-03-30T11:41:40Z

Includes related fixed from the SV-COMP PR #363.

kroening · 2017-04-04T12:37:07Z

src/util/simplify_expr.cpp

+       base_type_eq(expr_type, pointer_typet(o.type().subtype()), ns))
+    {
+      unsignedbv_typet size_type(config.ansi_c.pointer_width);
+


use size_type() ?

Will be fixed.

kroening · 2017-04-04T12:37:28Z

src/util/simplify_expr.cpp

-     op_type.id()==ID_pointer)
+       expr.op0().op0().op0().is_zero()) ||
+      (expr.op0().op0().is_constant() &&
+       to_constant_expr(expr.op0().op0()).get_value()==ID_NULL)))
  {
    unsignedbv_typet size_type(config.ansi_c.pointer_width);



Use size_type() ?

tautschnig · 2017-04-07T12:38:13Z

@kroening I have now added another patch to use the functions from c_types across the code base.

tautschnig · 2017-04-10T07:38:31Z

... and now the bugs that this change made apparent are fixed as well.

tautschnig · 2017-05-23T10:11:36Z

This should not be merged until #955 is merged (which has an overlapping, yet improved commit). I will also hold off on the necessary rebase until this has happened.

peterschrammel · 2017-06-25T21:06:46Z

@kroening, looks good to go.

marek-trtik · 2017-11-02T18:05:44Z

I merged all commits of this PR into the PR #1532. The correctness can be easily checked as log messages of commits match (here and in #1532).

tautschnig · 2017-11-02T18:20:27Z

@thk123 Could non-impact on test gen also be assessed for this one? (I guess it's not only @thk123 who can do this...)

thk123 · 2017-11-06T12:13:25Z

Thanks for asking (it isn't just me if I'm away from next week for two weeks), but done anyway 🙂 ( 🔒 https://github.com./diffblue/test-gen/pull/1155). Waiting on CI - will post the results - but the PR is quite a long way behind current develop so it might require a rebase and re-run if it fails.

That said - I don't see anything in this PR that looks likely to have a breaking impact.

thk123 · 2017-11-06T12:15:05Z

Yup sorry this requires a rebase as it doesn't include the cmake file which is tripping over the CI... (we only use CMake on TG)

kroening · 2017-11-06T12:17:22Z

src/goto-symex/symex_clean_expr.cpp

@@ -166,12 +168,41 @@ void goto_symext::replace_array_equal(exprt &expr)
    replace_array_equal(*it);
 }

+/// Rewrite index/member expressions in byte_extract to offset
+static void adjust_byte_extract_rec(exprt &expr, const namespacet &ns)
+{


Wouldn't the natural place for this to happen be the simplifier?

While that is true, it would cause/require repeated simplification of the same expression (note that this simplification needs to happen before doing symex_assign_*). It may, however, also be useful to add this to the simplifier as a fallback?

Not sure the object descriptors fit into simplifier; leave as is for now.

kroening · 2017-11-06T12:20:17Z

src/solvers/flattening/flatten_byte_operators.cpp

@@ -242,12 +242,12 @@ exprt flatten_byte_extract(

        byte_extract_exprt tmp(unpacked);
        tmp.type()=subtype;
-        tmp.offset()=simplify_expr(new_offset, ns);
+        tmp.offset()=new_offset;


Also wondering about the right place for this -- perhaps where flatten_byte_* is used?

I am also kind of tempted to do the flattening in the simplifier.

I do agree that this should be moved to the simplifier (also note that it only has dependencies within util/), but may I suggest that this happens as a separate PR? Otherwise this PR will become huge.

yes, separate PR

Thanks, I've put a note on my TODO list. I'll take care of it later on.

tautschnig · 2017-11-06T12:22:18Z

@thk123 Rebase done. (Now looking at the incoming comments.)

kroening · 2017-11-06T12:30:43Z

src/util/simplify_expr_boolean.cpp

+    {
+      if(negate)
+        expr.op0().make_not();
+      exprt tmp(operands.front());


Style thing: use operands.front() above instead of op0(), or it's confusing.

kroening

Looks good to me, mod the minor comments in the review.

thk123 · 2017-11-06T13:51:44Z

TG when pointed at this PR fails - but for what seems to be the same reason as this PR.

… the root object

The code paths are vastly different, and splitting thus enables optimisations.

Further simplify extracts from arrays and structs

We construct several non-trivial expressions that, e.g., contain sums over constants. As these expressions may be passed to array post-processing (flatten_byte_operators is in particular used with unbounded arrays) this can have a significant impact on the size of array index sets.

tautschnig · 2017-11-06T15:20:59Z

@thk123 could you please trigger a re-run of TG?

tautschnig · 2017-11-06T16:14:12Z

I'd intend to merge as soon as @thk123 sends an "ok." @marek-trtik any objections?

thk123 · 2017-11-06T17:25:25Z

Re-triggered - will let you know once it has finished.

tautschnig · 2017-11-07T11:09:31Z

Pinging @thk123 @marek-trtik or @peterschrammel in case anyone of you is available to check the TG status?

kroening · 2017-11-07T17:49:21Z

The TG build still fails, but I believe that's caused by a dependency on something else (related to the symbol table) that is assumed to be merged.

I'll merge this one unless I hear a good reason why not.

thk123 · 2017-11-07T19:00:17Z

Sorry yes been busy today - I'd prefer to wait, it should just need a rebase and re-run, though if this is urgent, as I said before - I don't view this as high risk. I can rebase tomorrow unless someone gets to it first.

peterschrammel · 2017-11-07T21:30:49Z

The TG build is passing now.

e8b3cb9 Merge remote-tracking branch 'upstream/develop' into smowton/merge/develop_20171116 dc4a293 Merge pull request diffblue#1594 from reuk/reuk/cmake-fixup 48fc3d4 Merge pull request diffblue#1592 from antlechner/antonia/char-escape 538eef6 Merge pull request diffblue#1577 from smowton/smowton/fix/dependence_graph_inconsistency d3d632d Use multi-argument form of FILE command 81e56cc Tidy up CMakeLists f7141c0 Merge pull request diffblue#1582 from romainbrenguier/refactor/numerical-cast 8ed1023 Use UTF-16 conversion function in expr2java a53f5bf Split UTF-16 conversion code into two cases e0ad069 Merge pull request diffblue#1558 from NathanJPhillips/feature/complete-journalling_symbol_table 69d1a52 Added usages of base class symbol table 3e42a8d Add comment on has_symbol a2b45e3 Update to journalling symbol table 7aa80ad Remove lookup_impl - it won't work for recording symbol table and adds complexity cdbac8c Sort output of symbol_tablet::show 2ef1c94 Fix bug where move from const symbol collections 8035397 Style improvements 6dae8e8 Merge pull request diffblue#1515 from smowton/smowton/admin/codeowners 5297646 another ranged for 3d66779 Merge branch 'develop' of github.com.:diffblue/cbmc into develop 4b5467c another ranged for f5dbfd4 Merge pull request diffblue#1589 from reuk/reuk/fewer-exceptions 8e99272 use ranged for 95cf5c3 Add directories without code owners and adapt code owners 8da6a81 Replace try-catch with nullptr checks 9ff48e0 Add numeric_cast template for numeric conversion af31813 Merge pull request diffblue#1575 from svorenova/nested_generics_tg1301 21b4e7e Extend unit tests to test for nested generics cf47dcb Extending parsing of generics to parse nested generic types 1aefb09 Merge pull request diffblue#1547 from smowton/smowton/feature/remove_virtual_functions_single_call 2b4ed77 Merge pull request diffblue#1579 from smowton/smowton/fix/cmdline_destructor 7305506 Merge pull request diffblue#1580 from smowton/smowton/fix/cast_materialised_temporary 87b9de1 Remove pointless typecasts a59dea6 Add unit test checking dependence graph consistency 80e66ba Remove virtual functions: expose single-call entry-point ffe02e4 Remove useless cmdlinet::clear() call ae34e9b Merge pull request diffblue#1578 from thk123/bugfix/specalised-classes 779d0aa Merge pull request diffblue#1574 from diffblue/taint-memcpy-develop 28a4846 Merge pull request diffblue#1568 from smowton/smowton/fix/java_div_by_zero ffd089f Constructed class to mimic the original class in all but name of symbol 7f53f02 Merge pull request diffblue#1569 from thk123/bugfix/TG-1403/generic-field-arrays 1abc75e Dependence graph: ensure grapht representation is consistent with domain e03b0cb Abstract interpreter: add finalize hook fa7d62a Makefile for goto-analyzer-taint-ansi-c 758ebb3 transfer taint on memcpy and memmove d0a844b Assert denominator non-zero when Java runtime exceptions are disabled e5744b2 Reorder code owner definition according to change risk 0f98cb4 Removed redundant if statement ffa104c Enforce condition that generic references must refer to generic classes 6e06fbd Extending tests to deal with specialising with arrays when array fields a01a0f2 Extend the specialisation code to handle generic fields 1ccbf83 Correctly handle generic classes that have a array field f60d8c8 Unit utility for symbol types 21a33fa Renaming to_java_generic_class_type to remove spurious s 94ffce3 Merge pull request diffblue#1567 from mgudemann/mgudemann/feature/support_arrays_in_generic_parameters 5be97db Create new and adapt existing unit tests for generic array param ef6b4af Post-fix arrays as generic types with their element type 4db6fc6 Merge pull request diffblue#1553 from mgudemann/bugfix/initialize_pointer_width_in_unit_test b17ed58 Merge pull request diffblue#1555 from thk123/feature/remove-redundant-specalisation-code 9b34cdb Merge pull request diffblue#1564 from owen-jones-diffblue/bugfix/object-numbering-references 52d4326 Merge pull request diffblue#731 from tautschnig/more-rewriting 51133db Remove test checking don't specalise unspecalised generic types bf10b1b Manually call specalisation code bba9f76 Remove redundant regression test 3047678 Removed old method of specalising generics 2db8c45 Merge pull request diffblue#982 from tautschnig/pointer-handling fb532e8 Generalize ID_malloc to ID_allocate with optional zero-init 3c47ccb Use invariant annotations instead of asserts ebd5343 More unwinding should not yield additional assertion failures cc659c9 Use a known constant offset when dereferencing c507ccf Update all constant offsets, not just 0 0361c2a Merge pull request diffblue#1534 from svorenova/unit-test-cleanup f653f85 Merge pull request diffblue#263 from diffblue/owen/fix-memory-bug ede0e8c Fix bug that can cause segfault 51cbfc9 Deleting a utility function for generics 03438bb Disabling part of unit test due to a bug e3019f2 Extending test for derived generics f5ec45a Adding JIRA tickets cont. 1fa8e2f Adding unit test for generic fields 398c88a Applying new utility functions for generics cce7814 Refactoring unit test utility functions to make them easier to use c1e1ba2 Applying new function for accessing elements of arrays e908f0c Updating utility functions to check generic/non-generic java classes d9d9ea1 Cleaning includes, unifying scenario names, adding JIRA references 2883bb1 Extending test for generic arrays de97e23 Adding unit test for nested generics c9a3716 Adding unit test for functions with generics 9db9947 Extending test for generic class 89b99ce Extending test for generic functions 3e6cf35 Extending test for signature/descriptor mismatch 80be2fd Extending and cleaning test for generic class with generic inner classes 2e2e34b Renaming unit test for generic inner classes to bounded generic inner classes c5b06e6 Breaking the old parse_generic_class into two unit tests d3ff11c Adding a utility for checking java generic class 707ebf6 Cleaning existing unit tests af3efea Renaming java files 14c00dc Simplify all expressions generated by flatten_byte_operators 71e9642 Extensions to simplify_byte_extract 81943f2 Split ID_and/ID_or vs ID_xor simplification 77236cc Avoid nesting of ID_with/byte_update by rewriting byte_extract to use the root object ddd3d03 Extended simplify for byte_update, typing 7064483 simplify_typecast: simplify more pointer arithmetic 2b18e0c Merge pull request diffblue#1562 from NathanJPhillips/feature/extend-main_function_result 599a2f9 Merge pull request diffblue#264 from diffblue/smowton/fix/slice24_include de905e7 slice24 test: switch from malloc.h to stdlib.h 89a1132 Merge pull request diffblue#1559 from NathanJPhillips/bugfix/variable-scope 0aeb459 Tidied up get_main_symbol af2d3dd Merge pull request diffblue#1560 from NathanJPhillips/bugfix/catch-by-const-ref c8efb6f Fix bug that can cause segfault b7cc0ae Merge pull request diffblue#1561 from NathanJPhillips/bugfix/erroneous-replacement 7d66469 Typo in reachable 7de4858 Added copyright notice to fix linting error 476270b catch by const ref instead of by value or non-const ref 2f32aee Fixed scope of moved symbol 5057c57 Merge pull request diffblue#1557 from janmroczkowski/janmroczkowski/further-improvements-to-unified_difft 5e067bf Merge pull request diffblue#1481 from andreast271/do-c++-regression c9b6c42 Merge pull request diffblue#1513 from romainbrenguier/feature/input-string-printable c4486f1 Merge pull request diffblue#1552 from thk123/feature/goto-functions-utilities 2648cbb Make unified_difft::lcss return by value cd1258a Merge pull request diffblue#1425 from romainbrenguier/feature/java_new_array_data 6e3a0b0 Make more member function static 9efb65c Merge pull request diffblue#1556 from diffblue/revert-1554-janmroczkowski/more-static-member-functions-in-unified_difft 1c96ae5 Revert "Make more member function static in unified_difft" 9cb4569 Amend doxygen comments 4550676 Added missing utilities to the Makefile 7938bac Correcting linting errors 25d765b Use a for loop rather than chained algorithms e67d229 Renamed find declaration method fa14b47 Renamed utility file to require_goto_statements a657ec1 Moved functions into a namespace and documented them b96199f Moved and simplified the code for finding sub statements b9914a8 Add some java testing utilities. 2c175bd Update load_java_class to construct the entry point function 3453a89 Merge pull request diffblue#1554 from janmroczkowski/janmroczkowski/more-static-member-functions-in-unified_difft feaa85f Merge pull request diffblue#1455 from romainbrenguier/doc/string-solver-documentation c5ab866 Merge pull request diffblue#1430 from romainbrenguier/refactor/gather_indices fac9dea Rename "#lva_mode" to "lvsa_mode" 72c8533 Make two irep IDs 55b6ac5 Merge pull request diffblue#1502 from tautschnig/merge-failed-tests-printer dfa2ed2 Make more member function static d378980 Style: Disabling clang-format in get f5991ee Refactor universal_only_in_index to use expression iterators 9d1aa99 Correct constraints added for char_set e125e8a Refactor gather_indices to use for_each instead of visitor 4b0e2d4 Create goto-gcc symlink in cmake builds and enable goto-gcc tests 7736672 Style: use NOLINTNEXTLINE to avoid cpplint errors on long links 6016bef Improve readability of code imported from failed-tests-printer.pl dd6e431 test.pl: Use native perl instead of "cat" to print log file 3321735 Move implementation of failed-tests-printer.pl into test.pl ba16006 Do not use shell built-ins 96e169a Use single quotes for Windows compatibility d2c3752 Remove string_printable option from the solver b0de0e3 Test for string printable option on input strings 4b36fc6 Merge pull request diffblue#1533 from mgudemann/fix/support_class_bounds_generics 35096b8 Initialize architecture in `instantiate_not_contains` unit test b25630a Merge pull request diffblue#1550 from chrisr-diffblue/cleanup/java-generics-test-helpers 542a26d Stop adding printable constraints on all strings e65e340 Use command line option for string-printable param 8e92362 Propagate string-printable option in object_factory ae5f32e Add a printable option to string initialization 514e6a1 Add function to call constrain_character primitive 1d92c48 Add string primitive to constrain characters cb01526 Minor refactoring in add_default_axioms e1280cc Add utility function add_constraint_on_characters 6b88eb8 Add unit test for class / interface bound 2ed059a Support interface and class bound parsing in generics ccdd483 Merge pull request diffblue#1545 from chrisr-diffblue/TG-1158/unit-test-for-specialising-with-array-types 73808aa Merge pull request diffblue#1544 from smowton/smowton/feature/value_set_eq_operator 0507355 Refactored unit test helpers to be more general and extend their use-cases 93ebb84 Merge commit '356aed461b387a8ae815a9901a16d26f32f102be' into develop db758fb Add some unit test helper functions, useful for Java generics unit tests 98de899 Add a unit test for specialising Java generic types with array types b07fcdd Documentation improvements and readme for strings 1fa64a9 Avoid using is_valid_java_array in builin_functions 0dafac2 Add unit test for goto_trace_output in Makefile 435958f Unit test for goto_trace::output 5a0343f Doc: Summary for count_type_leaves fc363b3 Typo in goto_trace output 42c079d Use existing function for checking object is array 465e5dc Style: improve documentation in interpreter evaluate fe2efa7 Style: Replace assert by appropriate macros e36d7d8 Check if object is nil before writing trace 6b519ad Add identifier and rename statement to java_new_array_data d4f1b29 Add eq and neq operators to value_sett and related types b03ec16 Merge pull request diffblue#239 from diffblue/bugfix/value_sets_fi_and_reaching_defs_retrievals_of_dynamic_objects db79106 Added explanatory comment for the introduced condition. dfc6a20 Fixing C++ code-style issues. b0742cf Disable cbmc-cpp tests in appveyor, which runs regression on windows. All cbmc-cpp tests #include <assert.h> and cbmc cannot yet parse Microsoft C++ headers. d55a8da Add tests to cmake regression: cbmc-cover, cbmc-cpp, goto-analyzer-taint 3a4e48c Run cbmc c++ regression as part of default regression test Set is_parameter for c++ function parameter symbol 7989831 Added regression test for the fixed bug. 00b4af2 Bugfix: Explicit retrievals of DOs from value_set amd reaching_defs. git-subtree-dir: cbmc git-subtree-split: e8b3cb9

tautschnig force-pushed the more-rewriting branch from de9dd18 to 41bafb7 Compare March 30, 2017 11:53

tautschnig requested a review from kroening March 30, 2017 11:53

kroening reviewed Apr 4, 2017

View reviewed changes

tautschnig force-pushed the more-rewriting branch from 41bafb7 to 4760af3 Compare April 7, 2017 12:37

tautschnig force-pushed the more-rewriting branch 3 times, most recently from 16c3d57 to 570cd83 Compare April 10, 2017 07:10

tautschnig mentioned this pull request Apr 21, 2017

Clean up code for pointer function parameters #853

Merged

tautschnig mentioned this pull request May 23, 2017

Properly handle void* #955

Merged

tautschnig added the do not merge label May 23, 2017

tautschnig self-assigned this May 23, 2017

tautschnig force-pushed the more-rewriting branch 2 times, most recently from c6377a0 to 809c36a Compare June 2, 2017 08:08

tautschnig removed the do not merge label Jun 2, 2017

tautschnig assigned kroening and unassigned tautschnig Jun 2, 2017

tautschnig force-pushed the more-rewriting branch from ac95157 to 3b7b57a Compare June 8, 2017 08:56

peterschrammel approved these changes Jun 25, 2017

View reviewed changes

peterschrammel added enhancement Solvers labels Jun 25, 2017

tautschnig force-pushed the more-rewriting branch from 3b7b57a to e1a0d00 Compare June 28, 2017 14:44

tautschnig force-pushed the more-rewriting branch from e1a0d00 to fb0c891 Compare August 7, 2017 12:26

tautschnig changed the base branch from master to develop August 22, 2017 12:31

tautschnig assigned tautschnig and unassigned kroening Sep 2, 2017

kroening reviewed Nov 6, 2017

View reviewed changes

tautschnig force-pushed the more-rewriting branch from 8279ae8 to eb52331 Compare November 6, 2017 12:20

tautschnig requested review from martin-cs and romainbrenguier as code owners November 6, 2017 12:20

kroening reviewed Nov 6, 2017

View reviewed changes

kroening approved these changes Nov 6, 2017

View reviewed changes

tautschnig and others added 6 commits November 6, 2017 14:09

simplify_typecast: simplify more pointer arithmetic

7064483

Extended simplify for byte_update, typing

ddd3d03

Avoid nesting of ID_with/byte_update by rewriting byte_extract to use…

77236cc

… the root object

Split ID_and/ID_or vs ID_xor simplification

81943f2

The code paths are vastly different, and splitting thus enables optimisations.

Extensions to simplify_byte_extract

71e9642

Further simplify extracts from arrays and structs

tautschnig force-pushed the more-rewriting branch from eb52331 to 14c00dc Compare November 6, 2017 14:09

peterschrammel merged commit 52d4326 into diffblue:develop Nov 7, 2017

tautschnig deleted the more-rewriting branch November 8, 2017 12:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplifier and symex rewriting fixes/extensions #731

Simplifier and symex rewriting fixes/extensions #731

tautschnig commented Mar 30, 2017

kroening Apr 4, 2017

tautschnig Apr 7, 2017

kroening Apr 4, 2017

tautschnig commented Apr 7, 2017

tautschnig commented Apr 10, 2017

tautschnig commented May 23, 2017

peterschrammel commented Jun 25, 2017

marek-trtik commented Nov 2, 2017

tautschnig commented Nov 2, 2017

thk123 commented Nov 6, 2017

thk123 commented Nov 6, 2017

kroening Nov 6, 2017

tautschnig Nov 6, 2017

kroening Nov 6, 2017

kroening Nov 6, 2017

kroening Nov 6, 2017

tautschnig Nov 6, 2017

kroening Nov 6, 2017

tautschnig Nov 6, 2017

tautschnig commented Nov 6, 2017

kroening Nov 6, 2017

kroening left a comment

thk123 commented Nov 6, 2017 •

edited

Loading

tautschnig commented Nov 6, 2017

tautschnig commented Nov 6, 2017

thk123 commented Nov 6, 2017

tautschnig commented Nov 7, 2017

kroening commented Nov 7, 2017

thk123 commented Nov 7, 2017

peterschrammel commented Nov 7, 2017

Simplifier and symex rewriting fixes/extensions #731

Simplifier and symex rewriting fixes/extensions #731

Conversation

tautschnig commented Mar 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tautschnig commented Apr 7, 2017

tautschnig commented Apr 10, 2017

tautschnig commented May 23, 2017

peterschrammel commented Jun 25, 2017

marek-trtik commented Nov 2, 2017

tautschnig commented Nov 2, 2017

thk123 commented Nov 6, 2017

thk123 commented Nov 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tautschnig commented Nov 6, 2017

Choose a reason for hiding this comment

kroening left a comment

Choose a reason for hiding this comment

thk123 commented Nov 6, 2017 • edited Loading

tautschnig commented Nov 6, 2017

tautschnig commented Nov 6, 2017

thk123 commented Nov 6, 2017

tautschnig commented Nov 7, 2017

kroening commented Nov 7, 2017

thk123 commented Nov 7, 2017

peterschrammel commented Nov 7, 2017

thk123 commented Nov 6, 2017 •

edited

Loading