Skip to content

Permit initializers in library functions #6590

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 6 commits into from
Dec 9, 2022

Conversation

tautschnig
Copy link
Collaborator

@tautschnig tautschnig commented Jan 18, 2022

The goal of this series of commits is to reduce the number of __CPROVER_... static-lifetime objects that appear in every program irrespective of their use by C (model) library functions. This will limit verification overhead to those cases where they are actually used. This will also enable more costly objects (such as unbounded arrays) to be used by future library instrumentation.

  • Each commit message has a non-empty body, explaining why the change was made.
  • n/a 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).
  • My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@tautschnig tautschnig self-assigned this Jan 18, 2022
@codecov
Copy link

codecov bot commented Jan 18, 2022

Codecov Report

Base: 78.41% // Head: 78.39% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (d849c8f) compared to base (2653d78).
Patch coverage: 96.66% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6590      +/-   ##
===========================================
- Coverage    78.41%   78.39%   -0.03%     
===========================================
  Files         1655     1655              
  Lines       190264   190281      +17     
===========================================
- Hits        149201   149172      -29     
- Misses       41063    41109      +46     
Impacted Files Coverage Δ
src/ansi-c/ansi_c_internal_additions.cpp 90.12% <ø> (ø)
src/cpp/cpp_internal_additions.cpp 88.09% <ø> (-2.11%) ⬇️
...strument/contracts/dynamic-frames/dfcc_library.cpp 95.54% <ø> (ø)
...cts/dynamic-frames/dfcc_lift_memory_predicates.cpp 89.60% <40.00%> (ø)
src/ansi-c/c_typecheck_expr.cpp 75.68% <100.00%> (ø)
src/ansi-c/cprover_library.cpp 97.91% <100.00%> (ø)
src/cpp/cprover_library.cpp 92.30% <100.00%> (ø)
...ument/contracts/dynamic-frames/dfcc_instrument.cpp 81.44% <100.00%> (ø)
...contracts/dynamic-frames/dfcc_pointer_in_range.cpp 100.00% <100.00%> (ø)
...t/contracts/dynamic-frames/dfcc_pointer_in_range.h 100.00% <100.00%> (ø)
... and 24 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tautschnig tautschnig force-pushed the link-library-functions branch 5 times, most recently from c3f5f03 to ad72b1e Compare January 21, 2022 14:51
@tautschnig tautschnig marked this pull request as ready for review January 21, 2022 16:38
@tautschnig tautschnig removed their assignment Jan 21, 2022
@tautschnig tautschnig force-pushed the link-library-functions branch from ad72b1e to a52875a Compare January 21, 2022 21:45
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.

This seems reasonable and sensible to me.

@tautschnig tautschnig force-pushed the link-library-functions branch from a52875a to 236b6ef Compare January 22, 2022 20:05
@tautschnig tautschnig removed their assignment Jan 22, 2022
@tautschnig tautschnig force-pushed the link-library-functions branch 3 times, most recently from d544bb6 to 10bec57 Compare February 7, 2022 21:29
Comment on lines +119 to +127
{
changed = true;
added_functions.insert(id);

Copy link
Collaborator

@remi-delmas-3000 remi-delmas-3000 Sep 28, 2022

Choose a reason for hiding this comment

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

How about reformulating the control flow structure as

if(body_available || already_added)
 continue;

// add the symbol
changed = true;
added_functions.insert(id);
...
 

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could do, but then I'd have to touch more code in this PR.

@tautschnig tautschnig changed the title Permit initializers in library functions Permit initializers in library functions [depends-on: #7370, #7371, #7372] Nov 22, 2022
@tautschnig tautschnig marked this pull request as draft November 22, 2022 14:04
@tautschnig tautschnig changed the title Permit initializers in library functions [depends-on: #7370, #7371, #7372] Permit initializers in library functions Nov 22, 2022
@tautschnig tautschnig removed their assignment Nov 22, 2022
@tautschnig tautschnig force-pushed the link-library-functions branch from a9db9b9 to 4572567 Compare November 22, 2022 19:33
@tautschnig tautschnig marked this pull request as ready for review November 22, 2022 22:44
@tautschnig tautschnig force-pushed the link-library-functions branch from 4572567 to 42ddaab Compare December 1, 2022 16:07
@peterschrammel peterschrammel removed their assignment Dec 8, 2022
@tautschnig tautschnig assigned tautschnig and unassigned kroening and nwetzler Dec 8, 2022
tautschnig and others added 6 commits December 8, 2022 23:13
MSVC won't have the built-in declarations available, and will thus fail
type checking.
The test descriptions expect valid enum values, which we didn't actually
have any guarantee built into the test for.
This enables type checking of missing functions vs library-provided functions,
and also enables type sanitisation as done by the linker.

Co-authored-by: Peter Schrammel <[email protected]>
With the support of building initialisers while adding the library we
can move library-only symbols to the library. This avoids polluting the
initialisation function for all cases that don't actually use
pipe-related library functions.
With the support of building initialisers while adding the library we
can move library-only symbols to the library. This avoids polluting the
initialisation function for all cases that don't actually use
memory (de-)allocation library functions.
With the support of building initialisers while adding the library we
can move library-only symbols to the library. This avoids polluting the
initialisation function for all cases that don't actually use
pthreads-related library functions.

This removal of unused symbols required adding a symbol into the
array_of_bool_as_bitvec test, which previously implicitly relied on
those library-specific symbols in the patterns being tested for.
@tautschnig tautschnig force-pushed the link-library-functions branch from 42ddaab to d849c8f Compare December 8, 2022 23:13
@tautschnig tautschnig merged commit 345f4bf into diffblue:develop Dec 9, 2022
@tautschnig tautschnig deleted the link-library-functions branch December 9, 2022 00:41
tautschnig added a commit to tautschnig/cbmc that referenced this pull request Dec 12, 2022
With diffblue#6590, several unnecessary symbols are no longer included in goto
programs. These include unbounded arrays, which the incremental SMT
back-end hitherto does not support. The use of --slice-formula was a
workaround to make sure the unnecessary symbols (or equalities over
them) don't end up in the formula; now they aren't part of the goto
program, so the workaround no longer adds value.
tautschnig added a commit to tautschnig/cbmc that referenced this pull request Dec 13, 2022
With diffblue#6590, several unnecessary symbols are no longer included in goto
programs. These include unbounded arrays, which the incremental SMT
back-end hitherto does not support. The use of --slice-formula was a
workaround to make sure the unnecessary symbols (or equalities over
them) don't end up in the formula; now they aren't part of the goto
program, so the workaround no longer adds value.
tautschnig added a commit to tautschnig/cbmc that referenced this pull request Dec 13, 2022
With diffblue#6590, several unnecessary symbols are no longer included in goto
programs. These include unbounded arrays, which the incremental SMT
back-end hitherto does not support. The use of --slice-formula was a
workaround to make sure the unnecessary symbols (or equalities over
them) don't end up in the formula; now they aren't part of the goto
program, so the workaround no longer adds value.
tautschnig added a commit to tautschnig/cbmc that referenced this pull request Dec 14, 2022
With diffblue#6590, several unnecessary symbols are no longer included in goto
programs. These include unbounded arrays, which the incremental SMT
back-end hitherto does not support. The use of --slice-formula was a
workaround to make sure the unnecessary symbols (or equalities over
them) don't end up in the formula; now they aren't part of the goto
program, so the workaround no longer adds value.
tautschnig added a commit to tautschnig/cbmc that referenced this pull request Dec 14, 2022
With diffblue#6590, several unnecessary symbols are no longer included in goto
programs. These include unbounded arrays, which the incremental SMT
back-end hitherto does not support. The use of --slice-formula was a
workaround to make sure the unnecessary symbols (or equalities over
them) don't end up in the formula; now they aren't part of the goto
program, so the workaround no longer adds value.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants