fix(core): correct constructor mirror backup target and reset methodN… - #5
Open
nanhang-950 wants to merge 1 commit into
Conversation
…ames per iteration Bug 1 — Constructor "mirror backup" passed the wrong target member: In the HOOK_CONSTRUCTOR / BACKUP_CONSTRUCTOR branches (both hook and unhook paths) putMirrorBackup(backupRecords, targetMethod, ...) was called with targetMethod, which is always null in the constructor branch (it should be targetConstructor). This caused BackupRecord.target to be null, propagating into backup(null) / backupNative(null) and, on slot-populated classes, could silently fail or crash the ART runtime. Non-constructor method branches already passed targetMethod correctly and are unchanged. Bug 2 — methodNames array reused across iterations retained stale data: String[] methodNames = new String[2]; was declared outside the loop. Each iteration only overwrote some slots (depending on whether the hook/backup member was null), so stale values from a previous target leaked into slotMap.containsKey matching, causing spurious slot matches for prior targets. Move the declaration inside the loop so every iteration gets a fresh array.
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
1 participant
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.
…ames per iteration
Bug 1 — Constructor "mirror backup" passed the wrong target member: In the HOOK_CONSTRUCTOR / BACKUP_CONSTRUCTOR branches (both hook and unhook paths) putMirrorBackup(backupRecords, targetMethod, ...) was called with targetMethod, which is always null in the constructor branch (it should be targetConstructor). This caused BackupRecord.target to be null, propagating into backup(null) / backupNative(null) and, on slot-populated classes, could silently fail or crash the ART runtime. Non-constructor method branches already passed targetMethod correctly and are unchanged.
Bug 2 — methodNames array reused across iterations retained stale data: String[] methodNames = new String[2]; was declared outside the loop. Each iteration only overwrote some slots (depending on whether the hook/backup member was null), so stale values from a previous target leaked into slotMap.containsKey matching, causing spurious slot matches for prior targets. Move the declaration inside the loop so every iteration gets a fresh array.