Fix "One Definition Rule" Violation for Prover::GetForm#306
Merged
Conversation
Member
|
Apologies for not kicking off the CI earlier - I cherry-picked the change into my hardware branch earlier today. Hopefully this comes up green and I'll merge it in |
Member
|
You can ignore mac-intel failures. Those runners are getting brittle and I will re-run them |
Member
|
Thanks! |
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As discussed in this comment, commit 767a534 changed
prover_base.hppto declarevirtual form GetForm(uint64_t iteration) = 0;, but didn't adaptvdf_base.hppleaving it withvirtual form* GetForm(uint64_t iteration) = 0;. This violates the "one definition rule" and causes a double-free crash inhw_proof.cpp(for all "hw" binaries,hw_proof.cppis compiled usingvdf_base.hpp, but linked withvdf_base.cppwhich is compiled withprover_base.hpp).This commit fixes this by adjusting
vdf_base.hppto once again matchprover_base.hpp. The changes introduced in #299 will runemu_hw_testduring CI, which would have caught that issue.Ideally, a future change should clean up these headers to share a single defintion of
Prover.Note
Low Risk
Small signature alignment and corresponding call-site adjustment; behavior is unchanged aside from avoiding pointer lifetime/ABI issues that previously caused crashes.
Overview
Fixes a header mismatch by changing
Prover::GetForminvdf_base.hppfrom returningform*to returningform, aligning it withprover_base.hppand preventing ODR/link-time type inconsistencies.Updates
HwProver::GetForminhw_proof.cppto returnformby value, dereferencinghw_proof_value_at(...)and returning a concrete fallbackformwhen stopping, which avoids the crash/double-free seen in HW proof binaries.Written by Cursor Bugbot for commit 12dcbf8. This will update automatically on new commits. Configure here.