Make function pointer removal deterministic#8952
Conversation
Sort function pointer candidates by identifier name before generating the dispatch code. This ensures the order of IF/GOTO branches in the resulting GOTO program is independent of internal hash table ordering, which can vary when irep IDs are added or removed. Without this fix, unrelated changes to irep_ids.def can cause different function pointer dispatch orderings, leading to different symbolic execution paths and dramatically different SAT solver performance. Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR makes function-pointer dispatch generation deterministic by sorting candidate targets by identifier before emitting IF/GOTO branches, preventing nondeterministic ordering from unordered_set iteration and avoiding downstream symbolic-execution / solver-performance variance.
Changes:
- Convert the function-candidate container from
unordered_setiteration order to a sortedvectorfor deterministic dispatch emission. - Update the assertion-comment helper to accept an ordered candidate list.
- Add a regression test ensuring deterministic sorted emission independent of CLI target ordering.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/goto-programs/remove_function_pointers.cpp | Sort function-pointer targets by identifier before generating dispatch; adjust helper signature accordingly. |
| regression/goto-instrument/restrict-function-pointer-deterministic-order/test.desc | Adds regression expectations asserting stable, sorted IF-branch order and assertion text. |
| regression/goto-instrument/restrict-function-pointer-deterministic-order/test.c | New minimal C test exercising function-pointer call-site restriction/dispatch. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static std::string | ||
| function_pointer_assertion_comment(const std::vector<symbol_exprt> &candidates) |
There was a problem hiding this comment.
After changing function_pointer_assertion_comment to take a std::vector, its output determinism now depends on callers providing a pre-sorted vector. To make this helper robust (and prevent future call sites from accidentally reintroducing nondeterministic output), either (a) sort a local copy inside function_pointer_assertion_comment, or (b) clearly encode the precondition in the API (e.g., rename the parameter to sorted_candidates and add a short comment that the vector must be sorted by identifier).
| functions.begin(), | ||
| functions.end(), | ||
| [](const symbol_exprt &a, const symbol_exprt &b) | ||
| { return id2string(a.get_identifier()) < id2string(b.get_identifier()); }); |
There was a problem hiding this comment.
The comparator converts both identifiers via id2string(...) on every comparison, which increases work during sorting (O(n log n) conversions/allocations). Prefer comparing identifiers without repeated string materialization (e.g., compare irep_idt directly if it has a strict ordering, or precompute the string keys once and sort by those).
| { return id2string(a.get_identifier()) < id2string(b.get_identifier()); }); | |
| { return a.get_identifier() < b.get_identifier(); }); |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #8952 +/- ##
========================================
Coverage 80.47% 80.47%
========================================
Files 1704 1704
Lines 188694 188699 +5
Branches 73 73
========================================
+ Hits 151843 151854 +11
+ Misses 36851 36845 -6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This release ensures cross-platform determinism of formulae created by CBMC (for a given input program), ensuring consistent solver performance (via diffblue#8830, diffblue#8952). This releases fixes a variety of bugs, including the problem of orphan (SMT-solver) child processes left behind after terminating CBMC (via diffblue#8900), and various encoding improvements for unions (via diffblue#8950, diffblue#8958) and multi-dimensional arrays (via diffblue#8705).
This release ensures cross-platform determinism of formulae created by CBMC (for a given input program), ensuring consistent solver performance (via diffblue#8830, diffblue#8952). This releases fixes a variety of bugs, including the problem of orphan (SMT-solver) child processes left behind after terminating CBMC (via diffblue#8900), and various encoding improvements for unions (via diffblue#8950, diffblue#8958) and multi-dimensional arrays (via diffblue#8705).
Sort function pointer candidates by identifier name before generating the dispatch code. This ensures the order of IF/GOTO branches in the resulting GOTO program is independent of internal hash table ordering, which can vary when irep IDs are added or removed.
Without this fix, unrelated changes to irep_ids.def can cause different function pointer dispatch orderings, leading to different symbolic execution paths and dramatically different SAT solver performance.