Skip to content

Fix linking in case of function type conflict #6547

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
Oct 10, 2022

Conversation

tautschnig
Copy link
Collaborator

We already almost did the right thing, but were failing to update types at call sites. This PR also adds improved conflict reporting and an actual test of such type conflicts.

  • 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.
  • n/a 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.
  • n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@tautschnig tautschnig marked this pull request as draft December 27, 2021 09:15
@tautschnig tautschnig force-pushed the pointer-parameter-type-conflict branch 3 times, most recently from c0aa8f5 to 068a9d8 Compare December 31, 2021 14:19
@codecov
Copy link

codecov bot commented Dec 31, 2021

Codecov Report

Base: 77.92% // Head: 77.96% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (39e7078) compared to base (50c43e2).
Patch coverage: 82.82% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6547      +/-   ##
===========================================
+ Coverage    77.92%   77.96%   +0.03%     
===========================================
  Files         1616     1616              
  Lines       186814   186855      +41     
===========================================
+ Hits        145580   145675      +95     
+ Misses       41234    41180      -54     
Impacted Files Coverage Δ
src/linking/linking_class.h 100.00% <ø> (ø)
src/linking/linking.cpp 69.05% <79.76%> (+9.50%) ⬆️
src/goto-programs/link_goto_model.cpp 87.50% <100.00%> (+1.99%) ⬆️
src/goto-programs/goto_program.cpp 80.24% <0.00%> (-1.56%) ⬇️
src/goto-cc/compile.cpp 73.39% <0.00%> (+0.61%) ⬆️
src/util/rename_symbol.cpp 83.43% <0.00%> (+2.95%) ⬆️

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 marked this pull request as ready for review December 31, 2021 16:37
@tautschnig tautschnig removed their assignment Dec 31, 2021
@tautschnig tautschnig force-pushed the pointer-parameter-type-conflict branch from 068a9d8 to 3b73b4d Compare February 1, 2022 11:37
Copy link

@chris-ryder chris-ryder left a comment

Choose a reason for hiding this comment

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

Looks sensible, and thanks for the extra tests!

@chris-ryder chris-ryder removed their assignment Feb 5, 2022
@tautschnig tautschnig force-pushed the pointer-parameter-type-conflict branch 4 times, most recently from c0a3df4 to 3a2bbfc Compare February 9, 2022 22:24
@tautschnig tautschnig force-pushed the pointer-parameter-type-conflict branch 2 times, most recently from 7cc5253 to 0e8fb79 Compare March 30, 2022 21:33
@tautschnig tautschnig force-pushed the pointer-parameter-type-conflict branch from 0e8fb79 to 1e1426f Compare July 12, 2022 11:21
@tautschnig tautschnig force-pushed the pointer-parameter-type-conflict branch 2 times, most recently from 9f54262 to 92fc2e3 Compare September 28, 2022 10:32
@feliperodri feliperodri added the aws Bugs or features of importance to AWS CBMC users label Oct 1, 2022
@tautschnig tautschnig self-assigned this Oct 4, 2022
@tautschnig tautschnig force-pushed the pointer-parameter-type-conflict branch from 92fc2e3 to d56bedf Compare October 9, 2022 09:35
@tautschnig tautschnig assigned tautschnig and unassigned tautschnig Oct 9, 2022
We previously just reported pointer type conflicts without elaborating
what the exact difference was. To produce type-consistent goto programs,
we chose to use the type that the definition had, but lacked any test
that doing so would actually result in a program that can be analysed
successfully.

This commit fixes bugs in the code producing diagnostics, enables
diagnostics for the case of pointer type conflicts, and adds a test to
demonstrate that we successfully analyse programs after type conflict
resolution. Despite this, however, inconsistencies in the goto program
remain: the call types still need fixing up.

Fixes: diffblue#198
The linker already accepted a number of type mismatches for function
declarations, but did not actually modify the goto functions to reflect
these type mismatches, just the type information in the symbol table was
updated. These changes enable support for conflicts on function return
types, which are now resolved by introducing type casts.
@tautschnig tautschnig force-pushed the pointer-parameter-type-conflict branch from d56bedf to 39e7078 Compare October 9, 2022 15:31
@tautschnig tautschnig removed their assignment Oct 9, 2022
@kroening kroening merged commit 60b95b2 into diffblue:develop Oct 10, 2022
@tautschnig tautschnig deleted the pointer-parameter-type-conflict branch October 10, 2022 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws Bugs or features of importance to AWS CBMC users bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants