Repository navigation
c-api: refresh cached Ref in C++ type copy assignment - #15109
Closed
netliomax25-code wants to merge 1 commit into
Closed
netliomax25-code wants to merge 1 commit into
netliomax25-code wants to merge 1 commit into
Conversation
FuncType, GlobalType, TableType and MemoryType cache a Ref next to the owning ptr and hand it out from operator-> and operator*, but their copy assignment only reset ptr, which left ref pointing at the object that reset just freed. Update ref the same way TagType and ValType already do.
netliomax25-code
requested review from
pchickey
and removed request for
a team
November 2, 2026 11:25
Member
|
Thanks for the PR, but this is a duplicate of #15098 which fixes another location as well, so I'm going to close this |
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
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.
Related: #15085, which fixes a different problem in other C++ copy assignment operators and does not touch these four classes.
FuncType,GlobalType,TableTypeandMemoryTypecache aRef refnext to the owningptr, andoperator->andoperator*hand out that cachedref. Their copy assignment replaces the owned object withptr.reset(...)but does not updateref, so afterb = athe cached pointer still refers to the objectresetjust freed, andb->params()reads freed memory. This needs nothing more unusual than assigning one existing value to another.__asan_describe_addresson the pointer held byrefreports it as freed insideFuncType::operator=, and a debug build aborts inty():Move assignment copies
refalong withptr, and the existing tests assign from temporaries, which is likely why this went unnoticed.func_type.ccdoes copy-assign, but never reads the result.The fix adds
ref = ptr.get();to all four operators, which is whatTagTypeandValTypealready do. Those two are the only other classes with a cachedRef. Only use through->or*should be affected, since the other paths readptrdirectly. The C++ API is tier 2 and the trigger is in embedder code, so I don't think this falls under the security process.I added a
CopyAssigntest for each type. All four abort without the fix and pass with it. The C API job'sninja test(184 tests, Debug,-DBUILD_SHARED_LIBS=ON -DBUILD_TESTS=ON) passes on macOS aarch64 with and without-fsanitize=address, and clang-format 22 is clean. I have not run it on Linux or Windows.